All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] net: dsa: tag_rtl4_a: Fix egress tags
@ 2021-08-31 18:50 Linus Walleij
  2021-08-31 20:55 ` Florian Fainelli
  2021-09-01 10:50 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Linus Walleij @ 2021-08-31 18:50 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S . Miller, Jakub Kicinski
  Cc: netdev, Linus Walleij, DENG Qingfang, Mauri Sandberg

I noticed that only port 0 worked on the RTL8366RB since we
started to use custom tags.

It turns out that the format of egress custom tags is actually
different from ingress custom tags. While the lower bits just
contain the port number in ingress tags, egress tags need to
indicate destination port by setting the bit for the
corresponding port.

It was working on port 0 because port 0 added 0x00 as port
number in the lower bits, and if you do this the packet appears
at all ports, including the intended port. Ooops.

Fix this and all ports work again. Use the define for shifting
the "type A" into place while we're at it.

Tested on the D-Link DIR-685 by sending traffic to each of
the ports in turn. It works.

Fixes: 86dd9868b878 ("net: dsa: tag_rtl4_a: Support also egress tags")
Cc: DENG Qingfang <dqfext@gmail.com>
Cc: Mauri Sandberg <sandberg@mailfence.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Drop the removal of bit 9 (2 << 8) so as to not stir up unwanted
  side effects.
---
 net/dsa/tag_rtl4_a.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/dsa/tag_rtl4_a.c b/net/dsa/tag_rtl4_a.c
index 57c46b4ab2b3..e34b80fa52e1 100644
--- a/net/dsa/tag_rtl4_a.c
+++ b/net/dsa/tag_rtl4_a.c
@@ -54,9 +54,10 @@ static struct sk_buff *rtl4a_tag_xmit(struct sk_buff *skb,
 	p = (__be16 *)tag;
 	*p = htons(RTL4_A_ETHERTYPE);
 
-	out = (RTL4_A_PROTOCOL_RTL8366RB << 12) | (2 << 8);
-	/* The lower bits is the port number */
-	out |= (u8)dp->index;
+	out = (RTL4_A_PROTOCOL_RTL8366RB << RTL4_A_PROTOCOL_SHIFT) | (2 << 8);
+	/* The lower bits indicate the port number */
+	out |= BIT(dp->index);
+
 	p = (__be16 *)(tag + 2);
 	*p = htons(out);
 
-- 
2.31.1


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

* Re: [PATCH net v2] net: dsa: tag_rtl4_a: Fix egress tags
  2021-08-31 18:50 [PATCH net v2] net: dsa: tag_rtl4_a: Fix egress tags Linus Walleij
@ 2021-08-31 20:55 ` Florian Fainelli
  2021-09-01 10:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Florian Fainelli @ 2021-08-31 20:55 UTC (permalink / raw)
  To: Linus Walleij, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	David S . Miller, Jakub Kicinski
  Cc: netdev, DENG Qingfang, Mauri Sandberg



On 8/31/2021 11:50 AM, Linus Walleij wrote:
> I noticed that only port 0 worked on the RTL8366RB since we
> started to use custom tags.
> 
> It turns out that the format of egress custom tags is actually
> different from ingress custom tags. While the lower bits just
> contain the port number in ingress tags, egress tags need to
> indicate destination port by setting the bit for the
> corresponding port.
> 
> It was working on port 0 because port 0 added 0x00 as port
> number in the lower bits, and if you do this the packet appears
> at all ports, including the intended port. Ooops.
> 
> Fix this and all ports work again. Use the define for shifting
> the "type A" into place while we're at it.
> 
> Tested on the D-Link DIR-685 by sending traffic to each of
> the ports in turn. It works.
> 
> Fixes: 86dd9868b878 ("net: dsa: tag_rtl4_a: Support also egress tags")
> Cc: DENG Qingfang <dqfext@gmail.com>
> Cc: Mauri Sandberg <sandberg@mailfence.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

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

* Re: [PATCH net v2] net: dsa: tag_rtl4_a: Fix egress tags
  2021-08-31 18:50 [PATCH net v2] net: dsa: tag_rtl4_a: Fix egress tags Linus Walleij
  2021-08-31 20:55 ` Florian Fainelli
@ 2021-09-01 10:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-09-01 10:50 UTC (permalink / raw)
  To: Linus Walleij
  Cc: andrew, vivien.didelot, f.fainelli, olteanv, davem, kuba, netdev,
	dqfext, sandberg

Hello:

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

On Tue, 31 Aug 2021 20:50:50 +0200 you wrote:
> I noticed that only port 0 worked on the RTL8366RB since we
> started to use custom tags.
> 
> It turns out that the format of egress custom tags is actually
> different from ingress custom tags. While the lower bits just
> contain the port number in ingress tags, egress tags need to
> indicate destination port by setting the bit for the
> corresponding port.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: dsa: tag_rtl4_a: Fix egress tags
    https://git.kernel.org/netdev/net/c/0e90dfa7a8d8

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-09-01 10:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-31 18:50 [PATCH net v2] net: dsa: tag_rtl4_a: Fix egress tags Linus Walleij
2021-08-31 20:55 ` Florian Fainelli
2021-09-01 10: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.