[PATCH bpf-next v2 00/11] net: sched: add extack support for cls offload

[PATCH bpf-next v2 00/11] net: sched: add extack support for cls offload

[Jakub Kicinski]

Hi!

This series adds extack to cls offloads, as such it could arguably be
targeted at net-next.  Unfortunately, git am is not able to deal cleanly
with minor conflicts on the nfp patches..  Since the series is really
about cls_bpf I hope it's OK if it went via the bpf-next tree.

Quentin says:

This series tries to improve user experience when eBPF hardware offload
hits error paths at load time. In particular, it introduces netlink
extended ack support in the nfp driver.

To that aim, transmission of the pointer to the extack object is piped
through the `change()` operation of the existing classifiers (patch 1 to
6). Then it is used for TC offload in the nfp driver (patch 8) and in
netdevsim (patch 9, selftest in patch 10). Patch 7 adds a helper to handle
extack messages in the core when TC offload is disabled on the net device.

For completeness extack is propagated for classifiers other than cls_bpf,
but it's up to the drivers to make use of it.

v2:
 - auto-skip the extack message validation in patch 11 (Dave A).

Quentin Monnet (11):
  net: sched: add extack support to change() classifier operation
  net: sched: prepare extack support for offload via
    tc_cls_common_offload
  net: sched: cls_flower: propagate extack support for filter offload
  net: sched: cls_matchall: propagate extack support for filter offload
  net: sched: cls_u32: propagate extack support for filter offload
  net: sched: cls_bpf: plumb extack support in filter for hardware
    offload
  net: sched: create tc_can_offload_extack() wrapper
  nfp: bpf: plumb extack into functions related to XDP offload
  nfp: bpf: use extack support to improve debugging
  netdevsim: add extack support for TC eBPF offload
  selftests/bpf: add checks on extack messages for eBPF hw offload tests

 drivers/net/ethernet/netronome/nfp/bpf/main.c      |  35 +++++--
 drivers/net/ethernet/netronome/nfp/bpf/main.h      |   2 +-
 drivers/net/ethernet/netronome/nfp/bpf/offload.c   |  24 +++--
 drivers/net/ethernet/netronome/nfp/nfp_app.h       |   9 +-
 .../net/ethernet/netronome/nfp/nfp_net_common.c    |   2 +-
 drivers/net/netdevsim/bpf.c                        |  35 +++++--
 include/net/pkt_cls.h                              |  16 +++-
 include/net/sch_generic.h                          |   3 +-
 net/sched/cls_api.c                                |   3 +-
 net/sched/cls_basic.c                              |   3 +-
 net/sched/cls_bpf.c                                |  20 ++--
 net/sched/cls_cgroup.c                             |   3 +-
 net/sched/cls_flow.c                               |   2 +-
 net/sched/cls_flower.c                             |  14 +--
 net/sched/cls_fw.c                                 |   2 +-
 net/sched/cls_matchall.c                           |  12 ++-
 net/sched/cls_route.c                              |   3 +-
 net/sched/cls_rsvp.h                               |   2 +-
 net/sched/cls_tcindex.c                            |   3 +-
 net/sched/cls_u32.c                                |  21 +++--
 tools/testing/selftests/bpf/test_offload.py        | 104 +++++++++++++++------
 21 files changed, 221 insertions(+), 97 deletions(-)

-- 
2.15.1

[PATCH bpf-next v2 01/11] net: sched: add extack support to change() classifier operation

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Add an extra argument to `->change()` operation for passing a pointer to
a struct netlink_ext_ack. Update the operation for all classifiers
accordingly. Extack is not used at this point.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/sch_generic.h | 3 ++-
 net/sched/cls_api.c       | 3 ++-
 net/sched/cls_basic.c     | 3 ++-
 net/sched/cls_bpf.c       | 2 +-
 net/sched/cls_cgroup.c    | 3 ++-
 net/sched/cls_flow.c      | 2 +-
 net/sched/cls_flower.c    | 2 +-
 net/sched/cls_fw.c        | 2 +-
 net/sched/cls_matchall.c  | 2 +-
 net/sched/cls_route.c     | 3 ++-
 net/sched/cls_rsvp.h      | 2 +-
 net/sched/cls_tcindex.c   | 3 ++-
 net/sched/cls_u32.c       | 3 ++-
 13 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index ac029d5d88e4..5e77f2639c67 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -232,7 +232,8 @@ struct tcf_proto_ops {
 	int			(*change)(struct net *net, struct sk_buff *,
 					struct tcf_proto*, unsigned long,
 					u32 handle, struct nlattr **,
-					void **, bool);
+					void **, bool,
+					struct netlink_ext_ack *);
 	int			(*delete)(struct tcf_proto*, void *, bool*);
 	void			(*walk)(struct tcf_proto*, struct tcf_walker *arg);
 	void			(*bind_class)(void *, u32, unsigned long);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 6708b6953bfa..0460cc22d48c 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -912,7 +912,8 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	}
 
 	err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh,
