| Message ID | 20241211124657.1357330-1-robert.hodaszi@digi.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: tag_ocelot_8021q: fix broken reception | expand |
2024. 12. 11. 14:34 keltezéssel, Andrew Lunn írta: > [EXTERNAL E-MAIL] Warning! This email originated outside of the organization! Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > On Wed, Dec 11, 2024 at 01:46:56PM +0100, Robert Hodaszi wrote: >> Commit dcfe7673787b4bfea2c213df443d312aa754757b ("net: dsa: tag_sja1105: >> absorb logic for not overwriting precise info into dsa_8021q_rcv()") >> added support to let the DSA switch driver set source_port and >> switch_id. tag_8021q's logic overrides the previously set source_port >> and switch_id only if they are marked as "invalid" (-1). sja1105 and >> vsc73xx drivers are doing that properly, but ocelot_8021q driver doesn't >> initialize those variables. That causes dsa_8021q_rcv() doesn't set >> them, and they remain unassigned. >> >> Initialize them as invalid to so dsa_8021q_rcv() can return with the >> proper values. > Hi Robert > > Since this is a fix, it needs a Fixes: tag. Please also base it on net. > > There is more here: > > https://linkprotect.cudasvc.com/url?a=https%3a%2f%2fwww.kernel.org%2fdoc%2fhtml%2flatest%2fprocess%2fmaintainer-netdev.html&c=E,1,4wyDgaiEANqhFn2GG0tcaR1j57JQtD08e0Ct4VSwo05Uu1yGRm73RMgwy0gs_ClxX5Mmerf2f6bxOoXZF9G_zKOP4Isr6iVcIkPhTzyMVHt9DNUF0KUfXCE,&typo=1 > > Andrew Hi, OK, thanks! Let me try again! Robert
On Wed, Dec 11, 2024 at 01:46:56PM +0100, Robert Hodaszi wrote: > Commit dcfe7673787b4bfea2c213df443d312aa754757b ("net: dsa: tag_sja1105: > absorb logic for not overwriting precise info into dsa_8021q_rcv()") > added support to let the DSA switch driver set source_port and > switch_id. tag_8021q's logic overrides the previously set source_port > and switch_id only if they are marked as "invalid" (-1). sja1105 and > vsc73xx drivers are doing that properly, but ocelot_8021q driver doesn't > initialize those variables. That causes dsa_8021q_rcv() doesn't set > them, and they remain unassigned. > > Initialize them as invalid to so dsa_8021q_rcv() can return with the > proper values. > > Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> > Cc: stable@vger.kernel.org > --- Thank you for the patch, and sorry for the breakage. I suggest the following alternative commit message: The blamed commit changed the dsa_8021q_rcv() calling convention to accept pre-populated source_port and switch_id arguments. If those are not available, as in the case of tag_ocelot_8021q, the arguments must be preinitialized with -1. Due to the bug of passing uninitialized arguments, dsa_8021q_rcv() does not detect that it needs to populate the source_port and switch_id, and this makes dsa_conduit_find_user() fail, and leads to packet loss on reception. Fixes: dcfe7673787b ("net: dsa: tag_sja1105: absorb logic for not overwriting precise info into dsa_8021q_rcv()") Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> Cc: stable@vger.kernel.org
2024. 12. 11. 15:37 keltezéssel, Vladimir Oltean írta: > [EXTERNAL E-MAIL] Warning! This email originated outside of the organization! Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > > On Wed, Dec 11, 2024 at 01:46:56PM +0100, Robert Hodaszi wrote: >> Commit dcfe7673787b4bfea2c213df443d312aa754757b ("net: dsa: tag_sja1105: >> absorb logic for not overwriting precise info into dsa_8021q_rcv()") >> added support to let the DSA switch driver set source_port and >> switch_id. tag_8021q's logic overrides the previously set source_port >> and switch_id only if they are marked as "invalid" (-1). sja1105 and >> vsc73xx drivers are doing that properly, but ocelot_8021q driver doesn't >> initialize those variables. That causes dsa_8021q_rcv() doesn't set >> them, and they remain unassigned. >> >> Initialize them as invalid to so dsa_8021q_rcv() can return with the >> proper values. >> >> Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> >> Cc: stable@vger.kernel.org >> --- > Thank you for the patch, and sorry for the breakage. I suggest the > following alternative commit message: > > The blamed commit changed the dsa_8021q_rcv() calling convention to > accept pre-populated source_port and switch_id arguments. If those are > not available, as in the case of tag_ocelot_8021q, the arguments must > be preinitialized with -1. > > Due to the bug of passing uninitialized arguments, dsa_8021q_rcv() > does not detect that it needs to populate the source_port and > switch_id, and this makes dsa_conduit_find_user() fail, and leads to > packet loss on reception. > > Fixes: dcfe7673787b ("net: dsa: tag_sja1105: absorb logic for not overwriting precise info into dsa_8021q_rcv()") > Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> > Cc: stable@vger.kernel.org Yes, that commit message makes much more sense than mine. Unfortunately, I already sent out the new patch. I rewrite it, and send out a v2 then. Robert
diff --git a/net/dsa/tag_ocelot_8021q.c b/net/dsa/tag_ocelot_8021q.c index 8e8b1bef6af6..11ea8cfd6266 100644 --- a/net/dsa/tag_ocelot_8021q.c +++ b/net/dsa/tag_ocelot_8021q.c @@ -79,7 +79,7 @@ static struct sk_buff *ocelot_xmit(struct sk_buff *skb, static struct sk_buff *ocelot_rcv(struct sk_buff *skb, struct net_device *netdev) { - int src_port, switch_id; + int src_port = -1, switch_id = -1; dsa_8021q_rcv(skb, &src_port, &switch_id, NULL, NULL);
Commit dcfe7673787b4bfea2c213df443d312aa754757b ("net: dsa: tag_sja1105: absorb logic for not overwriting precise info into dsa_8021q_rcv()") added support to let the DSA switch driver set source_port and switch_id. tag_8021q's logic overrides the previously set source_port and switch_id only if they are marked as "invalid" (-1). sja1105 and vsc73xx drivers are doing that properly, but ocelot_8021q driver doesn't initialize those variables. That causes dsa_8021q_rcv() doesn't set them, and they remain unassigned. Initialize them as invalid to so dsa_8021q_rcv() can return with the proper values. Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> Cc: stable@vger.kernel.org --- net/dsa/tag_ocelot_8021q.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)