All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key()
@ 2020-03-23 20:48 Guillaume Nault
  2020-03-23 20:48 ` [PATCH net-next 1/4] net: sched: refine extack messages in tcf_change_indev Guillaume Nault
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Guillaume Nault @ 2020-03-23 20:48 UTC (permalink / raw
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko

Add missing extack messages in fl_set_key(), so that users can get more
meaningfull error messages when netlink attributes are rejected.

Patch 1 also extends extack in tcf_change_indev() (in pkt_cls.h) since
this function is used by fl_set_key().

Guillaume Nault (4):
  net: sched: refine extack messages in tcf_change_indev
  cls_flower: Add extack support for mpls options
  cls_flower: Add extack support for src and dst port range options
  cls_flower: Add extack support for flags key

 include/net/pkt_cls.h  |  8 ++++--
 net/sched/cls_flower.c | 60 ++++++++++++++++++++++++++++++------------
 2 files changed, 49 insertions(+), 19 deletions(-)

-- 
2.21.1


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

* [PATCH net-next 1/4] net: sched: refine extack messages in tcf_change_indev
  2020-03-23 20:48 [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() Guillaume Nault
@ 2020-03-23 20:48 ` Guillaume Nault
  2020-03-23 20:48 ` [PATCH net-next 2/4] cls_flower: Add extack support for mpls options Guillaume Nault
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guillaume Nault @ 2020-03-23 20:48 UTC (permalink / raw
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko

Add an error message when device wasn't found.
While there, also set the bad attribute's offset in extack.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 include/net/pkt_cls.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 1db8b27d4515..41902e10d503 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -502,12 +502,16 @@ tcf_change_indev(struct net *net, struct nlattr *indev_tlv,
 	struct net_device *dev;
 
 	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) >= IFNAMSIZ) {
-		NL_SET_ERR_MSG(extack, "Interface name too long");
+		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
+				    "Interface name too long");
 		return -EINVAL;
 	}
 	dev = __dev_get_by_name(net, indev);
-	if (!dev)
+	if (!dev) {
+		NL_SET_ERR_MSG_ATTR(extack, indev_tlv,
+				    "Network device not found");
 		return -ENODEV;
+	}
 	return dev->ifindex;
 }
 
-- 
2.21.1


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

* [PATCH net-next 2/4] cls_flower: Add extack support for mpls options
  2020-03-23 20:48 [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() Guillaume Nault
  2020-03-23 20:48 ` [PATCH net-next 1/4] net: sched: refine extack messages in tcf_change_indev Guillaume Nault
@ 2020-03-23 20:48 ` Guillaume Nault
  2020-03-23 20:48 ` [PATCH net-next 3/4] cls_flower: Add extack support for src and dst port range options Guillaume Nault
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guillaume Nault @ 2020-03-23 20:48 UTC (permalink / raw
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko

Pass extack down to fl_set_key_mpls() and set message on error.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 net/sched/cls_flower.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 258dc45ab7e3..544cc7b490a3 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -766,7 +766,8 @@ static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
 
 static int fl_set_key_mpls(struct nlattr **tb,
 			   struct flow_dissector_key_mpls *key_val,
-			   struct flow_dissector_key_mpls *key_mask)
+			   struct flow_dissector_key_mpls *key_mask,
+			   struct netlink_ext_ack *extack)
 {
 	if (tb[TCA_FLOWER_KEY_MPLS_TTL]) {
 		key_val->mpls_ttl = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_TTL]);
@@ -775,24 +776,36 @@ static int fl_set_key_mpls(struct nlattr **tb,
 	if (tb[TCA_FLOWER_KEY_MPLS_BOS]) {
 		u8 bos = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_BOS]);
 
-		if (bos & ~MPLS_BOS_MASK)
+		if (bos & ~MPLS_BOS_MASK) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[TCA_FLOWER_KEY_MPLS_BOS],
+					    "Bottom Of Stack (BOS) must be 0 or 1");
 			return -EINVAL;
+		}
 		key_val->mpls_bos = bos;
 		key_mask->mpls_bos = MPLS_BOS_MASK;
 	}
 	if (tb[TCA_FLOWER_KEY_MPLS_TC]) {
 		u8 tc = nla_get_u8(tb[TCA_FLOWER_KEY_MPLS_TC]);
 
-		if (tc & ~MPLS_TC_MASK)
+		if (tc & ~MPLS_TC_MASK) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[TCA_FLOWER_KEY_MPLS_TC],
+					    "Traffic Class (TC) must be between 0 and 7");
 			return -EINVAL;
