From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [PATCH net-next v4 6/8] net: sched: create tc_can_offload_extack() wrapper Date: Sat, 20 Jan 2018 11:22:36 +0100 Message-ID: <20180120102236.GF2147@nanopsycho.orion> References: <20180120014450.29666-1-jakub.kicinski@netronome.com> <20180120014450.29666-7-jakub.kicinski@netronome.com> <20180120085929.GB2147@nanopsycho.orion> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: David Miller , David Ahern , aring@mojatatu.com, Daniel Borkmann , Alexei Starovoitov , Linux Netdev List , oss-drivers@netronome.com, Quentin Monnet To: Jakub Kicinski Return-path: Received: from mail-wr0-f193.google.com ([209.85.128.193]:43012 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751022AbeATKWi (ORCPT ); Sat, 20 Jan 2018 05:22:38 -0500 Received: by mail-wr0-f193.google.com with SMTP id t16so3730907wrc.10 for ; Sat, 20 Jan 2018 02:22:37 -0800 (PST) Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Sat, Jan 20, 2018 at 11:12:25AM CET, jakub.kicinski@netronome.com wrote: >On Sat, Jan 20, 2018 at 12:59 AM, Jiri Pirko wrote: >> Sat, Jan 20, 2018 at 02:44:48AM CET, jakub.kicinski@netronome.com wrote: >>>From: Quentin Monnet >>> >>>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 >>>Reviewed-by: Jakub Kicinski >>>--- >>> 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? Yes.