-			      n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE);
+			      n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE,
+			      extack);
 	if (err == 0) {
 		if (tp_created)
 			tcf_chain_tp_insert(chain, &chain_info, tp);
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 5f169ded347e..2cc38cd71938 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -175,7 +175,8 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
 
 static int basic_change(struct net *net, struct sk_buff *in_skb,
 			struct tcf_proto *tp, unsigned long base, u32 handle,
-			struct nlattr **tca, void **arg, bool ovr)
+			struct nlattr **tca, void **arg, bool ovr,
+			struct netlink_ext_ack *extack)
 {
 	int err;
 	struct basic_head *head = rtnl_dereference(tp->root);
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 8d78e7f4ecc3..fcb831b3917e 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -449,7 +449,7 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
 static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
 			  struct tcf_proto *tp, unsigned long base,
 			  u32 handle, struct nlattr **tca,
-			  void **arg, bool ovr)
+			  void **arg, bool ovr, struct netlink_ext_ack *extack)
 {
 	struct cls_bpf_head *head = rtnl_dereference(tp->root);
 	struct cls_bpf_prog *oldprog = *arg;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 309d5899265f..b74af0b55820 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -91,7 +91,8 @@ static void cls_cgroup_destroy_rcu(struct rcu_head *root)
 static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 			     struct tcf_proto *tp, unsigned long base,
 			     u32 handle, struct nlattr **tca,
-			     void **arg, bool ovr)
+			     void **arg, bool ovr,
+			     struct netlink_ext_ack *extack)
 {
 	struct nlattr *tb[TCA_CGROUP_MAX + 1];
 	struct cls_cgroup_head *head = rtnl_dereference(tp->root);
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 25c2a888e1f0..e944f01d5394 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -401,7 +401,7 @@ static void flow_destroy_filter(struct rcu_head *head)
 static int flow_change(struct net *net, struct sk_buff *in_skb,
 		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle, struct nlattr **tca,
-		       void **arg, bool ovr)
+		       void **arg, bool ovr, struct netlink_ext_ack *extack)
 {
 	struct flow_head *head = rtnl_dereference(tp->root);
 	struct flow_filter *fold, *fnew;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 6132a7317efa..998ee4faf934 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -852,7 +852,7 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
 static int fl_change(struct net *net, struct sk_buff *in_skb,
 		     struct tcf_proto *tp, unsigned long base,
 		     u32 handle, struct nlattr **tca,
-		     void **arg, bool ovr)
+		     void **arg, bool ovr, struct netlink_ext_ack *extack)
 {
 	struct cls_fl_head *head = rtnl_dereference(tp->root);
 	struct cls_fl_filter *fold = *arg;
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 20f0de1a960a..72784491ce20 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -257,7 +257,7 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp,
 static int fw_change(struct net *net, struct sk_buff *in_skb,
 		     struct tcf_proto *tp, unsigned long base,
 		     u32 handle, struct nlattr **tca, void **arg,
-		     bool ovr)
+		     bool ovr, struct netlink_ext_ack *extack)
 {
 	struct fw_head *head = rtnl_dereference(tp->root);
 	struct fw_filter *f = *arg;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 66d4e0099158..dc3c57116bbd 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -159,7 +159,7 @@ static int mall_set_parms(struct net *net, struct tcf_proto *tp,
 static int mall_change(struct net *net, struct sk_buff *in_skb,
 		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle, struct nlattr **tca,
-		       void **arg, bool ovr)
+		       void **arg, bool ovr, struct netlink_ext_ack *extack)
 {
 	struct cls_mall_head *head = rtnl_dereference(tp->root);
 	struct nlattr *tb[TCA_MATCHALL_MAX + 1];
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index ac9a5b8825b9..bf305db65de0 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -471,7 +471,8 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 
 static int route4_change(struct net *net, struct sk_buff *in_skb,
 			 struct tcf_proto *tp, unsigned long base, u32 handle,
-			 struct nlattr **tca, void **arg, bool ovr)
+			 struct nlattr **tca, void **arg, bool ovr,
+			 struct netlink_ext_ack *extack)
 {
 	struct route4_head *head = rtnl_dereference(tp->root);
 	struct route4_filter __rcu **fp;
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index cf325625c99d..d1f67529c01d 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -486,7 +486,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
 		       struct tcf_proto *tp, unsigned long base,
 		       u32 handle,
 		       struct nlattr **tca,
-		       void **arg, bool ovr)
+		       void **arg, bool ovr, struct netlink_ext_ack *extack)
 {
 	struct rsvp_head *data = rtnl_dereference(tp->root);
 	struct rsvp_filter *f, *nfp;
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 67467ae24c97..0ec84cf2d6b7 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -520,7 +520,8 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 static int
 tcindex_change(struct net *net, struct sk_buff *in_skb,
 	       struct tcf_proto *tp, unsigned long base, u32 handle,
-	       struct nlattr **tca, void **arg, bool ovr)
+	       struct nlattr **tca, void **arg, bool ovr,
+	       struct netlink_ext_ack *extack)
 {
 	struct nlattr *opt = tca[TCA_OPTIONS];
 	struct nlattr *tb[TCA_TCINDEX_MAX + 1];
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 507859cdd1cb..3ef5c32741c1 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -892,7 +892,8 @@ static struct tc_u_knode *u32_init_knode(struct tcf_proto *tp,
 
 static int u32_change(struct net *net, struct sk_buff *in_skb,
 		      struct tcf_proto *tp, unsigned long base, u32 handle,
-		      struct nlattr **tca, void **arg, bool ovr)
+		      struct nlattr **tca, void **arg, bool ovr,
+		      struct netlink_ext_ack *extack)
 {
 	struct tc_u_common *tp_c = tp->data;
 	struct tc_u_hnode *ht;
-- 
2.15.1

[PATCH bpf-next v2 02/11] net: sched: prepare extack support for offload via tc_cls_common_offload

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Prepare for extack support for hardware offload of classifiers. In order
to achieve this, a pointer to a struct netlink_ext_ack is added to the
struct tc_cls_common_offload that is passed to the callback for setting
up the classifier. Function tc_cls_common_offload_init() is updated to
support initialization of this new attribute.

Extack plumbing is not complete yet.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/pkt_cls.h    | 5 ++++-
 net/sched/cls_bpf.c      | 4 ++--
 net/sched/cls_flower.c   | 6 +++---
 net/sched/cls_matchall.c | 4 ++--
 net/sched/cls_u32.c      | 8 ++++----
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 0d1343cba84c..c88c61234cb3 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -590,15 +590,18 @@ struct tc_cls_common_offload {
 	u32 chain_index;
 	__be16 protocol;
 	u32 prio;
+	struct netlink_ext_ack *extack;
 };
 
 static inline void
 tc_cls_common_offload_init(struct tc_cls_common_offload *cls_common,
-			   const struct tcf_proto *tp)
+			   const struct tcf_proto *tp,
+			   struct netlink_ext_ack *extack)
 {
 	cls_common->chain_index = tp->chain->index;
 	cls_common->protocol = tp->protocol;
 	cls_common->prio = tp->prio;
+	cls_common->extack = extack;
 }
 
 struct tc_cls_u32_knode {
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index fcb831b3917e..b8729124d209 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -158,7 +158,7 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	skip_sw = prog && tc_skip_sw(prog->gen_flags);
 	obj = prog ?: oldprog;
 
-	tc_cls_common_offload_init(&cls_bpf.common, tp);
+	tc_cls_common_offload_init(&cls_bpf.common, tp, NULL);
 	cls_bpf.command = TC_CLSBPF_OFFLOAD;
 	cls_bpf.exts = &obj->exts;
 	cls_bpf.prog = prog ? prog->filter : NULL;
@@ -215,7 +215,7 @@ static void cls_bpf_offload_update_stats(struct tcf_proto *tp,
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_bpf_offload cls_bpf = {};
 
-	tc_cls_common_offload_init(&cls_bpf.common, tp);
+	tc_cls_common_offload_init(&cls_bpf.common, tp, NULL);
 	cls_bpf.command = TC_CLSBPF_STATS;
 	cls_bpf.exts = &prog->exts;
 	cls_bpf.prog = prog->filter;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 998ee4faf934..f1640a98a3d2 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -223,7 +223,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init(&cls_flower.common, tp);
+	tc_cls_common_offload_init(&cls_flower.common, tp, NULL);
 	cls_flower.command = TC_CLSFLOWER_DESTROY;
 	cls_flower.cookie = (unsigned long) f;
 
@@ -241,7 +241,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp,
 	bool skip_sw = tc_skip_sw(f->flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_flower.common, tp);
+	tc_cls_common_offload_init(&cls_flower.common, tp, NULL);
 	cls_flower.command = TC_CLSFLOWER_REPLACE;
 	cls_flower.cookie = (unsigned long) f;
 	cls_flower.dissector = dissector;
@@ -270,7 +270,7 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f)
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init(&cls_flower.common, tp);
+	tc_cls_common_offload_init(&cls_flower.common, tp, NULL);
 	cls_flower.command = TC_CLSFLOWER_STATS;
 	cls_flower.cookie = (unsigned long) f;
 	cls_flower.exts = &f->exts;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index dc3c57116bbd..24bec37e9747 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -76,7 +76,7 @@ static void mall_destroy_hw_filter(struct tcf_proto *tp,
 	struct tc_cls_matchall_offload cls_mall = {};
 	struct tcf_block *block = tp->chain->block;
 
-	tc_cls_common_offload_init(&cls_mall.common, tp);
+	tc_cls_common_offload_init(&cls_mall.common, tp, NULL);
 	cls_mall.command = TC_CLSMATCHALL_DESTROY;
 	cls_mall.cookie = cookie;
 
@@ -92,7 +92,7 @@ static int mall_replace_hw_filter(struct tcf_proto *tp,
 	bool skip_sw = tc_skip_sw(head->flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_mall.common, tp);
+	tc_cls_common_offload_init(&cls_mall.common, tp, NULL);
 	cls_mall.command = TC_CLSMATCHALL_REPLACE;
 	cls_mall.exts = &head->exts;
 	cls_mall.cookie = cookie;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 3ef5c32741c1..8127b15d8d50 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -491,7 +491,7 @@ static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h)
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 
-	tc_cls_common_offload_init(&cls_u32.common, tp);
+	tc_cls_common_offload_init(&cls_u32.common, tp, NULL);
 	cls_u32.command = TC_CLSU32_DELETE_HNODE;
 	cls_u32.hnode.divisor = h->divisor;
 	cls_u32.hnode.handle = h->handle;
@@ -509,7 +509,7 @@ static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
 	bool offloaded = false;
 	int err;
 
-	tc_cls_common_offload_init(&cls_u32.common, tp);
+	tc_cls_common_offload_init(&cls_u32.common, tp, NULL);
 	cls_u32.command = TC_CLSU32_NEW_HNODE;
 	cls_u32.hnode.divisor = h->divisor;
 	cls_u32.hnode.handle = h->handle;
@@ -534,7 +534,7 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, u32 handle)
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 
-	tc_cls_common_offload_init(&cls_u32.common, tp);
+	tc_cls_common_offload_init(&cls_u32.common, tp, NULL);
 	cls_u32.command = TC_CLSU32_DELETE_KNODE;
 	cls_u32.knode.handle = handle;
 
@@ -549,7 +549,7 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
 	bool skip_sw = tc_skip_sw(flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_u32.common, tp);
+	tc_cls_common_offload_init(&cls_u32.common, tp, NULL);
 	cls_u32.command = TC_CLSU32_REPLACE_KNODE;
 	cls_u32.knode.handle = n->handle;
 	cls_u32.knode.fshift = n->fshift;
-- 
2.15.1

[PATCH bpf-next v2 03/11] net: sched: cls_flower: propagate extack support for filter offload

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Propagate the extack pointer from the `->change()` classifier operation
to the function used for filter replacement in cls_flower, and feed it
to tc_cls_common_offload_init(). This makes it possible to use netlink
extack messages in the future at replacement time for this filter,
although it is not used at this point.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/sched/cls_flower.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index f1640a98a3d2..fe7d96d12435 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -234,14 +234,15 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
 static int fl_hw_replace_filter(struct tcf_proto *tp,
 				struct flow_dissector *dissector,
 				struct fl_flow_key *mask,
-				struct cls_fl_filter *f)
+				struct cls_fl_filter *f,
+				struct netlink_ext_ack *extack)
 {
 	struct tc_cls_flower_offload cls_flower = {};
 	struct tcf_block *block = tp->chain->block;
 	bool skip_sw = tc_skip_sw(f->flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_flower.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_flower.common, tp, extack);
 	cls_flower.command = TC_CLSFLOWER_REPLACE;
 	cls_flower.cookie = (unsigned long) f;
 	cls_flower.dissector = dissector;
@@ -939,7 +940,8 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 		err = fl_hw_replace_filter(tp,
 					   &head->dissector,
 					   &mask.key,
-					   fnew);
+					   fnew,
+					   extack);
 		if (err)
 			goto errout_idr;
 	}
