Messages in this thread | | | Date | Sun, 8 Aug 2021 01:57:21 +0300 | From | Vladimir Oltean <> | Subject | Re: [RFC net-next 3/3] net: dsa: tag_qca: set offload_fwd_mark |
| |
On Sat, Aug 07, 2021 at 08:07:26PM +0800, DENG Qingfang wrote: > As we offload flooding and forwarding, set offload_fwd_mark according to > Atheros header's type. > > Signed-off-by: DENG Qingfang <dqfext@gmail.com> > --- > net/dsa/tag_qca.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/net/dsa/tag_qca.c b/net/dsa/tag_qca.c > index 6e3136990491..ee5c1fdfef47 100644 > --- a/net/dsa/tag_qca.c > +++ b/net/dsa/tag_qca.c > @@ -50,7 +50,7 @@ static struct sk_buff *qca_tag_xmit(struct sk_buff *skb, struct net_device *dev) > > static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev) > { > - u8 ver; > + u8 ver, type; > u16 hdr; > int port; > __be16 *phdr; > @@ -82,6 +82,15 @@ static struct sk_buff *qca_tag_rcv(struct sk_buff *skb, struct net_device *dev) > if (!skb->dev) > return NULL; > > + type = (hdr & QCA_HDR_RECV_TYPE_MASK) >> QCA_HDR_RECV_TYPE_S; > + switch (type) { > + case 0x00: /* Normal packet */ > + case 0x19: /* Flooding to CPU */ > + case 0x1a: /* Forwarding to CPU */ > + dsa_default_offload_fwd_mark(skb); > + break; > + } > + > return skb; > } > > -- > 2.25.1 >
In this day and age, I consider this commit to be a bug fix, since the software bridge, seeing an skb with offload_fwd_mark = false on an offloaded port, will think it hasn't been forwarded and do that job itself. So all broadcast and multicast traffic flooded to the CPU will end up being transmitted with duplicates on the other bridge ports.
When the qca8k tagger was added in 2016 in commit cafdc45c949b ("net-next: dsa: add Qualcomm tag RX/TX handler"), the offload_fwd_mark framework was already there, but no DSA driver was using it - the first commit I can find that uses offload_fwd_mark in DSA is f849772915e5 ("net: dsa: lan9303: lan9303_rcv set skb->offload_fwd_mark") in 2017, and then quite a few more followed suit. But you could still blame commit cafdc45c949b.
Curious, I also see that the gswip driver is in the same situation: it implements .port_bridge_join but does not set skb->offload_fwd_mark. I've copied Hauke Mehrtens to make him aware. I would rather not send the patch myself because I would do a rather lousy job and set it unconditionally to 'true', but the hardware can probably do better in informing the tagger about whether a frame was received only by the host or not, since it has an 8 byte header on RX.
For the record, I've checked the other tagging drivers too, to see who else does not set skb->offload_fwd_mark, and they all correspond to switch drivers which don't implement .port_bridge_join, which in that case would be the correct thing to do.
| |