All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers
@ 2018-01-25 22:00 Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 01/11] pkt_cls: add new tc cls helper to check offload flag and chain index Jakub Kicinski
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski

Hi!

This set makes all drivers use a new tc_cls_can_offload_and_chain0()
helper which will set extack in case TC hw offload flag is disabled.

I chose to keep the new helper which also looks at the chain but
renamed it more appropriately.  The rationale being that most drivers
don't accept chains other than 0 and since we have to pass extack
to the helper we can as well pass the entire struct tc_cls_common_offload
and perform the most common checks.

This code makes the assumption that type_data in the callback can
be interpreted as struct tc_cls_common_offload, i.e. the real offload
structure has common part as the first member.  This allows us to
make the check once for all classifier types if driver supports
more than one.

v1:
 - drop the type validation in nfp and netdevsim.
v2:
 - reorder checks in patch 1;
 - split other changes from patch 1;
 - add the i40e patch in;
 - add one more test case - for chain 0 extack.

Jakub Kicinski (11):
  pkt_cls: add new tc cls helper to check offload flag and chain index
  netdevsim: use tc_cls_can_offload_and_chain0()
  nfp: use tc_cls_can_offload_and_chain0()
  cxgb4: use tc_cls_can_offload_and_chain0()
  mlx5: use tc_cls_can_offload_and_chain0()
  bnxt: use tc_cls_can_offload_and_chain0()
  ixgbe: use tc_cls_can_offload_and_chain0()
  i40e: use tc_cls_can_offload_and_chain0()
  mlxsw: use tc_cls_can_offload_and_chain0()
  selftests/bpf: check for spurious extacks from the driver
  selftests/bpf: check for chain-non-0 extack message

 drivers/net/ethernet/broadcom/bnxt/bnxt.c          |  3 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c       |  3 --
 drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c      |  3 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c    |  8 +---
 drivers/net/ethernet/intel/i40e/i40e_main.c        |  8 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |  5 +--
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  |  5 +--
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c   |  5 +--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     |  6 +--
 drivers/net/ethernet/netronome/nfp/bpf/main.c      |  4 +-
 .../net/ethernet/netronome/nfp/flower/offload.c    |  7 ++--
 drivers/net/netdevsim/bpf.c                        |  5 +--
 include/net/pkt_cls.h                              | 14 +++++++
 tools/testing/selftests/bpf/test_offload.py        | 43 ++++++++++++++++++++--
 14 files changed, 72 insertions(+), 47 deletions(-)

-- 
2.15.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 01/11] pkt_cls: add new tc cls helper to check offload flag and chain index
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 02/11] netdevsim: use tc_cls_can_offload_and_chain0() Jakub Kicinski
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski

Very few (mlxsw) upstream drivers seem to allow offload of chains
other than 0.  Save driver developers typing and add a helper for
checking both if ethtool's TC offload flag is on and if chain is 0.
This helper will set the extack appropriately in both error cases.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 include/net/pkt_cls.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index fa2f6fb14093..87406252f0a3 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -656,6 +656,20 @@ static inline bool tc_can_offload_extack(const struct net_device *dev,
 	return can;
 }
 
+static inline bool
+tc_cls_can_offload_and_chain0(const struct net_device *dev,
+			      struct tc_cls_common_offload *common)
+{
+	if (!tc_can_offload_extack(dev, common->extack))
+		return false;
+	if (common->chain_index) {
+		NL_SET_ERR_MSG(common->extack,
+			       "Driver supports only offload of chain 0");
+		return false;
+	}
+	return true;
+}
+
 static inline bool tc_skip_hw(u32 flags)
 {
 	return (flags & TCA_CLS_FLAGS_SKIP_HW) ? true : false;
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 02/11] netdevsim: use tc_cls_can_offload_and_chain0()
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 01/11] pkt_cls: add new tc cls helper to check offload flag and chain index Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 03/11] nfp: " Jakub Kicinski
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski

Make use of tc_cls_can_offload_and_chain0() to set extack msg in case
ethtool tc offload flag is not set or chain unsupported.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 drivers/net/netdevsim/bpf.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 8166f121bbcc..de73c1ff0939 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -135,7 +135,7 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
 		return -EOPNOTSUPP;
 	}
 