-- 
2.15.1

[PATCH bpf-next v2 04/11] net: sched: cls_matchall: propagate extack support for filter offload

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Propagate the extack pointer from the `->change()` classifier operation
to the function used for filter replacement in cls_matchall, and feed it
to tc_cls_common_offload_init(). This makes it possible to use netlink
extack messages in the future at replacement time for this filter,
although it is not used at this point.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/sched/cls_matchall.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 24bec37e9747..fe6b673db5c6 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -85,14 +85,15 @@ static void mall_destroy_hw_filter(struct tcf_proto *tp,
 
 static int mall_replace_hw_filter(struct tcf_proto *tp,
 				  struct cls_mall_head *head,
-				  unsigned long cookie)
+				  unsigned long cookie,
+				  struct netlink_ext_ack *extack)
 {
 	struct tc_cls_matchall_offload cls_mall = {};
 	struct tcf_block *block = tp->chain->block;
 	bool skip_sw = tc_skip_sw(head->flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_mall.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_mall.common, tp, extack);
 	cls_mall.command = TC_CLSMATCHALL_REPLACE;
 	cls_mall.exts = &head->exts;
 	cls_mall.cookie = cookie;
@@ -202,7 +203,8 @@ static int mall_change(struct net *net, struct sk_buff *in_skb,
 		goto err_set_parms;
 
 	if (!tc_skip_hw(new->flags)) {
-		err = mall_replace_hw_filter(tp, new, (unsigned long) new);
+		err = mall_replace_hw_filter(tp, new, (unsigned long)new,
+					     extack);
 		if (err)
 			goto err_replace_hw_filter;
 	}
-- 
2.15.1

[PATCH bpf-next v2 05/11] net: sched: cls_u32: propagate extack support for filter offload

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Propagate the extack pointer from the `->change()` classifier operation
to the function used for filter replacement in cls_u32, and feed it to
tc_cls_common_offload_init(). This makes it possible to use netlink
extack messages in the future at replacement time for this filter,
although it is not used at this point.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/sched/cls_u32.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 8127b15d8d50..ef1b746de80b 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -501,7 +501,7 @@ static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h)
 }
 
 static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
-				u32 flags)
+				u32 flags, struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
@@ -509,7 +509,7 @@ static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h,
 	bool offloaded = false;
 	int err;
 
-	tc_cls_common_offload_init(&cls_u32.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_u32.common, tp, extack);
 	cls_u32.command = TC_CLSU32_NEW_HNODE;
 	cls_u32.hnode.divisor = h->divisor;
 	cls_u32.hnode.handle = h->handle;
@@ -542,14 +542,14 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, u32 handle)
 }
 
 static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
-				u32 flags)
+				u32 flags, struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_u32_offload cls_u32 = {};
 	bool skip_sw = tc_skip_sw(flags);
 	int err;
 
-	tc_cls_common_offload_init(&cls_u32.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_u32.common, tp, extack);
 	cls_u32.command = TC_CLSU32_REPLACE_KNODE;
 	cls_u32.knode.handle = n->handle;
 	cls_u32.knode.fshift = n->fshift;
@@ -943,7 +943,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 			return err;
 		}
 