+		}
 		key_val->mpls_tc = tc;
 		key_mask->mpls_tc = MPLS_TC_MASK;
 	}
 	if (tb[TCA_FLOWER_KEY_MPLS_LABEL]) {
 		u32 label = nla_get_u32(tb[TCA_FLOWER_KEY_MPLS_LABEL]);
 
-		if (label & ~MPLS_LABEL_MASK)
+		if (label & ~MPLS_LABEL_MASK) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[TCA_FLOWER_KEY_MPLS_LABEL],
+					    "Label must be between 0 and 1048575");
 			return -EINVAL;
+		}
 		key_val->mpls_label = label;
 		key_mask->mpls_label = MPLS_LABEL_MASK;
 	}
@@ -1364,7 +1377,7 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 			       sizeof(key->icmp.code));
 	} else if (key->basic.n_proto == htons(ETH_P_MPLS_UC) ||
 		   key->basic.n_proto == htons(ETH_P_MPLS_MC)) {
-		ret = fl_set_key_mpls(tb, &key->mpls, &mask->mpls);
+		ret = fl_set_key_mpls(tb, &key->mpls, &mask->mpls, extack);
 		if (ret)
 			return ret;
 	} else if (key->basic.n_proto == htons(ETH_P_ARP) ||
-- 
2.21.1


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

* [PATCH net-next 3/4] cls_flower: Add extack support for src and dst port range options
  2020-03-23 20:48 [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() Guillaume Nault
  2020-03-23 20:48 ` [PATCH net-next 1/4] net: sched: refine extack messages in tcf_change_indev Guillaume Nault
  2020-03-23 20:48 ` [PATCH net-next 2/4] cls_flower: Add extack support for mpls options Guillaume Nault
@ 2020-03-23 20:48 ` Guillaume Nault
  2020-03-23 20:48 ` [PATCH net-next 4/4] cls_flower: Add extack support for flags key Guillaume Nault
  2020-03-27  2:52 ` [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Guillaume Nault @ 2020-03-23 20:48 UTC (permalink / raw
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko

Pass extack down to fl_set_key_port_range() and set message on error.

Both the min and max ports would qualify as invalid attributes here.
Report the min one as invalid, as it's probably what makes the most
sense from a user point of view.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 net/sched/cls_flower.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 544cc7b490a3..5811dd971ee5 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -738,7 +738,8 @@ static void fl_set_key_val(struct nlattr **tb,
 }
 
 static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
-				 struct fl_flow_key *mask)
+				 struct fl_flow_key *mask,
+				 struct netlink_ext_ack *extack)
 {
 	fl_set_key_val(tb, &key->tp_range.tp_min.dst,
 		       TCA_FLOWER_KEY_PORT_DST_MIN, &mask->tp_range.tp_min.dst,
@@ -753,13 +754,22 @@ static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
 		       TCA_FLOWER_KEY_PORT_SRC_MAX, &mask->tp_range.tp_max.src,
 		       TCA_FLOWER_UNSPEC, sizeof(key->tp_range.tp_max.src));
 
-	if ((mask->tp_range.tp_min.dst && mask->tp_range.tp_max.dst &&
-	     htons(key->tp_range.tp_max.dst) <=
-		 htons(key->tp_range.tp_min.dst)) ||
-	    (mask->tp_range.tp_min.src && mask->tp_range.tp_max.src &&
-	     htons(key->tp_range.tp_max.src) <=
-		 htons(key->tp_range.tp_min.src)))
+	if (mask->tp_range.tp_min.dst && mask->tp_range.tp_max.dst &&
+	    htons(key->tp_range.tp_max.dst) <=
+	    htons(key->tp_range.tp_min.dst)) {
+		NL_SET_ERR_MSG_ATTR(extack,
+				    tb[TCA_FLOWER_KEY_PORT_DST_MIN],
+				    "Invalid destination port range (min must be strictly smaller than max)");
 		return -EINVAL;
+	}
+	if (mask->tp_range.tp_min.src && mask->tp_range.tp_max.src &&
+	    htons(key->tp_range.tp_max.src) <=
+	    htons(key->tp_range.tp_min.src)) {
+		NL_SET_ERR_MSG_ATTR(extack,
+				    tb[TCA_FLOWER_KEY_PORT_SRC_MIN],
+				    "Invalid source port range (min must be strictly smaller than max)");
+		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -1402,7 +1412,7 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 	if (key->basic.ip_proto == IPPROTO_TCP ||
 	    key->basic.ip_proto == IPPROTO_UDP ||
 	    key->basic.ip_proto == IPPROTO_SCTP) {
-		ret = fl_set_key_port_range(tb, key, mask);
+		ret = fl_set_key_port_range(tb, key, mask, extack);
 		if (ret)
 			return ret;
 	}
-- 
2.21.1


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

* [PATCH net-next 4/4] cls_flower: Add extack support for flags key
  2020-03-23 20:48 [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() Guillaume Nault
                   ` (2 preceding siblings ...)
  2020-03-23 20:48 ` [PATCH net-next 3/4] cls_flower: Add extack support for src and dst port range options Guillaume Nault
@ 2020-03-23 20:48 ` Guillaume Nault
  2020-03-27  2:52 ` [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Guillaume Nault @ 2020-03-23 20:48 UTC (permalink / raw
  To: David Miller, Jakub Kicinski
  Cc: netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko

Pass extack down to fl_set_key_flags() and set message on error.

Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
 net/sched/cls_flower.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 5811dd971ee5..9b6acd736dc8 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -856,14 +856,16 @@ static void fl_set_key_flag(u32 flower_key, u32 flower_mask,
 	}
 }
 
-static int fl_set_key_flags(struct nlattr **tb,
-			    u32 *flags_key, u32 *flags_mask)
+static int fl_set_key_flags(struct nlattr **tb, u32 *flags_key,
+			    u32 *flags_mask, struct netlink_ext_ack *extack)
 {
 	u32 key, mask;
 
 	/* mask is mandatory for flags */
-	if (!tb[TCA_FLOWER_KEY_FLAGS_MASK])
+	if (!tb[TCA_FLOWER_KEY_FLAGS_MASK]) {
+		NL_SET_ERR_MSG(extack, "Missing flags mask");
 		return -EINVAL;
+	}
 
 	key = be32_to_cpu(nla_get_u32(tb[TCA_FLOWER_KEY_FLAGS]));
 	mask = be32_to_cpu(nla_get_u32(tb[TCA_FLOWER_KEY_FLAGS_MASK]));
@@ -1474,7 +1476,8 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 		return ret;
 
 	if (tb[TCA_FLOWER_KEY_FLAGS])
-		ret = fl_set_key_flags(tb, &key->control.flags, &mask->control.flags);
+		ret = fl_set_key_flags(tb, &key->control.flags,
+				       &mask->control.flags, extack);
 
 	return ret;
 }
-- 
2.21.1


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

* Re: [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key()
  2020-03-23 20:48 [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() Guillaume Nault
                   ` (3 preceding siblings ...)
  2020-03-23 20:48 ` [PATCH net-next 4/4] cls_flower: Add extack support for flags key Guillaume Nault
@ 2020-03-27  2:52 ` David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-03-27  2:52 UTC (permalink / raw
  To: gnault; +Cc: kuba, netdev, jhs, xiyou.wangcong, jiri

From: Guillaume Nault <gnault@redhat.com>
Date: Mon, 23 Mar 2020 21:48:45 +0100

> Add missing extack messages in fl_set_key(), so that users can get more
> meaningfull error messages when netlink attributes are rejected.
> 
> Patch 1 also extends extack in tcf_change_indev() (in pkt_cls.h) since
> this function is used by fl_set_key().

Series applied, thanks.

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

end of thread, other threads:[~2020-03-27  2:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-23 20:48 [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() Guillaume Nault
2020-03-23 20:48 ` [PATCH net-next 1/4] net: sched: refine extack messages in tcf_change_indev Guillaume Nault
2020-03-23 20:48 ` [PATCH net-next 2/4] cls_flower: Add extack support for mpls options Guillaume Nault
2020-03-23 20:48 ` [PATCH net-next 3/4] cls_flower: Add extack support for src and dst port range options Guillaume Nault
2020-03-23 20:48 ` [PATCH net-next 4/4] cls_flower: Add extack support for flags key Guillaume Nault
2020-03-27  2:52 ` [PATCH net-next 0/4] cls_flower: Use extack in fl_set_key() 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.