All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net: mscc: ocelot: avoid type promotion when calling ocelot_ifh_set_dest
@ 2021-02-15 13:31 Vladimir Oltean
  2021-02-15 20:50 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 2+ messages in thread
From: Vladimir Oltean @ 2021-02-15 13:31 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, Florian Fainelli, Vivien Didelot, Claudiu Manoil,
	Alexandre Belloni, Vladimir Oltean, UNGLinuxDriver,
	Dan Carpenter

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

Smatch is confused by the fact that a 32-bit BIT(port) macro is passed
as argument to the ocelot_ifh_set_dest function and warns:

ocelot_xmit() warn: should '(((1))) << (dp->index)' be a 64 bit type?
seville_xmit() warn: should '(((1))) << (dp->index)' be a 64 bit type?

The destination port mask is copied into a 12-bit field of the packet,
starting at bit offset 67 and ending at 56.

So this DSA tagging protocol supports at most 12 bits, which is clearly
less than 32. Attempting to send to a port number > 12 will cause the
packing() call to truncate way before there will be 32-bit truncation
due to type promotion of the BIT(port) argument towards u64.

Therefore, smatch's fears that BIT(port) will do the wrong thing and
cause unexpected truncation for "port" values >= 32 are unfounded.
Nonetheless, let's silence the warning by explicitly passing an u64
value to ocelot_ifh_set_dest, such that the compiler does not need to do
a questionable type promotion.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot.c | 2 +-
 net/dsa/tag_ocelot.c               | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 8d97c731e953..5d13087c85d6 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -803,7 +803,7 @@ void ocelot_port_inject_frame(struct ocelot *ocelot, int port, int grp,
 			 QS_INJ_CTRL_SOF, QS_INJ_CTRL, grp);
 
 	ocelot_ifh_set_bypass(ifh, 1);
-	ocelot_ifh_set_dest(ifh, BIT(port));
+	ocelot_ifh_set_dest(ifh, BIT_ULL(port));
 	ocelot_ifh_set_tag_type(ifh, IFH_TAG_TYPE_C);
 	ocelot_ifh_set_vid(ifh, skb_vlan_tag_get(skb));
 	ocelot_ifh_set_rew_op(ifh, rew_op);
diff --git a/net/dsa/tag_ocelot.c b/net/dsa/tag_ocelot.c
index a7dd61c8e005..f9df9cac81c5 100644
--- a/net/dsa/tag_ocelot.c
+++ b/net/dsa/tag_ocelot.c
@@ -56,7 +56,7 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb,
 	void *injection;
 
 	ocelot_xmit_common(skb, netdev, cpu_to_be32(0x8880000a), &injection);
-	ocelot_ifh_set_dest(injection, BIT(dp->index));
+	ocelot_ifh_set_dest(injection, BIT_ULL(dp->index));
 
 	return skb;
 }
@@ -68,7 +68,7 @@ static struct sk_buff *seville_xmit(struct sk_buff *skb,
 	void *injection;
 
 	ocelot_xmit_common(skb, netdev, cpu_to_be32(0x88800005), &injection);
-	seville_ifh_set_dest(injection, BIT(dp->index));
+	seville_ifh_set_dest(injection, BIT_ULL(dp->index));
 
 	return skb;
 }
-- 
2.25.1


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

* Re: [PATCH net-next] net: mscc: ocelot: avoid type promotion when calling ocelot_ifh_set_dest
  2021-02-15 13:31 [PATCH net-next] net: mscc: ocelot: avoid type promotion when calling ocelot_ifh_set_dest Vladimir Oltean
@ 2021-02-15 20:50 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-15 20:50 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, netdev, andrew, f.fainelli, vivien.didelot,
	claudiu.manoil, alexandre.belloni, vladimir.oltean,
	UNGLinuxDriver, dan.carpenter

Hello:

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

On Mon, 15 Feb 2021 15:31:43 +0200 you wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Smatch is confused by the fact that a 32-bit BIT(port) macro is passed
> as argument to the ocelot_ifh_set_dest function and warns:
> 
> ocelot_xmit() warn: should '(((1))) << (dp->index)' be a 64 bit type?
> seville_xmit() warn: should '(((1))) << (dp->index)' be a 64 bit type?
> 
> [...]

Here is the summary with links:
  - [net-next] net: mscc: ocelot: avoid type promotion when calling ocelot_ifh_set_dest
    https://git.kernel.org/netdev/net-next/c/1f778d500df3

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] 2+ messages in thread

end of thread, other threads:[~2021-02-15 20:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 13:31 [PATCH net-next] net: mscc: ocelot: avoid type promotion when calling ocelot_ifh_set_dest Vladimir Oltean
2021-02-15 20:50 ` 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.