-		err = u32_replace_hw_knode(tp, new, flags);
+		err = u32_replace_hw_knode(tp, new, flags, extack);
 		if (err) {
 			u32_destroy_key(tp, new, false);
 			return err;
@@ -990,7 +990,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		ht->prio = tp->prio;
 		idr_init(&ht->handle_idr);
 
-		err = u32_replace_hw_hnode(tp, ht, flags);
+		err = u32_replace_hw_hnode(tp, ht, flags, extack);
 		if (err) {
 			idr_remove_ext(&tp_c->handle_idr, handle);
 			kfree(ht);
@@ -1088,7 +1088,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		struct tc_u_knode __rcu **ins;
 		struct tc_u_knode *pins;
 
-		err = u32_replace_hw_knode(tp, n, flags);
+		err = u32_replace_hw_knode(tp, n, flags, extack);
 		if (err)
 			goto errhw;
 
-- 
2.15.1

[PATCH bpf-next v2 06/11] net: sched: cls_bpf: plumb extack support in filter for hardware offload

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Pass the extack pointer obtained in the `->change()` filter operation to
cls_bpf_offload() and then to cls_bpf_offload_cmd(), where it can be
used to initialize the new field of tc_cls_common_offload passed to the
callback for offload. This makes it possible to use this extack pointer
in drivers offloading BPF programs in a future patch.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 net/sched/cls_bpf.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index b8729124d209..d15ef9ab7243 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -147,7 +147,8 @@ static bool cls_bpf_is_ebpf(const struct cls_bpf_prog *prog)
 }
 
 static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
-			       struct cls_bpf_prog *oldprog)
+			       struct cls_bpf_prog *oldprog,
+			       struct netlink_ext_ack *extack)
 {
 	struct tcf_block *block = tp->chain->block;
 	struct tc_cls_bpf_offload cls_bpf = {};
@@ -158,7 +159,7 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	skip_sw = prog && tc_skip_sw(prog->gen_flags);
 	obj = prog ?: oldprog;
 
-	tc_cls_common_offload_init(&cls_bpf.common, tp, NULL);
+	tc_cls_common_offload_init(&cls_bpf.common, tp, extack);
 	cls_bpf.command = TC_CLSBPF_OFFLOAD;
 	cls_bpf.exts = &obj->exts;
 	cls_bpf.prog = prog ? prog->filter : NULL;
@@ -170,7 +171,7 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSBPF, &cls_bpf, skip_sw);
 	if (prog) {
 		if (err < 0) {
-			cls_bpf_offload_cmd(tp, oldprog, prog);
+			cls_bpf_offload_cmd(tp, oldprog, prog, extack);
 			return err;
 		} else if (err > 0) {
 			prog->gen_flags |= TCA_CLS_FLAGS_IN_HW;
@@ -184,7 +185,8 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 }
 
 static int cls_bpf_offload(struct tcf_proto *tp, struct cls_bpf_prog *prog,
-			   struct cls_bpf_prog *oldprog)
+			   struct cls_bpf_prog *oldprog,
+			   struct netlink_ext_ack *extack)
 {
 	if (prog && oldprog && prog->gen_flags != oldprog->gen_flags)
 		return -EINVAL;
@@ -196,7 +198,7 @@ static int cls_bpf_offload(struct tcf_proto *tp, struct cls_bpf_prog *prog,
 	if (!prog && !oldprog)
 		return 0;
 
-	return cls_bpf_offload_cmd(tp, prog, oldprog);
+	return cls_bpf_offload_cmd(tp, prog, oldprog, extack);
 }
 
 static void cls_bpf_stop_offload(struct tcf_proto *tp,
@@ -204,7 +206,7 @@ static void cls_bpf_stop_offload(struct tcf_proto *tp,
 {
 	int err;
 
-	err = cls_bpf_offload_cmd(tp, NULL, prog);
+	err = cls_bpf_offload_cmd(tp, NULL, prog, NULL);
 	if (err)
 		pr_err("Stopping hardware offload failed: %d\n", err);
 }
@@ -501,7 +503,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
 	if (ret < 0)
 		goto errout_idr;
 
-	ret = cls_bpf_offload(tp, prog, oldprog);
+	ret = cls_bpf_offload(tp, prog, oldprog, extack);
 	if (ret)
 		goto errout_parms;
 
-- 
2.15.1

[PATCH bpf-next v2 07/11] net: sched: create tc_can_offload_extack() wrapper

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.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@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.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 c88c61234cb3..a3ad6a5a2d12 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -644,6 +644,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)
+{
+	bool can = tc_can_offload(dev);
+
+	if (!can)
+		NL_SET_ERR_MSG(extack, "TC offload is disabled on net device");
+
+	return can;
+}
+
 static inline bool tc_skip_hw(u32 flags)
 {
 	return (flags & TCA_CLS_FLAGS_SKIP_HW) ? true : false;
-- 
2.15.1

[PATCH bpf-next v2 08/11] nfp: bpf: plumb extack into functions related to XDP offload

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Pass a pointer to an extack object to nfp_app_xdp_offload() in order to
prepare for extack usage in the nfp driver. Next step will be to forward
this extack pointer to nfp_net_bpf_offload(), once this function is able
to use it for printing error messages.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/main.c       | 4 ++--
 drivers/net/ethernet/netronome/nfp/nfp_app.h        | 9 ++++++---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index 8823c8360047..e8816ab8fb63 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -54,7 +54,7 @@ static bool nfp_net_ebpf_capable(struct nfp_net *nn)
 
 static int
 nfp_bpf_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
-		    struct bpf_prog *prog)
+		    struct bpf_prog *prog, struct netlink_ext_ack *extack)
 {
 	bool running, xdp_running;
 	int ret;
@@ -73,7 +73,7 @@ nfp_bpf_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
 	ret = nfp_net_bpf_offload(nn, prog, running);
 	/* Stop offload if replace not possible */
 	if (ret && prog)
-		nfp_bpf_xdp_offload(app, nn, NULL);
+		nfp_bpf_xdp_offload(app, nn, NULL, extack);
 
 	nn->dp.bpf_offload_xdp = prog && !ret;
 	return ret;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
index 6a6eb02b516e..1229a34f8da5 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
@@ -43,6 +43,7 @@
 struct bpf_prog;
 struct net_device;
 struct netdev_bpf;
+struct netlink_ext_ack;
 struct pci_dev;
 struct sk_buff;
 struct sk_buff;
@@ -134,7 +135,8 @@ struct nfp_app_type {
 	int (*bpf)(struct nfp_app *app, struct nfp_net *nn,
 		   struct netdev_bpf *xdp);
 	int (*xdp_offload)(struct nfp_app *app, struct nfp_net *nn,
-			   struct bpf_prog *prog);
+			   struct bpf_prog *prog,
+			   struct netlink_ext_ack *extack);
 
 	int (*sriov_enable)(struct nfp_app *app, int num_vfs);
 	void (*sriov_disable)(struct nfp_app *app);
@@ -320,11 +322,12 @@ static inline int nfp_app_bpf(struct nfp_app *app, struct nfp_net *nn,
 }
 
 static inline int nfp_app_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
-				      struct bpf_prog *prog)
+				      struct bpf_prog *prog,
+				      struct netlink_ext_ack *extack)
 {
 	if (!app || !app->type->xdp_offload)
 		return -EOPNOTSUPP;
-	return app->type->xdp_offload(app, nn, prog);
+	return app->type->xdp_offload(app, nn, prog, extack);
 }
 
 static inline bool __nfp_app_ctrl_tx(struct nfp_app *app, struct sk_buff *skb)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 2b5cad3069a7..14f23e8d27fa 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3395,7 +3395,7 @@ nfp_net_xdp_setup(struct nfp_net *nn, struct bpf_prog *prog, u32 flags,
 	if (err)
 		return err;
 
-	err = nfp_app_xdp_offload(nn->app, nn, offload_prog);
+	err = nfp_app_xdp_offload(nn->app, nn, offload_prog, extack);
 	if (err && flags & XDP_FLAGS_HW_MODE)
 		return err;
 
-- 
2.15.1

[PATCH bpf-next v2 09/11] nfp: bpf: use extack support to improve debugging

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Use the recently added extack support for eBPF offload in the driver.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/main.c    | 31 ++++++++++++++++++------
 drivers/net/ethernet/netronome/nfp/bpf/main.h    |  2 +-
 drivers/net/ethernet/netronome/nfp/bpf/offload.c | 24 +++++++++++-------
 3 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index e8816ab8fb63..a638c3ab6b61 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -70,7 +70,7 @@ nfp_bpf_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
 	if (prog && running && !xdp_running)
 		return -EBUSY;
 
-	ret = nfp_net_bpf_offload(nn, prog, running);
+	ret = nfp_net_bpf_offload(nn, prog, running, extack);
 	/* Stop offload if replace not possible */
 	if (ret && prog)
 		nfp_bpf_xdp_offload(app, nn, NULL, extack);
@@ -125,17 +125,31 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
 	struct nfp_bpf_vnic *bv;
 	int err;
 
-	if (type != TC_SETUP_CLSBPF ||
-	    !tc_can_offload(nn->dp.netdev) ||
-	    !nfp_net_ebpf_capable(nn) ||
-	    cls_bpf->common.protocol != htons(ETH_P_ALL) ||
-	    cls_bpf->common.chain_index)
+	if (type != TC_SETUP_CLSBPF) {
+		NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
+				   "only offload of BPF classifiers supported");
+		return -EOPNOTSUPP;
+	}
+	if (!tc_can_offload_extack(nn->dp.netdev, cls_bpf->common.extack))
+		return -EOPNOTSUPP;
+	if (!nfp_net_ebpf_capable(nn)) {
+		NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
+				   "NFP firmware does not support eBPF offload");
+		return -EOPNOTSUPP;
+	}
+	if (cls_bpf->common.protocol != htons(ETH_P_ALL)) {
+		NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
+				   "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 ||
 	    tcf_exts_has_actions(cls_bpf->exts)) {
-		nn_err(nn, "only direct action with no legacy actions supported\n");
+		NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
+				   "only direct action with no legacy actions supported");
 		return -EOPNOTSUPP;
 	}
 
