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(-)
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);
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://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
Andrew
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%2fh...
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
linux-stable-mirror@lists.linaro.org
-
Andrew Lunn -
Robert Hodaszi -
Vladimir Oltean