-	if (!tc_can_offload_extack(ns->netdev, cls_bpf->common.extack))
+	if (!tc_cls_can_offload_and_chain0(ns->netdev, &cls_bpf->common))
 		return -EOPNOTSUPP;
 
 	if (cls_bpf->common.protocol != htons(ETH_P_ALL)) {
@@ -144,9 +144,6 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
 		return -EOPNOTSUPP;
 	}
 
-	if (cls_bpf->common.chain_index)
-		return -EOPNOTSUPP;
-
 	if (!ns->bpf_tc_accept) {
 		NSIM_EA(cls_bpf->common.extack,
 			"netdevsim configured to reject BPF TC offload");
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 03/11] nfp: use tc_cls_can_offload_and_chain0()
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 01/11] pkt_cls: add new tc cls helper to check offload flag and chain index Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 02/11] netdevsim: use tc_cls_can_offload_and_chain0() Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 04/11] cxgb4: " Jakub Kicinski
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski

Make use of tc_cls_can_offload_and_chain0() to set extack msg in case
ethtool tc offload flag is not set or chain unsupported.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/main.c       | 4 +---
 drivers/net/ethernet/netronome/nfp/flower/offload.c | 7 +++----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index b3206855535a..322027792fe8 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -130,7 +130,7 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
 				   "only offload of BPF classifiers supported");
 		return -EOPNOTSUPP;
 	}
-	if (!tc_can_offload_extack(nn->dp.netdev, cls_bpf->common.extack))
+	if (!tc_cls_can_offload_and_chain0(nn->dp.netdev, &cls_bpf->common))
 		return -EOPNOTSUPP;
 	if (!nfp_net_ebpf_capable(nn)) {
 		NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
@@ -142,8 +142,6 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
 				   "only ETH_P_ALL supported as filter protocol");
 		return -EOPNOTSUPP;
 	}