@@ -152,7 +166,8 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
 			return 0;
 	}
 
-	err = nfp_net_bpf_offload(nn, cls_bpf->prog, oldprog);
+	err = nfp_net_bpf_offload(nn, cls_bpf->prog, oldprog,
+				  cls_bpf->common.extack);
 	if (err)
 		return err;
 
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index b80e75a8ecda..80855d43b25e 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -334,7 +334,7 @@ struct nfp_net;
 int nfp_ndo_bpf(struct nfp_app *app, struct nfp_net *nn,
 		struct netdev_bpf *bpf);
 int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
-			bool old_prog);
+			bool old_prog, struct netlink_ext_ack *extack);
 
 struct nfp_insn_meta *
 nfp_bpf_goto_meta(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index e2859b2e9c6a..9c78a09cda24 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -271,7 +271,9 @@ int nfp_ndo_bpf(struct nfp_app *app, struct nfp_net *nn, struct netdev_bpf *bpf)
 	}
 }
 
-static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
+static int
+nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog,
+		 struct netlink_ext_ack *extack)
 {
 	struct nfp_prog *nfp_prog = prog->aux->offload->dev_priv;
 	unsigned int max_mtu;
@@ -281,7 +283,7 @@ static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
 
 	max_mtu = nn_readb(nn, NFP_NET_CFG_BPF_INL_MTU) * 64 - 32;
 	if (max_mtu < nn->dp.netdev->mtu) {
-		nn_info(nn, "BPF offload not supported with MTU larger than HW packet split boundary\n");
+		NL_SET_ERR_MSG_MOD(extack, "BPF offload not supported with MTU larger than HW packet split boundary");
 		return -EOPNOTSUPP;
 	}
 
@@ -303,7 +305,8 @@ static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
 	/* Load up the JITed code */
 	err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_BPF);
 	if (err)
-		nn_err(nn, "FW command error while loading BPF: %d\n", err);
+		NL_SET_ERR_MSG_MOD(extack,
+				   "FW command error while loading BPF");
 
 	dma_unmap_single(nn->dp.dev, dma_addr, nfp_prog->prog_len * sizeof(u64),
 			 DMA_TO_DEVICE);
@@ -312,7 +315,8 @@ static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
 	return err;
 }
 
-static void nfp_net_bpf_start(struct nfp_net *nn)
+static void
+nfp_net_bpf_start(struct nfp_net *nn, struct netlink_ext_ack *extack)
 {
 	int err;
 
@@ -321,7 +325,8 @@ static void nfp_net_bpf_start(struct nfp_net *nn)
 	nn_writel(nn, NFP_NET_CFG_CTRL, nn->dp.ctrl);
 	err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_GEN);
 	if (err)
-		nn_err(nn, "FW command error while enabling BPF: %d\n", err);
+		NL_SET_ERR_MSG_MOD(extack,
+				   "FW command error while enabling BPF");
 }
 
 static int nfp_net_bpf_stop(struct nfp_net *nn)
@@ -336,7 +341,7 @@ static int nfp_net_bpf_stop(struct nfp_net *nn)
 }
 
 int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
-			bool old_prog)
+			bool old_prog, struct netlink_ext_ack *extack)
 {
 	int err;
 
@@ -354,7 +359,8 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
 
 		cap = nn_readb(nn, NFP_NET_CFG_BPF_CAP);
 		if (!(cap & NFP_NET_BPF_CAP_RELO)) {
-			nn_err(nn, "FW does not support live reload\n");
+			NL_SET_ERR_MSG_MOD(extack,
+					   "FW does not support live reload");
 			return -EBUSY;
 		}
 	}
@@ -366,12 +372,12 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
 	if (old_prog && !prog)
 		return nfp_net_bpf_stop(nn);
 
-	err = nfp_net_bpf_load(nn, prog);
+	err = nfp_net_bpf_load(nn, prog, extack);
 	if (err)
 		return err;
 
 	if (!old_prog)
-		nfp_net_bpf_start(nn);
+		nfp_net_bpf_start(nn, extack);
 
 	return 0;
 }
-- 
2.15.1

[PATCH bpf-next v2 10/11] netdevsim: add extack support for TC eBPF offload

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Use the recently added extack support for TC eBPF filters in netdevsim.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/netdevsim/bpf.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 5134d5c1306c..0de8ba91b262 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -109,17 +109,35 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
 	struct netdevsim *ns = cb_priv;
 	struct bpf_prog *oldprog;
 
-	if (type != TC_SETUP_CLSBPF ||
-	    !tc_can_offload(ns->netdev) ||
-	    cls_bpf->common.protocol != htons(ETH_P_ALL) ||
-	    cls_bpf->common.chain_index)
+	if (type != TC_SETUP_CLSBPF) {
+		NSIM_EA(cls_bpf->common.extack,
+			"only offload of BPF classifiers supported");
+		return -EOPNOTSUPP;
+	}
+
+	if (!tc_can_offload_extack(ns->netdev, cls_bpf->common.extack))
+		return -EOPNOTSUPP;
+
+	if (cls_bpf->common.protocol != htons(ETH_P_ALL)) {
+		NSIM_EA(cls_bpf->common.extack,
+			"only ETH_P_ALL supported as filter protocol");
+		return -EOPNOTSUPP;
+	}
+
+	if (cls_bpf->common.chain_index)
 		return -EOPNOTSUPP;
 
-	if (!ns->bpf_tc_accept)
+	if (!ns->bpf_tc_accept) {
+		NSIM_EA(cls_bpf->common.extack,
+			"netdevsim configured to reject BPF TC offload");
 		return -EOPNOTSUPP;
+	}
 	/* Note: progs without skip_sw will probably not be dev bound */
-	if (prog && !prog->aux->offload && !ns->bpf_tc_non_bound_accept)
+	if (prog && !prog->aux->offload && !ns->bpf_tc_non_bound_accept) {
+		NSIM_EA(cls_bpf->common.extack,
+			"netdevsim configured to reject unbound programs");
 		return -EOPNOTSUPP;
+	}
 
 	if (cls_bpf->command != TC_CLSBPF_OFFLOAD)
 		return -EOPNOTSUPP;
