netdev - Re: [PATCH net-next v4 6/8] net: sched: create tc_can_offload

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 20 Jan 2018 02:12:25 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     David Miller <davem@...emloft.net>,
        David Ahern <dsahern@...il.com>, aring@...atatu.com,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Linux Netdev List <netdev@...r.kernel.org>,
        oss-drivers@...ronome.com,
        Quentin Monnet <quentin.monnet@...ronome.com>
Subject: Re: [PATCH net-next v4 6/8] net: sched: create tc_can_offload_extack()
 wrapper

On Sat, Jan 20, 2018 at 12:59 AM, Jiri Pirko <jiri@...nulli.us> wrote:
> Sat, Jan 20, 2018 at 02:44:48AM CET, jakub.kicinski@...ronome.com wrote:
>>From: Quentin Monnet <quentin.monnet@...ronome.com>
>>
>>Create a wrapper around tc_can_offload() that takes an additional
>>extack pointer argument in order to output an error message if TC
>>offload is disabled on the device.
>>
>>In this way, the error message is handled by the core and can be the
>>same for all drivers.
>>
>>Signed-off-by: Quentin Monnet <quentin.monnet@...ronome.com>
>>Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>>---
>> include/net/pkt_cls.h | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>>diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
>>index f497f622580b..2f8f16a4d88e 100644
>>--- a/include/net/pkt_cls.h
>>+++ b/include/net/pkt_cls.h
>>@@ -656,6 +656,17 @@ static inline bool tc_can_offload(const struct net_device *dev)
>>       return dev->features & NETIF_F_HW_TC;
>> }
>>
>>+static inline bool tc_can_offload_extack(const struct net_device *dev,
>>+                                       struct netlink_ext_ack *extack)
>
> I don't like to add tc_can_offload variant for this. It makes sense
> the original tc_can_offload to be extended and set the extack message
> always.
>
> It would require some more work in drivers (5), sure, but we endup with
> nicer and consistent code.

$ git grep tc_can_offload
drivers/net/ethernet/broadcom/bnxt/bnxt.c:      if
(!bnxt_tc_flower_enabled(bp) || !tc_can_offload(bp->dev))
drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c:  if
(!bnxt_tc_flower_enabled(vf_rep->bp) || !tc_can_offload(bp->dev))
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c:        if
(!tc_can_offload(dev))
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:  if
(!tc_can_offload(adapter->netdev))
drivers/net/ethernet/mellanox/mlx5/core/en_main.c:      if
(!tc_can_offload(priv->netdev))
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:       if
(!tc_can_offload(priv->netdev))
drivers/net/ethernet/mellanox/mlxsw/spectrum.c:         if
(!tc_can_offload(mlxsw_sp_port->dev))
drivers/net/ethernet/netronome/nfp/bpf/main.c:
!tc_can_offload(nn->dp.netdev) ||
drivers/net/ethernet/netronome/nfp/flower/offload.c:    if
(!tc_can_offload(repr->netdev))
drivers/net/ethernet/netronome/nfp/flower/offload.c:    if
(!tc_can_offload(repr->netdev))
drivers/net/netdevsim/bpf.c:        !tc_can_offload(ns->netdev) ||
include/net/pkt_cls.h:static inline bool tc_can_offload(const struct
net_device *dev)
net/dsa/slave.c:        if (!tc_can_offload(dev))
net/sched/cls_api.c:    if (!tc_can_offload(dev) &&
tcf_block_offload_in_use(block))
net/sched/sch_mq.c:     if (!tc_can_offload(dev) ||
!dev->netdev_ops->ndo_setup_tc)
net/sched/sch_prio.c:   if (!tc_can_offload(dev) ||
!dev->netdev_ops->ndo_setup_tc)
net/sched/sch_prio.c:   if (!tc_can_offload(dev) ||
!dev->netdev_ops->ndo_setup_tc)
net/sched/sch_red.c:    if (!tc_can_offload(dev) ||
!dev->netdev_ops->ndo_setup_tc)
net/sched/sch_red.c:    if (!tc_can_offload(dev) ||
!dev->netdev_ops->ndo_setup_tc)

Do you mean the qdisc offloads too?  The whole lot?