-	if (cls_bpf->common.chain_index)
-		return -EOPNOTSUPP;
 
 	/* Only support TC direct action */
 	if (!cls_bpf->exts_integrated ||
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 837134a9137c..08c4c6dc5f7f 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -483,8 +483,7 @@ static int
 nfp_flower_repr_offload(struct nfp_app *app, struct net_device *netdev,
 			struct tc_cls_flower_offload *flower, bool egress)
 {
-	if (!eth_proto_is_802_3(flower->common.protocol) ||
-	    flower->common.chain_index)
+	if (!eth_proto_is_802_3(flower->common.protocol))
 		return -EOPNOTSUPP;
 
 	switch (flower->command) {
@@ -504,7 +503,7 @@ int nfp_flower_setup_tc_egress_cb(enum tc_setup_type type, void *type_data,
 {
 	struct nfp_repr *repr = cb_priv;
 
-	if (!tc_can_offload(repr->netdev))
+	if (!tc_cls_can_offload_and_chain0(repr->netdev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
@@ -521,7 +520,7 @@ static int nfp_flower_setup_tc_block_cb(enum tc_setup_type type,
 {
 	struct nfp_repr *repr = cb_priv;
 
-	if (!tc_can_offload(repr->netdev))
+	if (!tc_cls_can_offload_and_chain0(repr->netdev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 04/11] cxgb4: use tc_cls_can_offload_and_chain0()
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (2 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 03/11] nfp: " Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 05/11] mlx5: " Jakub Kicinski
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski, Ganesh Goudar

Make use of tc_cls_can_offload_and_chain0() to set extack msg in case
ethtool tc offload flag is not set or chain unsupported.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
CC: Ganesh Goudar <ganeshgr@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index f0fd2eba30c2..1e3cd8abc56d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -2928,9 +2928,6 @@ static int cxgb_set_tx_maxrate(struct net_device *dev, int index, u32 rate)
 static int cxgb_setup_tc_flower(struct net_device *dev,
 				struct tc_cls_flower_offload *cls_flower)
 {
-	if (cls_flower->common.chain_index)
-		return -EOPNOTSUPP;
-
 	switch (cls_flower->command) {
 	case TC_CLSFLOWER_REPLACE:
 		return cxgb4_tc_flower_replace(dev, cls_flower);
@@ -2946,9 +2943,6 @@ static int cxgb_setup_tc_flower(struct net_device *dev,
 static int cxgb_setup_tc_cls_u32(struct net_device *dev,
 				 struct tc_cls_u32_offload *cls_u32)
 {
-	if (cls_u32->common.chain_index)
-		return -EOPNOTSUPP;
-
 	switch (cls_u32->command) {
 	case TC_CLSU32_NEW_KNODE:
 	case TC_CLSU32_REPLACE_KNODE:
@@ -2974,7 +2968,7 @@ static int cxgb_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
 		return -EINVAL;
 	}
 
-	if (!tc_can_offload(dev))
+	if (!tc_cls_can_offload_and_chain0(dev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 05/11] mlx5: use tc_cls_can_offload_and_chain0()
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (3 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 04/11] cxgb4: " Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 06/11] bnxt: " Jakub Kicinski
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem
  Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski,
	Saeed Mahameed, Or Gerlitz

Make use of tc_cls_can_offload_and_chain0() to set extack msg in case
ethtool tc offload flag is not set or chain unsupported.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Acked-by: Or Gerlitz <ogerlitz@mellanox.com>
---
CC: Saeed Mahameed <saeedm@mellanox.com>
CC: Or Gerlitz <ogerlitz@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 5 +----
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c  | 5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 8530c770c873..47bab842c5ee 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2944,9 +2944,6 @@ static int mlx5e_setup_tc_mqprio(struct net_device *netdev,
 static int mlx5e_setup_tc_cls_flower(struct mlx5e_priv *priv,
 				     struct tc_cls_flower_offload *cls_flower)
 {
-	if (cls_flower->common.chain_index)
-		return -EOPNOTSUPP;
-
 	switch (cls_flower->command) {
 	case TC_CLSFLOWER_REPLACE:
 		return mlx5e_configure_flower(priv, cls_flower);
@@ -2964,7 +2961,7 @@ int mlx5e_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
 {
 	struct mlx5e_priv *priv = cb_priv;
 
-	if (!tc_can_offload(priv->netdev))
+	if (!tc_cls_can_offload_and_chain0(priv->netdev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 10fa6a18fcf9..363d8dcb7f17 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -719,9 +719,6 @@ static int
 mlx5e_rep_setup_tc_cls_flower(struct mlx5e_priv *priv,
 			      struct tc_cls_flower_offload *cls_flower)
 {
-	if (cls_flower->common.chain_index)
-		return -EOPNOTSUPP;
-
 	switch (cls_flower->command) {
 	case TC_CLSFLOWER_REPLACE:
 		return mlx5e_configure_flower(priv, cls_flower);
@@ -739,7 +736,7 @@ static int mlx5e_rep_setup_tc_cb(enum tc_setup_type type, void *type_data,
 {
 	struct mlx5e_priv *priv = cb_priv;
 
-	if (!tc_can_offload(priv->netdev))
+	if (!tc_cls_can_offload_and_chain0(priv->netdev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 06/11] bnxt: use tc_cls_can_offload_and_chain0()
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (4 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 05/11] mlx5: " Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 07/11] ixgbe: " Jakub Kicinski
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski, Michael Chan

Make use of tc_cls_can_offload_and_chain0() to set extack msg in case
ethtool tc offload flag is not set or chain unsupported.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
CC: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 3 ++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c  | 3 ---
 drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 3 ++-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 6b7e99675571..4b001d2050c2 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7778,7 +7778,8 @@ static int bnxt_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
 {
 	struct bnxt *bp = cb_priv;
 
-	if (!bnxt_tc_flower_enabled(bp) || !tc_can_offload(bp->dev))
+	if (!bnxt_tc_flower_enabled(bp) ||
+	    !tc_cls_can_offload_and_chain0(bp->dev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index 2ece1645f55d..fbe6e208e17b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -1474,9 +1474,6 @@ int bnxt_tc_setup_flower(struct bnxt *bp, u16 src_fid,
 {
 	int rc = 0;
 
-	if (cls_flower->common.chain_index)
-		return -EOPNOTSUPP;
-
 	switch (cls_flower->command) {
 	case TC_CLSFLOWER_REPLACE:
 		rc = bnxt_tc_add_flow(bp, src_fid, cls_flower);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
index 2ca11be64182..26290403f38f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
@@ -124,7 +124,8 @@ static int bnxt_vf_rep_setup_tc_block_cb(enum tc_setup_type type,
 	struct bnxt *bp = vf_rep->bp;
 	int vf_fid = bp->pf.vf[vf_rep->vf_idx].fw_fid;
 
-	if (!bnxt_tc_flower_enabled(vf_rep->bp) || !tc_can_offload(bp->dev))
+	if (!bnxt_tc_flower_enabled(vf_rep->bp) ||
+	    !tc_cls_can_offload_and_chain0(bp->dev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 07/11] ixgbe: use tc_cls_can_offload_and_chain0()
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (5 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 06/11] bnxt: " Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 08/11] i40e: " Jakub Kicinski
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski, Jeff Kirsher

Make use of tc_cls_can_offload_and_chain0() to set extack msg in case
ethtool tc offload flag is not set or chain unsupported.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 722cc3153a99..bbb622f15a77 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -9303,9 +9303,6 @@ static int ixgbe_configure_clsu32(struct ixgbe_adapter *adapter,
 static int ixgbe_setup_tc_cls_u32(struct ixgbe_adapter *adapter,
 				  struct tc_cls_u32_offload *cls_u32)
 {
-	if (cls_u32->common.chain_index)
-		return -EOPNOTSUPP;
-
 	switch (cls_u32->command) {
 	case TC_CLSU32_NEW_KNODE:
 	case TC_CLSU32_REPLACE_KNODE:
@@ -9327,7 +9324,7 @@ static int ixgbe_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
 {
 	struct ixgbe_adapter *adapter = cb_priv;
 
-	if (!tc_can_offload(adapter->netdev))
+	if (!tc_cls_can_offload_and_chain0(adapter->netdev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 08/11] i40e: use tc_cls_can_offload_and_chain0()
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (6 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 07/11] ixgbe: " Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 09/11] mlxsw: " Jakub Kicinski
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski, Jeff Kirsher

Make use of tc_cls_can_offload_and_chain0() to set extack msg in case
ethtool tc offload flag is not set or chain unsupported.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index a222d691958d..2703a92f3778 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -7437,11 +7437,6 @@ static int i40e_setup_tc_cls_flower(struct i40e_netdev_priv *np,
 {
 	struct i40e_vsi *vsi = np->vsi;
 
-	if (!tc_can_offload(vsi->netdev))
-		return -EOPNOTSUPP;
-	if (cls_flower->common.chain_index)
-		return -EOPNOTSUPP;
-
 	switch (cls_flower->command) {
 	case TC_CLSFLOWER_REPLACE:
 		return i40e_configure_clsflower(vsi, cls_flower);
@@ -7459,6 +7454,9 @@ static int i40e_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
 {
 	struct i40e_netdev_priv *np = cb_priv;
 
+	if (!tc_cls_can_offload_and_chain0(np->vsi->netdev, type_data))
+		return -EOPNOTSUPP;
+
 	switch (type) {
 	case TC_SETUP_CLSFLOWER:
 		return i40e_setup_tc_cls_flower(np, type_data);
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 09/11] mlxsw: use tc_cls_can_offload_and_chain0()
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (7 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 08/11] i40e: " Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 10/11] selftests/bpf: check for spurious extacks from the driver Jakub Kicinski
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem
  Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski, Jiri Pirko,
	Ido Schimmel

Make use of tc_cls_can_offload_and_chain0() to set extack msg in case
ethtool tc offload flag is not set or chain unsupported.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
CC: Jiri Pirko <jiri@mellanox.com>
CC: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 833cd0a96fd9..3dcc58d61506 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1738,9 +1738,6 @@ static int mlxsw_sp_setup_tc_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
 					  struct tc_cls_matchall_offload *f,
 					  bool ingress)
 {
-	if (f->common.chain_index)
-		return -EOPNOTSUPP;
-
 	switch (f->command) {
 	case TC_CLSMATCHALL_REPLACE:
 		return mlxsw_sp_port_add_cls_matchall(mlxsw_sp_port, f,
@@ -1780,7 +1777,8 @@ static int mlxsw_sp_setup_tc_block_cb_matchall(enum tc_setup_type type,
 
 	switch (type) {
 	case TC_SETUP_CLSMATCHALL:
-		if (!tc_can_offload(mlxsw_sp_port->dev))
+		if (!tc_cls_can_offload_and_chain0(mlxsw_sp_port->dev,
+						   type_data))
 			return -EOPNOTSUPP;
 
 		return mlxsw_sp_setup_tc_cls_matchall(mlxsw_sp_port, type_data,
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 10/11] selftests/bpf: check for spurious extacks from the driver
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (8 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 09/11] mlxsw: " Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-25 22:00 ` [PATCH net-next v2 11/11] selftests/bpf: check for chain-non-0 extack message Jakub Kicinski
  2018-01-26  2:23 ` [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski

Drivers should not report errors when offload is not forced.
Check stdout and stderr for familiar messages when with no
skip flags and with skip_hw.  Check for add, replace, and
destroy.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index ae3eea3ab820..49f5ceeabfa6 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -543,6 +543,10 @@ netns = [] # net namespaces to be removed
 def check_extack_nsim(output, reference, args):
     check_extack(output, "Error: netdevsim: " + reference, args)
 
+def check_no_extack(res, needle):
+    fail((res[1] + res[2]).count(needle) or (res[1] + res[2]).count("Warning:"),
+         "Found '%s' in command output, leaky extack?" % (needle))
+
 def check_verifier_log(output, reference):
     lines = output.split("\n")
     for l in reversed(lines):
@@ -550,6 +554,18 @@ netns = [] # net namespaces to be removed
             return
     fail(True, "Missing or incorrect message from netdevsim in verifier log")
 
+def test_spurios_extack(sim, obj, skip_hw, needle):
+    res = sim.cls_bpf_add_filter(obj, prio=1, handle=1, skip_hw=skip_hw,
+                                 include_stderr=True)
+    check_no_extack(res, needle)
+    res = sim.cls_bpf_add_filter(obj, op="replace", prio=1, handle=1,
+                                 skip_hw=skip_hw, include_stderr=True)
+    check_no_extack(res, needle)
+    res = sim.cls_filter_op(op="delete", prio=1, handle=1, cls="bpf",
+                            include_stderr=True)
+    check_no_extack(res, needle)
+
+
 # Parse command line
 parser = argparse.ArgumentParser()
 parser.add_argument("--log", help="output verbose log to given file")
@@ -687,6 +703,17 @@ netns = []
                  (j))
         sim.cls_filter_op(op="delete", prio=1, handle=1, cls="bpf")
 
+    start_test("Test spurious extack from the driver...")
+    test_spurios_extack(sim, obj, False, "netdevsim")
+    test_spurios_extack(sim, obj, True, "netdevsim")
+
+    sim.set_ethtool_tc_offloads(False)
+
+    test_spurios_extack(sim, obj, False, "TC offload is disabled")
+    test_spurios_extack(sim, obj, True, "TC offload is disabled")
+
+    sim.set_ethtool_tc_offloads(True)
+
     sim.tc_flush_filters()
 
     start_test("Test TC offloads work...")
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2 11/11] selftests/bpf: check for chain-non-0 extack message
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (9 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 10/11] selftests/bpf: check for spurious extacks from the driver Jakub Kicinski
@ 2018-01-25 22:00 ` Jakub Kicinski
  2018-01-26  2:23 ` [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2018-01-25 22:00 UTC (permalink / raw)
  To: davem; +Cc: jiri, dsahern, netdev, oss-drivers, Jakub Kicinski

Make sure netdevsim doesn't allow offload of chains other than 0,
and that it reports the expected extack message.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/testing/selftests/bpf/test_offload.py | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index 49f5ceeabfa6..e78aad0a68bb 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -430,13 +430,15 @@ netns = [] # net namespaces to be removed
         return filters
 
     def cls_filter_op(self, op, qdisc="ingress", prio=None, handle=None,
-                      cls="", params="",
+                      chain=None, cls="", params="",
                       fail=True, include_stderr=False):
         spec = ""
         if prio is not None:
             spec += " prio %d" % (prio)
         if handle:
             spec += " handle %s" % (handle)
+        if chain is not None:
+            spec += " chain %d" % (chain)
 
         return tc("filter {op} dev {dev} {qdisc} {spec} {cls} {params}"\
                   .format(op=op, dev=self['ifname'], qdisc=qdisc, spec=spec,
@@ -444,7 +446,7 @@ netns = [] # net namespaces to be removed
                   fail=fail, include_stderr=include_stderr)
 
     def cls_bpf_add_filter(self, bpf, op="add", prio=None, handle=None,
-                           da=False, verbose=False,
+                           chain=None, da=False, verbose=False,
                            skip_sw=False, skip_hw=False,
                            fail=True, include_stderr=False):
         cls = "bpf " + bpf
@@ -460,7 +462,7 @@ netns = [] # net namespaces to be removed
             params += " skip_hw"
 
         return self.cls_filter_op(op=op, prio=prio, handle=handle, cls=cls,
-                                  params=params,
+                                  chain=chain, params=params,
                                   fail=fail, include_stderr=include_stderr)
 
     def set_ethtool_tc_offloads(self, enable, fail=True):
@@ -679,6 +681,14 @@ netns = []
                       args)
     sim.wait_for_flush()
 
+    start_test("Test non-0 chain offload...")
+    ret, _, err = sim.cls_bpf_add_filter(obj, chain=1, prio=1, handle=1,
+                                         skip_sw=True,
+                                         fail=False, include_stderr=True)
+    fail(ret == 0, "Offloaded a filter to chain other than 0")
+    check_extack(err, "Error: Driver supports only offload of chain 0.", args)
+    sim.tc_flush_filters()
+
     start_test("Test TC replace...")
     sim.cls_bpf_add_filter(obj, prio=1, handle=1)
     sim.cls_bpf_add_filter(obj, op="replace", prio=1, handle=1)
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers
  2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
                   ` (10 preceding siblings ...)
  2018-01-25 22:00 ` [PATCH net-next v2 11/11] selftests/bpf: check for chain-non-0 extack message Jakub Kicinski
@ 2018-01-26  2:23 ` David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2018-01-26  2:23 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: jiri, dsahern, netdev, oss-drivers

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Thu, 25 Jan 2018 14:00:42 -0800

> This set makes all drivers use a new tc_cls_can_offload_and_chain0()
> helper which will set extack in case TC hw offload flag is disabled.
> 
> I chose to keep the new helper which also looks at the chain but
> renamed it more appropriately.  The rationale being that most drivers
> don't accept chains other than 0 and since we have to pass extack
> to the helper we can as well pass the entire struct tc_cls_common_offload
> and perform the most common checks.
> 
> This code makes the assumption that type_data in the callback can
> be interpreted as struct tc_cls_common_offload, i.e. the real offload
> structure has common part as the first member.  This allows us to
> make the check once for all classifier types if driver supports
> more than one.
> 
> v1:
>  - drop the type validation in nfp and netdevsim.
> v2:
>  - reorder checks in patch 1;
>  - split other changes from patch 1;
>  - add the i40e patch in;
>  - add one more test case - for chain 0 extack.

Series applied, thanks Jakub.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-01-26  2:23 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 22:00 [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 01/11] pkt_cls: add new tc cls helper to check offload flag and chain index Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 02/11] netdevsim: use tc_cls_can_offload_and_chain0() Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 03/11] nfp: " Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 04/11] cxgb4: " Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 05/11] mlx5: " Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 06/11] bnxt: " Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 07/11] ixgbe: " Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 08/11] i40e: " Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 09/11] mlxsw: " Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 10/11] selftests/bpf: check for spurious extacks from the driver Jakub Kicinski
2018-01-25 22:00 ` [PATCH net-next v2 11/11] selftests/bpf: check for chain-non-0 extack message Jakub Kicinski
2018-01-26  2:23 ` [PATCH net-next v2 00/11] use tc_cls_can_offload_and_chain0() throughout the drivers David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.