@@ -131,8 +149,11 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
 		oldprog = NULL;
 		if (!cls_bpf->prog)
 			return 0;
-		if (ns->bpf_offloaded)
+		if (ns->bpf_offloaded) {
+			NSIM_EA(cls_bpf->common.extack,
+				"driver and netdev offload states mismatch");
 			return -EBUSY;
+		}
 	}
 
 	return nsim_bpf_offload(ns, cls_bpf->prog, oldprog);
-- 
2.15.1

[PATCH bpf-next v2 11/11] selftests/bpf: add checks on extack messages for eBPF hw offload tests

[Jakub Kicinski]

From: Quentin Monnet <quentin.monnet@netronome.com>

Add checks to test that netlink extack messages are correctly displayed
in some expected error cases for eBPF offload to netdevsim with TC and
XDP.

iproute2 may be built without libmnl support, in which case the extack
messages will not be reported.  Try to detect this condition, and when
enountered print a mild warning to the user and skip the extack validation.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
v2:
 - auto-skip the extack message validation (David A).

 tools/testing/selftests/bpf/test_offload.py | 104 +++++++++++++++++++++-------
 1 file changed, 78 insertions(+), 26 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
index e3c750f17cb8..28ddb30884fd 100755
--- a/tools/testing/selftests/bpf/test_offload.py
+++ b/tools/testing/selftests/bpf/test_offload.py
@@ -25,6 +25,7 @@ import time
 
 logfile = None
 log_level = 1
+skip_extack = False
 bpf_test_dir = os.path.dirname(os.path.realpath(__file__))
 pp = pprint.PrettyPrinter()
 devs = [] # devices we created for clean up
@@ -131,7 +132,7 @@ netns = [] # net namespaces to be removed
     if f in files:
         files.remove(f)
 
-def tool(name, args, flags, JSON=True, ns="", fail=True):
+def tool(name, args, flags, JSON=True, ns="", fail=True, include_stderr=False):
     params = ""
     if JSON:
         params += "%s " % (flags["json"])
@@ -139,9 +140,20 @@ netns = [] # net namespaces to be removed
     if ns != "":
         ns = "ip netns exec %s " % (ns)
 
-    ret, out = cmd(ns + name + " " + params + args, fail=fail)
-    if JSON and len(out.strip()) != 0:
-        return ret, json.loads(out)
+    if include_stderr:
+        ret, stdout, stderr = cmd(ns + name + " " + params + args,
+                                  fail=fail, include_stderr=True)
+    else:
+        ret, stdout = cmd(ns + name + " " + params + args,
+                          fail=fail, include_stderr=False)
+
+    if JSON and len(stdout.strip()) != 0:
+        out = json.loads(stdout)
+    else:
+        out = stdout
+
+    if include_stderr:
+        return ret, out, stderr
     else:
         return ret, out
 
@@ -164,13 +176,15 @@ netns = [] # net namespaces to be removed
         time.sleep(0.05)
     raise Exception("Time out waiting for program counts to stabilize want %d, have %d" % (expected, nprogs))
 
-def ip(args, force=False, JSON=True, ns="", fail=True):
+def ip(args, force=False, JSON=True, ns="", fail=True, include_stderr=False):
     if force:
         args = "-force " + args
-    return tool("ip", args, {"json":"-j"}, JSON=JSON, ns=ns, fail=fail)
+    return tool("ip", args, {"json":"-j"}, JSON=JSON, ns=ns, fail=fail,
+                include_stderr=include_stderr)
 
-def tc(args, JSON=True, ns="", fail=True):
-    return tool("tc", args, {"json":"-p"}, JSON=JSON, ns=ns, fail=fail)
+def tc(args, JSON=True, ns="", fail=True, include_stderr=False):
+    return tool("tc", args, {"json":"-p"}, JSON=JSON, ns=ns, fail=fail,
+                include_stderr=include_stderr)
 
 def ethtool(dev, opt, args, fail=True):
     return cmd("ethtool %s %s %s" % (opt, dev["ifname"], args), fail=fail)
@@ -311,13 +325,13 @@ netns = [] # net namespaces to be removed
         return ip("link set dev %s mtu %d" % (self.dev["ifname"], mtu),
                   fail=fail)
 
-    def set_xdp(self, bpf, mode, force=False, fail=True):
+    def set_xdp(self, bpf, mode, force=False, fail=True, include_stderr=False):
         return ip("link set dev %s xdp%s %s" % (self.dev["ifname"], mode, bpf),
-                  force=force, fail=fail)
+                  force=force, fail=fail, include_stderr=include_stderr)
 
-    def unset_xdp(self, mode, force=False, fail=True):
+    def unset_xdp(self, mode, force=False, fail=True, include_stderr=False):
         return ip("link set dev %s xdp%s off" % (self.dev["ifname"], mode),
-                  force=force, fail=fail)
+                  force=force, fail=fail, include_stderr=include_stderr)
 
     def ip_link_show(self, xdp):
         _, link = ip("link show dev %s" % (self['ifname']))
@@ -373,7 +387,7 @@ netns = [] # net namespaces to be removed
         return filters
 
     def cls_bpf_add_filter(self, bpf, da=False, skip_sw=False, skip_hw=False,
-                           fail=True):
+                           fail=True, include_stderr=False):
         params = ""
         if da:
             params += " da"
@@ -382,7 +396,8 @@ netns = [] # net namespaces to be removed
         if skip_hw:
             params += " skip_hw"
         return tc("filter add dev %s ingress bpf %s %s" %
-                  (self['ifname'], bpf, params), fail=fail)
+                  (self['ifname'], bpf, params), fail=fail,
+                  include_stderr=include_stderr)
 
     def set_ethtool_tc_offloads(self, enable, fail=True):
         args = "hw-tc-offload %s" % ("on" if enable else "off")
@@ -434,6 +449,13 @@ netns = [] # net namespaces to be removed
             fail(dev["ns_dev"] != 0, "Device perameters not zero on removed")
             fail(dev["ns_inode"] != 0, "Device perameters not zero on removed")
 
+def check_extack(output, reference, args):
+    if skip_extack:
+        return
+    lines = output.split("\n")
+    comp = len(lines) >= 2 and lines[1] == reference
+    fail(not comp, "Missing or incorrect netlink extack message")
+
 # Parse command line
 parser = argparse.ArgumentParser()
 parser.add_argument("--log", help="output verbose log to given file")
@@ -470,6 +492,14 @@ samples = ["sample_ret0.o"]
     skip(ret != 0, "sample %s/%s not found, please compile it" %
          (bpf_test_dir, s))
 
+# Check if iproute2 is built with libmnl (needed by extack support)
+_, _, err = cmd("tc qdisc delete dev lo handle 0",
+                fail=False, include_stderr=True)
+if err.find("Error: Failed to find qdisc with specified handle.") == -1:
+    print("Warning: no extack message in iproute2 output, libmnl missing?")
+    log("Warning: no extack message in iproute2 output, libmnl missing?", "")
+    skip_extack = True
+
 # Check if net namespaces seem to work
 ns = mknetns()
 skip(ns is None, "Could not create a net namespace")
@@ -501,8 +531,10 @@ netns = []
     sim.tc_flush_filters()
 
     start_test("Test TC offloads are off by default...")
