* [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
@ 2021-09-13 14:31 Linus Walleij
2021-09-13 16:05 ` Andrew Lunn
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Linus Walleij @ 2021-09-13 14:31 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S . Miller, Jakub Kicinski
Cc: netdev, Linus Walleij
This drops the code setting bit 9 on egress frames on the
Realtek "type A" (RTL8366RB) frames.
This bit was set on ingress frames for unknown reason,
and was set on egress frames as the format of ingress
and egress frames was believed to be the same. As that
assumption turned out to be false, and since this bit
seems to have zero effect on the behaviour of the switch
let's drop this bit entirely.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
net/dsa/tag_rtl4_a.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dsa/tag_rtl4_a.c b/net/dsa/tag_rtl4_a.c
index f920487ae145..6d928ee3ef7a 100644
--- a/net/dsa/tag_rtl4_a.c
+++ b/net/dsa/tag_rtl4_a.c
@@ -54,7 +54,7 @@ static struct sk_buff *rtl4a_tag_xmit(struct sk_buff *skb,
p = (__be16 *)tag;
*p = htons(RTL4_A_ETHERTYPE);
- out = (RTL4_A_PROTOCOL_RTL8366RB << RTL4_A_PROTOCOL_SHIFT) | (2 << 8);
+ out = (RTL4_A_PROTOCOL_RTL8366RB << RTL4_A_PROTOCOL_SHIFT);
/* The lower bits indicate the port number */
out |= BIT(dp->index);
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-13 14:31 [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames Linus Walleij
@ 2021-09-13 16:05 ` Andrew Lunn
2021-09-13 16:06 ` Florian Fainelli
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Andrew Lunn @ 2021-09-13 16:05 UTC (permalink / raw)
To: Linus Walleij
Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S . Miller, Jakub Kicinski, netdev
On Mon, Sep 13, 2021 at 04:31:56PM +0200, Linus Walleij wrote:
> This drops the code setting bit 9 on egress frames on the
> Realtek "type A" (RTL8366RB) frames.
>
> This bit was set on ingress frames for unknown reason,
> and was set on egress frames as the format of ingress
> and egress frames was believed to be the same. As that
> assumption turned out to be false, and since this bit
> seems to have zero effect on the behaviour of the switch
> let's drop this bit entirely.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-13 14:31 [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames Linus Walleij
2021-09-13 16:05 ` Andrew Lunn
@ 2021-09-13 16:06 ` Florian Fainelli
2021-09-15 3:10 ` patchwork-bot+netdevbpf
2021-09-15 7:19 ` DENG Qingfang
3 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2021-09-13 16:06 UTC (permalink / raw)
To: Linus Walleij, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S . Miller, Jakub Kicinski
Cc: netdev
On 9/13/2021 7:31 AM, Linus Walleij wrote:
> This drops the code setting bit 9 on egress frames on the
> Realtek "type A" (RTL8366RB) frames.
>
> This bit was set on ingress frames for unknown reason,
> and was set on egress frames as the format of ingress
> and egress frames was believed to be the same. As that
> assumption turned out to be false, and since this bit
> seems to have zero effect on the behaviour of the switch
> let's drop this bit entirely.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-13 14:31 [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames Linus Walleij
2021-09-13 16:05 ` Andrew Lunn
2021-09-13 16:06 ` Florian Fainelli
@ 2021-09-15 3:10 ` patchwork-bot+netdevbpf
2021-09-15 7:19 ` DENG Qingfang
3 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-09-15 3:10 UTC (permalink / raw)
To: Linus Walleij
Cc: andrew, vivien.didelot, f.fainelli, olteanv, davem, kuba, netdev
Hello:
This patch was applied to netdev/net-next.git (refs/heads/master):
On Mon, 13 Sep 2021 16:31:56 +0200 you wrote:
> This drops the code setting bit 9 on egress frames on the
> Realtek "type A" (RTL8366RB) frames.
>
> This bit was set on ingress frames for unknown reason,
> and was set on egress frames as the format of ingress
> and egress frames was believed to be the same. As that
> assumption turned out to be false, and since this bit
> seems to have zero effect on the behaviour of the switch
> let's drop this bit entirely.
>
> [...]
Here is the summary with links:
- [net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
https://git.kernel.org/netdev/net-next/c/339133f6c318
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] 11+ messages in thread* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-13 14:31 [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames Linus Walleij
` (2 preceding siblings ...)
2021-09-15 3:10 ` patchwork-bot+netdevbpf
@ 2021-09-15 7:19 ` DENG Qingfang
2021-09-23 21:59 ` Linus Walleij
3 siblings, 1 reply; 11+ messages in thread
From: DENG Qingfang @ 2021-09-15 7:19 UTC (permalink / raw)
To: Linus Walleij
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S . Miller, Jakub Kicinski, netdev
On Mon, Sep 13, 2021 at 04:31:56PM +0200, Linus Walleij wrote:
> This drops the code setting bit 9 on egress frames on the
> Realtek "type A" (RTL8366RB) frames.
FYI, on RTL8366S, bit 9 on egress frames is disable learning.
> This bit was set on ingress frames for unknown reason,
I think it could be the reason why the frame is forwarded to the CPU.
> and was set on egress frames as the format of ingress
> and egress frames was believed to be the same. As that
> assumption turned out to be false, and since this bit
> seems to have zero effect on the behaviour of the switch
> let's drop this bit entirely.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-15 7:19 ` DENG Qingfang
@ 2021-09-23 21:59 ` Linus Walleij
2021-09-23 22:12 ` Vladimir Oltean
0 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2021-09-23 21:59 UTC (permalink / raw)
To: DENG Qingfang
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S . Miller, Jakub Kicinski, netdev
On Wed, Sep 15, 2021 at 9:20 AM DENG Qingfang <dqfext@gmail.com> wrote:
> On Mon, Sep 13, 2021 at 04:31:56PM +0200, Linus Walleij wrote:
> > This drops the code setting bit 9 on egress frames on the
> > Realtek "type A" (RTL8366RB) frames.
>
> FYI, on RTL8366S, bit 9 on egress frames is disable learning.
>
> > This bit was set on ingress frames for unknown reason,
>
> I think it could be the reason why the frame is forwarded to the CPU.
Hm I suspect it disable learning on RTL8366RB as well.
Do we have some use for that feature in DSA taggers?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-23 21:59 ` Linus Walleij
@ 2021-09-23 22:12 ` Vladimir Oltean
2021-09-23 22:21 ` Linus Walleij
0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2021-09-23 22:12 UTC (permalink / raw)
To: Linus Walleij
Cc: DENG Qingfang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S . Miller, Jakub Kicinski, netdev
On Thu, Sep 23, 2021 at 11:59:36PM +0200, Linus Walleij wrote:
> On Wed, Sep 15, 2021 at 9:20 AM DENG Qingfang <dqfext@gmail.com> wrote:
> > On Mon, Sep 13, 2021 at 04:31:56PM +0200, Linus Walleij wrote:
>
> > > This drops the code setting bit 9 on egress frames on the
> > > Realtek "type A" (RTL8366RB) frames.
> >
> > FYI, on RTL8366S, bit 9 on egress frames is disable learning.
> >
> > > This bit was set on ingress frames for unknown reason,
> >
> > I think it could be the reason why the frame is forwarded to the CPU.
>
> Hm I suspect it disable learning on RTL8366RB as well.
Suspicion based on what?
> Do we have some use for that feature in DSA taggers?
Yes.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-23 22:12 ` Vladimir Oltean
@ 2021-09-23 22:21 ` Linus Walleij
2021-09-23 22:25 ` Vladimir Oltean
0 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2021-09-23 22:21 UTC (permalink / raw)
To: Vladimir Oltean
Cc: DENG Qingfang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S . Miller, Jakub Kicinski, netdev
On Fri, Sep 24, 2021 at 12:12 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> > Hm I suspect it disable learning on RTL8366RB as well.
>
> Suspicion based on what?
I have a not yet finished patch that dumps the FDB :)
The contents change around a bit under the patch
sets I have floating, but can certainly be determined
when I have time to test things properly.
> > Do we have some use for that feature in DSA taggers?
>
> Yes.
OK I'll add it to my TODO, right now trying to fix up the base
of the RTL8366RB patch set to handle VLANs the right way.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-23 22:21 ` Linus Walleij
@ 2021-09-23 22:25 ` Vladimir Oltean
2021-09-23 22:58 ` Linus Walleij
0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2021-09-23 22:25 UTC (permalink / raw)
To: Linus Walleij
Cc: DENG Qingfang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S . Miller, Jakub Kicinski, netdev
On Fri, Sep 24, 2021 at 12:21:39AM +0200, Linus Walleij wrote:
> On Fri, Sep 24, 2021 at 12:12 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> > > Hm I suspect it disable learning on RTL8366RB as well.
> >
> > Suspicion based on what?
>
> I have a not yet finished patch that dumps the FDB :)
>
> The contents change around a bit under the patch
> sets I have floating, but can certainly be determined
> when I have time to test things properly.
To be clear, if bit 9 is a "disable learning" bit, address learning is a
process that takes place on the ingress of a packet, and in this case
the ingress port is the CPU port. But the ndo_fdb_dump works with net
devices, of which CPU ports have none. So it seems unlikely that you
would see any difference in the output of "bridge fdb" that could be
attributed to that bit.
> > > Do we have some use for that feature in DSA taggers?
> >
> > Yes.
>
> OK I'll add it to my TODO, right now trying to fix up the base
> of the RTL8366RB patch set to handle VLANs the right way.
But you didn't ask what that use is...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-23 22:25 ` Vladimir Oltean
@ 2021-09-23 22:58 ` Linus Walleij
2021-09-23 23:11 ` Vladimir Oltean
0 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2021-09-23 22:58 UTC (permalink / raw)
To: Vladimir Oltean
Cc: DENG Qingfang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S . Miller, Jakub Kicinski, netdev
On Fri, Sep 24, 2021 at 12:25 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> On Fri, Sep 24, 2021 at 12:21:39AM +0200, Linus Walleij wrote:
> > > > Do we have some use for that feature in DSA taggers?
> > >
> > > Yes.
> >
> > OK I'll add it to my TODO, right now trying to fix up the base
> > of the RTL8366RB patch set to handle VLANs the right way.
>
> But you didn't ask what that use is...
Allright spill the beans :D I might not be the most clever at times...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames
2021-09-23 22:58 ` Linus Walleij
@ 2021-09-23 23:11 ` Vladimir Oltean
0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Oltean @ 2021-09-23 23:11 UTC (permalink / raw)
To: Linus Walleij
Cc: DENG Qingfang, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S . Miller, Jakub Kicinski, netdev
On Fri, Sep 24, 2021 at 12:58:00AM +0200, Linus Walleij wrote:
> On Fri, Sep 24, 2021 at 12:25 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Fri, Sep 24, 2021 at 12:21:39AM +0200, Linus Walleij wrote:
> > > > > Do we have some use for that feature in DSA taggers?
> > > >
> > > > Yes.
> > >
> > > OK I'll add it to my TODO, right now trying to fix up the base
> > > of the RTL8366RB patch set to handle VLANs the right way.
> >
> > But you didn't ask what that use is...
>
> Allright spill the beans :D I might not be the most clever at times...
So the essence of the answer is in the discussion we've already had on
your v1 attempt to disable this "unknown" bit:
https://patchwork.kernel.org/project/netdevbpf/patch/20210828235619.249757-1-linus.walleij@linaro.org/
The idea is that bridges should learn only from data plane packets, and
the only chance a DSA tagger has to distinguish between a data and a
control packet sent by the network stack is to look at
skb->offload_fwd_mark, which will be set on xmit by the bridge driver,
if you implement the .port_bridge_tx_fwd_offload and .port_bridge_tx_fwd_unoffload
methods, and declare a non-zero ds->max_num_bridges value (note: the API
for bridge TX forwarding offload in DSA is subject to change/get
simplified during this development cycle). And to offload the replication
for data packets correctly, you should really re-read the discussion we
had about what happens when the egress port mask set by the tagger is all-zeroes.
Not to blow my own horn or anything, but I've repeated this stuff for so many
times now that I wrote it down for future reference, you can find the basic
concepts detailed in chapter "The data plane and the control plane" from the
paper attached here (there are also pictures, not sure how much they help):
https://linuxplumbersconf.org/event/11/contributions/949/
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-09-23 23:11 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 14:31 [PATCH net-next] net: dsa: tag_rtl4_a: Drop bit 9 from egress frames Linus Walleij
2021-09-13 16:05 ` Andrew Lunn
2021-09-13 16:06 ` Florian Fainelli
2021-09-15 3:10 ` patchwork-bot+netdevbpf
2021-09-15 7:19 ` DENG Qingfang
2021-09-23 21:59 ` Linus Walleij
2021-09-23 22:12 ` Vladimir Oltean
2021-09-23 22:21 ` Linus Walleij
2021-09-23 22:25 ` Vladimir Oltean
2021-09-23 22:58 ` Linus Walleij
2021-09-23 23:11 ` Vladimir Oltean
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.