All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports
@ 2021-03-21 21:05 Vladimir Oltean
  2021-03-21 21:05 ` [PATCH net-next 2/2] net/sched: cls_flower: use nla_get_be32 for TCA_FLOWER_KEY_FLAGS Vladimir Oltean
  2021-03-22 20:00 ` [PATCH net-next 1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Vladimir Oltean @ 2021-03-21 21:05 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Vladimir Oltean

From: Vladimir Oltean <vladimir.oltean@nxp.com>

A make W=1 build complains that:

net/sched/cls_flower.c:214:20: warning: cast from restricted __be16
net/sched/cls_flower.c:214:20: warning: incorrect type in argument 1 (different base types)
net/sched/cls_flower.c:214:20:    expected unsigned short [usertype] val
net/sched/cls_flower.c:214:20:    got restricted __be16 [usertype] dst

This is because we use htons on struct flow_dissector_key_ports members
src and dst, which are defined as __be16, so they are already in network
byte order, not host. The byte swap function for the other direction
should have been used.

Because htons and ntohs do the same thing (either both swap, or none
does), this change has no functional effect except to silence the
warnings.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/sched/cls_flower.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index d097b5c15faa..832a0ece6dbf 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -209,16 +209,16 @@ static bool fl_range_port_dst_cmp(struct cls_fl_filter *filter,
 				  struct fl_flow_key *key,
 				  struct fl_flow_key *mkey)
 {
-	__be16 min_mask, max_mask, min_val, max_val;
+	u16 min_mask, max_mask, min_val, max_val;
 
-	min_mask = htons(filter->mask->key.tp_range.tp_min.dst);
-	max_mask = htons(filter->mask->key.tp_range.tp_max.dst);
-	min_val = htons(filter->key.tp_range.tp_min.dst);
-	max_val = htons(filter->key.tp_range.tp_max.dst);
+	min_mask = ntohs(filter->mask->key.tp_range.tp_min.dst);
+	max_mask = ntohs(filter->mask->key.tp_range.tp_max.dst);
+	min_val = ntohs(filter->key.tp_range.tp_min.dst);
+	max_val = ntohs(filter->key.tp_range.tp_max.dst);
 
 	if (min_mask && max_mask) {
-		if (htons(key->tp_range.tp.dst) < min_val ||
-		    htons(key->tp_range.tp.dst) > max_val)
+		if (ntohs(key->tp_range.tp.dst) < min_val ||
+		    ntohs(key->tp_range.tp.dst) > max_val)
 			return false;
 
 		/* skb does not have min and max values */
@@ -232,16 +232,16 @@ static bool fl_range_port_src_cmp(struct cls_fl_filter *filter,
 				  struct fl_flow_key *key,
 				  struct fl_flow_key *mkey)
 {
-	__be16 min_mask, max_mask, min_val, max_val;
+	u16 min_mask, max_mask, min_val, max_val;
 
-	min_mask = htons(filter->mask->key.tp_range.tp_min.src);
-	max_mask = htons(filter->mask->key.tp_range.tp_max.src);
-	min_val = htons(filter->key.tp_range.tp_min.src);
-	max_val = htons(filter->key.tp_range.tp_max.src);
+	min_mask = ntohs(filter->mask->key.tp_range.tp_min.src);
+	max_mask = ntohs(filter->mask->key.tp_range.tp_max.src);
+	min_val = ntohs(filter->key.tp_range.tp_min.src);
+	max_val = ntohs(filter->key.tp_range.tp_max.src);
 
 	if (min_mask && max_mask) {
-		if (htons(key->tp_range.tp.src) < min_val ||
-		    htons(key->tp_range.tp.src) > max_val)
+		if (ntohs(key->tp_range.tp.src) < min_val ||
+		    ntohs(key->tp_range.tp.src) > max_val)
 			return false;
 
 		/* skb does not have min and max values */
@@ -783,16 +783,16 @@ static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
 		       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)) {
+	    ntohs(key->tp_range.tp_max.dst) <=
+	    ntohs(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)) {
+	    ntohs(key->tp_range.tp_max.src) <=
+	    ntohs(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)");
-- 
2.25.1


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

* [PATCH net-next 2/2] net/sched: cls_flower: use nla_get_be32 for TCA_FLOWER_KEY_FLAGS
  2021-03-21 21:05 [PATCH net-next 1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports Vladimir Oltean
@ 2021-03-21 21:05 ` Vladimir Oltean
  2021-03-22 20:00 ` [PATCH net-next 1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Vladimir Oltean @ 2021-03-21 21:05 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Vladimir Oltean

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The existing code is functionally correct: iproute2 parses the ip_flags
argument for tc-flower and really packs it as big endian into the
TCA_FLOWER_KEY_FLAGS netlink attribute. But there is a problem in the
fact that W=1 builds complain:

net/sched/cls_flower.c:1047:15: warning: cast to restricted __be32

This is because we should use the dedicated helper for obtaining a
__be32 pointer to the netlink attribute, not a u32 one. This ensures
type correctness for be32_to_cpu.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/sched/cls_flower.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 832a0ece6dbf..9736df97e04d 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1044,8 +1044,8 @@ static int fl_set_key_flags(struct nlattr **tb, u32 *flags_key,
 		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]));
+	key = be32_to_cpu(nla_get_be32(tb[TCA_FLOWER_KEY_FLAGS]));
+	mask = be32_to_cpu(nla_get_be32(tb[TCA_FLOWER_KEY_FLAGS_MASK]));
 
 	*flags_key  = 0;
 	*flags_mask = 0;
-- 
2.25.1


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

* Re: [PATCH net-next 1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports
  2021-03-21 21:05 [PATCH net-next 1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports Vladimir Oltean
  2021-03-21 21:05 ` [PATCH net-next 2/2] net/sched: cls_flower: use nla_get_be32 for TCA_FLOWER_KEY_FLAGS Vladimir Oltean
@ 2021-03-22 20:00 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-03-22 20:00 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, netdev, jhs, xiyou.wangcong, jiri, vladimir.oltean

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Sun, 21 Mar 2021 23:05:48 +0200 you wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> A make W=1 build complains that:
> 
> net/sched/cls_flower.c:214:20: warning: cast from restricted __be16
> net/sched/cls_flower.c:214:20: warning: incorrect type in argument 1 (different base types)
> net/sched/cls_flower.c:214:20:    expected unsigned short [usertype] val
> net/sched/cls_flower.c:214:20:    got restricted __be16 [usertype] dst
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports
    https://git.kernel.org/netdev/net-next/c/6215afcb9a7e
  - [net-next,2/2] net/sched: cls_flower: use nla_get_be32 for TCA_FLOWER_KEY_FLAGS
    https://git.kernel.org/netdev/net-next/c/abee13f53e88

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2021-03-22 20:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-21 21:05 [PATCH net-next 1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports Vladimir Oltean
2021-03-21 21:05 ` [PATCH net-next 2/2] net/sched: cls_flower: use nla_get_be32 for TCA_FLOWER_KEY_FLAGS Vladimir Oltean
2021-03-22 20:00 ` [PATCH net-next 1/2] net/sched: cls_flower: use ntohs for struct flow_dissector_key_ports patchwork-bot+netdevbpf

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.