-    ret, _ = sim.cls_bpf_add_filter(obj, skip_sw=True, fail=False)
+    ret, _, err = sim.cls_bpf_add_filter(obj, skip_sw=True, fail=False,
+                                         include_stderr=True)
     fail(ret == 0, "TC filter loaded without enabling TC offloads")
+    check_extack(err, "Error: TC offload is disabled on net device.", args)
     sim.wait_for_flush()
 
     sim.set_ethtool_tc_offloads(True)
@@ -530,8 +562,10 @@ netns = []
     sim.dfs["bpf_tc_non_bound_accept"] = "N"
 
     start_test("Test TC cBPF unbound bytecode doesn't offload...")
-    ret, _ = sim.cls_bpf_add_filter(bytecode, skip_sw=True, fail=False)
+    ret, _, err = sim.cls_bpf_add_filter(bytecode, skip_sw=True, fail=False,
+                                         include_stderr=True)
     fail(ret == 0, "TC bytecode loaded for offload")
+    check_extack(err, "Error: netdevsim: netdevsim configured to reject unbound programs.", args)
     sim.wait_for_flush()
 
     start_test("Test TC offloads work...")
@@ -612,16 +646,24 @@ netns = []
          "Device parameters reported for non-offloaded program")
 
     start_test("Test XDP prog replace with bad flags...")
-    ret, _ = sim.set_xdp(obj, "offload", force=True, fail=False)
+    ret, _, err = sim.set_xdp(obj, "offload", force=True, fail=False,
+                              include_stderr=True)
     fail(ret == 0, "Replaced XDP program with a program in different mode")
-    ret, _ = sim.set_xdp(obj, "", force=True, fail=False)
+    check_extack(err, "Error: netdevsim: program loaded with different flags.", args)
+    ret, _, err = sim.set_xdp(obj, "", force=True, fail=False,
+                              include_stderr=True)
     fail(ret == 0, "Replaced XDP program with a program in different mode")
+    check_extack(err, "Error: netdevsim: program loaded with different flags.", args)
 
     start_test("Test XDP prog remove with bad flags...")
-    ret, _ = sim.unset_xdp("offload", force=True, fail=False)
+    ret, _, err = sim.unset_xdp("offload", force=True, fail=False,
+                                include_stderr=True)
     fail(ret == 0, "Removed program with a bad mode mode")
-    ret, _ = sim.unset_xdp("", force=True, fail=False)
+    check_extack(err, "Error: netdevsim: program loaded with different flags.", args)
+    ret, _, err = sim.unset_xdp("", force=True, fail=False,
+                                include_stderr=True)
     fail(ret == 0, "Removed program with a bad mode mode")
+    check_extack(err, "Error: netdevsim: program loaded with different flags.", args)
 
     start_test("Test MTU restrictions...")
     ret, _ = sim.set_mtu(9000, fail=False)
@@ -630,8 +672,9 @@ netns = []
     sim.unset_xdp("drv")
     bpftool_prog_list_wait(expected=0)
     sim.set_mtu(9000)
-    ret, _ = sim.set_xdp(obj, "drv", fail=False)
+    ret, _, err = sim.set_xdp(obj, "drv", fail=False, include_stderr=True)
     fail(ret == 0, "Driver should refuse to load program with MTU of 9000...")
+    check_extack(err, "Error: netdevsim: MTU too large w/ XDP enabled.", args)
     sim.set_mtu(1500)
 
     sim.wait_for_flush()
@@ -667,25 +710,32 @@ netns = []
     sim2.set_xdp(obj, "offload")
     pin_file, pinned = pin_prog("/sys/fs/bpf/tmp")
 
-    ret, _ = sim.set_xdp(pinned, "offload", fail=False)
+    ret, _, err = sim.set_xdp(pinned, "offload", fail=False,
+                              include_stderr=True)
     fail(ret == 0, "Pinned program loaded for a different device accepted")
+    check_extack(err, "Error: netdevsim: program bound to different dev.", args)
     sim2.remove()
-    ret, _ = sim.set_xdp(pinned, "offload", fail=False)
+    ret, _, err = sim.set_xdp(pinned, "offload", fail=False,
+                              include_stderr=True)
     fail(ret == 0, "Pinned program loaded for a removed device accepted")
+    check_extack(err, "Error: netdevsim: xdpoffload of non-bound program.", args)
     rm(pin_file)
     bpftool_prog_list_wait(expected=0)
 
     start_test("Test mixing of TC and XDP...")
     sim.tc_add_ingress()
     sim.set_xdp(obj, "offload")
-    ret, _ = sim.cls_bpf_add_filter(obj, skip_sw=True, fail=False)
+    ret, _, err = sim.cls_bpf_add_filter(obj, skip_sw=True, fail=False,
+                                         include_stderr=True)
     fail(ret == 0, "Loading TC when XDP active should fail")
+    check_extack(err, "Error: netdevsim: driver and netdev offload states mismatch.", args)
     sim.unset_xdp("offload")
     sim.wait_for_flush()
 
     sim.cls_bpf_add_filter(obj, skip_sw=True)
-    ret, _ = sim.set_xdp(obj, "offload", fail=False)
+    ret, _, err = sim.set_xdp(obj, "offload", fail=False, include_stderr=True)
     fail(ret == 0, "Loading XDP when TC active should fail")
+    check_extack(err, "Error: netdevsim: TC program is already loaded.", args)
 
     start_test("Test binding TC from pinned...")
     pin_file, pinned = pin_prog("/sys/fs/bpf/tmp")
@@ -708,8 +758,10 @@ netns = []
 
     start_test("Test asking for TC offload of two filters...")
     sim.cls_bpf_add_filter(obj, da=True, skip_sw=True)
-    ret, _ = sim.cls_bpf_add_filter(obj, da=True, skip_sw=True, fail=False)
+    ret, _, err = sim.cls_bpf_add_filter(obj, da=True, skip_sw=True,
+                                         fail=False, include_stderr=True)
     fail(ret == 0, "Managed to offload two TC filters at the same time")
+    check_extack(err, "Error: netdevsim: driver and netdev offload states mismatch.", args)
 
     sim.tc_flush_filters(bound=2, total=2)
 
-- 
2.15.1

Re: [PATCH bpf-next v2 02/11] net: sched: prepare extack support for offload via tc_cls_common_offload

[Jiri Pirko]

Tue, Jan 16, 2018 at 03:08:36AM CET, jakub.kicinski@netronome.com wrote:
>From: Quentin Monnet <quentin.monnet@netronome.com>
>
>Prepare for extack support for hardware offload of classifiers. In order
>to achieve this, a pointer to a struct netlink_ext_ack is added to the
>struct tc_cls_common_offload that is passed to the callback for setting
>up the classifier. Function tc_cls_common_offload_init() is updated to
>support initialization of this new attribute.
>
>Extack plumbing is not complete yet.
>
>Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---

[...]


>diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>index 998ee4faf934..f1640a98a3d2 100644
>--- a/net/sched/cls_flower.c
>+++ b/net/sched/cls_flower.c
>@@ -223,7 +223,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
> 	struct tc_cls_flower_offload cls_flower = {};
> 	struct tcf_block *block = tp->chain->block;
> 
>-	tc_cls_common_offload_init(&cls_flower.common, tp);
>+	tc_cls_common_offload_init(&cls_flower.common, tp, NULL);

You should have the patch assing extack arg to fl_hw_replace_filter
prior to this patch. Then you don't need this odd NULL pass.
Same for other cls.

Re: [PATCH bpf-next v2 09/11] nfp: bpf: use extack support to improve debugging

[Jiri Pirko]

Tue, Jan 16, 2018 at 03:08:43AM CET, jakub.kicinski@netronome.com wrote:
>From: Quentin Monnet <quentin.monnet@netronome.com>
>
>Use the recently added extack support for eBPF offload in the driver.
>
>Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---

[...]


>@@ -303,7 +305,8 @@ static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
> 	/* Load up the JITed code */
> 	err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_BPF);
> 	if (err)
>-		nn_err(nn, "FW command error while loading BPF: %d\n", err);
>+		NL_SET_ERR_MSG_MOD(extack,
>+				   "FW command error while loading BPF");

One line please. Same for all others. Strings may overflow 80 cols.

Re: [PATCH bpf-next v2 02/11] net: sched: prepare extack support for offload via tc_cls_common_offload

[Quentin Monnet]

2018-01-16 10:33 UTC+0100 ~ Jiri Pirko <jiri@resnulli.us>
> Tue, Jan 16, 2018 at 03:08:36AM CET, jakub.kicinski@netronome.com wrote:
>> From: Quentin Monnet <quentin.monnet@netronome.com>
>>
>> Prepare for extack support for hardware offload of classifiers. In order
>> to achieve this, a pointer to a struct netlink_ext_ack is added to the
>> struct tc_cls_common_offload that is passed to the callback for setting
>> up the classifier. Function tc_cls_common_offload_init() is updated to
>> support initialization of this new attribute.
>>
>> Extack plumbing is not complete yet.
>>
>> Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
>> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> ---
> 
> [...]
> 
> 
>> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>> index 998ee4faf934..f1640a98a3d2 100644
>> --- a/net/sched/cls_flower.c
>> +++ b/net/sched/cls_flower.c
>> @@ -223,7 +223,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f)
>> 	struct tc_cls_flower_offload cls_flower = {};
>> 	struct tcf_block *block = tp->chain->block;
>>
>> -	tc_cls_common_offload_init(&cls_flower.common, tp);
>> +	tc_cls_common_offload_init(&cls_flower.common, tp, NULL);
> 
> You should have the patch assing extack arg to fl_hw_replace_filter
> prior to this patch. Then you don't need this odd NULL pass.
> Same for other cls.

It makes sense, thank you Jiri. I'll address this and your comment on
patch 9, and respin.

Quentin

Re: [PATCH bpf-next v2 09/11] nfp: bpf: use extack support to improve debugging

[Jakub Kicinski]

On Tue, 16 Jan 2018 10:36:01 +0100, Jiri Pirko wrote:
> >@@ -303,7 +305,8 @@ static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
> > 	/* Load up the JITed code */
> > 	err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_BPF);
> > 	if (err)
> >-		nn_err(nn, "FW command error while loading BPF: %d\n", err);
> >+		NL_SET_ERR_MSG_MOD(extack,
> >+				   "FW command error while loading BPF");  
> 
> One line please. Same for all others. Strings may overflow 80 cols.

Sorry, but this is the way I want things in the nfp driver.  If the
string would fit 80 chars placed on a new line, it should be placed 
on a new line.  If it doesn't fit anyway the new line is unnecessary.
This rules is adhered to throughout the driver (to the extent I'm able
to enforce it).

This is the diff of what you asked:

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c b/drivers/net/ethernet/netronome/nfp/bpf/main.c
index a638c3ab6b61..1e987bcbc086 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c
@@ -126,20 +126,17 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
        int err;
 
        if (type != TC_SETUP_CLSBPF) {
-               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
-                                  "only offload of BPF classifiers supported");
+               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, "only offload of BPF classifiers supported");
                return -EOPNOTSUPP;
        }
        if (!tc_can_offload_extack(nn->dp.netdev, cls_bpf->common.extack))
                return -EOPNOTSUPP;
        if (!nfp_net_ebpf_capable(nn)) {
-               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
-                                  "NFP firmware does not support eBPF offload");
+               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, "NFP firmware does not support eBPF offload");
                return -EOPNOTSUPP;
        }
        if (cls_bpf->common.protocol != htons(ETH_P_ALL)) {
-               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
-                                  "only ETH_P_ALL supported as filter protocol");
+               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, "only ETH_P_ALL supported as filter protocol");
                return -EOPNOTSUPP;
        }
        if (cls_bpf->common.chain_index)
@@ -148,8 +145,7 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type type,
        /* Only support TC direct action */
        if (!cls_bpf->exts_integrated ||
            tcf_exts_has_actions(cls_bpf->exts)) {
-               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack,
-                                  "only direct action with no legacy actions supported");
+               NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, "only direct action with no legacy actions supported");
                return -EOPNOTSUPP;
        }
 
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
index 9c78a09cda24..181c90476854 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
@@ -305,8 +305,7 @@ nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog,
        /* Load up the JITed code */
        err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_BPF);
        if (err)
-               NL_SET_ERR_MSG_MOD(extack,
-                                  "FW command error while loading BPF");
+               NL_SET_ERR_MSG_MOD(extack, "FW command error while loading BPF");
 
        dma_unmap_single(nn->dp.dev, dma_addr, nfp_prog->prog_len * sizeof(u64),
                         DMA_TO_DEVICE);
@@ -325,8 +324,7 @@ nfp_net_bpf_start(struct nfp_net *nn, struct netlink_ext_ack *extack)
        nn_writel(nn, NFP_NET_CFG_CTRL, nn->dp.ctrl);
        err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_GEN);
        if (err)
-               NL_SET_ERR_MSG_MOD(extack,
-                                  "FW command error while enabling BPF");
+               NL_SET_ERR_MSG_MOD(extack, "FW command error while enabling BPF");
 }
 
 static int nfp_net_bpf_stop(struct nfp_net *nn)
@@ -359,8 +357,7 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
 
                cap = nn_readb(nn, NFP_NET_CFG_BPF_CAP);
                if (!(cap & NFP_NET_BPF_CAP_RELO)) {
-                       NL_SET_ERR_MSG_MOD(extack,
-                                          "FW does not support live reload");
+                       NL_SET_ERR_MSG_MOD(extack, "FW does not support live reload");
                        return -EBUSY;
                }
        }

strings overflow to new line on the left which is yucky and avoidable.

Please speak up if you feel strongly about this, otherwise I will
repost addressing only your other comment.

Thanks for reviewing!

Re: [PATCH bpf-next v2 09/11] nfp: bpf: use extack support to improve debugging

[David Ahern]

On 1/16/18 12:11 PM, Jakub Kicinski wrote:
> On Tue, 16 Jan 2018 10:36:01 +0100, Jiri Pirko wrote:
>>> @@ -303,7 +305,8 @@ static int nfp_net_bpf_load(struct nfp_net *nn, struct bpf_prog *prog)
>>> 	/* Load up the JITed code */
>>> 	err = nfp_net_reconfig(nn, NFP_NET_CFG_UPDATE_BPF);
>>> 	if (err)
>>> -		nn_err(nn, "FW command error while loading BPF: %d\n", err);
>>> +		NL_SET_ERR_MSG_MOD(extack,
>>> +				   "FW command error while loading BPF");  
>>
>> One line please. Same for all others. Strings may overflow 80 cols.
> 
> Sorry, but this is the way I want things in the nfp driver.  If the
> string would fit 80 chars placed on a new line, it should be placed 
> on a new line.  If it doesn't fit anyway the new line is unnecessary.
> This rules is adhered to throughout the driver (to the extent I'm able
> to enforce it).
> 

+1