From: Vladimir Oltean <olteanv@gmail.com>
To: DENG Qingfang <dqfext@gmail.com>
Cc: Sean Wang <sean.wang@mediatek.com>,
Landen Chao <Landen.Chao@mediatek.com>,
Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
Florian Fainelli <f.fainelli@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC net-next 1/2] net: dsa: tag_mtk: skip address learning on transmit to standalone ports
Date: Wed, 28 Jul 2021 21:37:05 +0300 [thread overview]
Message-ID: <20210728183705.4gea64qlbe64kkpl@skbuf> (raw)
In-Reply-To: <20210728175327.1150120-2-dqfext@gmail.com>
On Thu, Jul 29, 2021 at 01:53:25AM +0800, DENG Qingfang wrote:
> Consider the following bridge configuration, where bond0 is not
> offloaded:
>
> +-- br0 --+
> / / | \
> / / | \
> / | | bond0
> / | | / \
> swp0 swp1 swp2 swp3 swp4
> . . .
> . . .
> A B C
>
> Address learning is enabled on offloaded ports (swp0~2) and the CPU
> port, so when client A sends a packet to C, the following will happen:
>
> 1. The switch learns that client A can be reached at swp0.
> 2. The switch probably already knows that client C can be reached at the
> CPU port, so it forwards the packet to the CPU.
> 3. The bridge core knows client C can be reached at bond0, so it
> forwards the packet back to the switch.
> 4. The switch learns that client A can be reached at the CPU port.
> 5. The switch forwards the packet to either swp3 or swp4, according to
> the packet's tag.
>
> That makes client A's MAC address flap between swp0 and the CPU port. If
> client B sends a packet to A, it is possible that the packet is
> forwarded to the CPU. With offload_fwd_mark = 1, the bridge core won't
> forward it back to the switch, resulting in packet loss.
>
> To avoid that, skip address learning on the CPU port when the destination
> port is standalone, which can be done by setting the SA_DIS bit of the
> MTK tag, if bridge_dev of the destination port is not set.
>
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> net/dsa/tag_mtk.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
> index cc3ba864ad5b..8c361812e21b 100644
> --- a/net/dsa/tag_mtk.c
> +++ b/net/dsa/tag_mtk.c
> @@ -15,8 +15,7 @@
> #define MTK_HDR_XMIT_TAGGED_TPID_8100 1
> #define MTK_HDR_XMIT_TAGGED_TPID_88A8 2
> #define MTK_HDR_RECV_SOURCE_PORT_MASK GENMASK(2, 0)
> -#define MTK_HDR_XMIT_DP_BIT_MASK GENMASK(5, 0)
> -#define MTK_HDR_XMIT_SA_DIS BIT(6)
> +#define MTK_HDR_XMIT_SA_DIS_SHIFT 6
>
> static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
> struct net_device *dev)
> @@ -50,7 +49,8 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
> * whether that's a combined special tag with 802.1Q header.
> */
> mtk_tag[0] = xmit_tpid;
> - mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
Why stop AND-ing with MTK_HDR_XMIT_DP_BIT_MASK if you were doing that
before? If it's not needed (probably isn't), it would be nice to split
that up.
> + mtk_tag[1] = BIT(dp->index) |
> + (!dp->bridge_dev << MTK_HDR_XMIT_SA_DIS_SHIFT);
>
> /* Tag control information is kept for 802.1Q */
> if (xmit_tpid == MTK_HDR_XMIT_UNTAGGED) {
> --
> 2.25.1
>
Otherwise this is as correct as can be without implementing TX
forwarding offload for the bridge (which you've explained why it doesn't
map 1:1 with what your hw can do). But just because a port is under a bridge
doesn't mean that the only packets it sends belong to that bridge. Think
AF_PACKET sockets, PTP etc. The bridge also has a no_linklocal_learn
option that maybe should be taken into consideration for drivers that
can do something meaningful about it. Anyway, food for thought.
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
next prev parent reply other threads:[~2021-07-28 18:37 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-28 17:53 [RFC net-next 0/2] mt7530 software fallback bridging fix DENG Qingfang
2021-07-28 17:53 ` [RFC net-next 1/2] net: dsa: tag_mtk: skip address learning on transmit to standalone ports DENG Qingfang
2021-07-28 18:37 ` Vladimir Oltean [this message]
2021-07-30 16:24 ` Vladimir Oltean
2021-07-30 17:32 ` DENG Qingfang
2021-07-30 17:39 ` Vladimir Oltean
2021-07-30 17:41 ` Vladimir Oltean
2021-07-30 17:58 ` DENG Qingfang
2021-07-30 19:00 ` DENG Qingfang
2021-07-30 19:07 ` Vladimir Oltean
2021-07-30 19:25 ` DENG Qingfang
2021-07-30 19:30 ` Vladimir Oltean
2021-07-28 17:53 ` [RFC net-next 2/2] net: dsa: mt7530: trap packets from standalone ports to the CPU DENG Qingfang
2021-07-28 18:47 ` Vladimir Oltean
2021-07-29 15:28 ` Vladimir Oltean
2021-07-29 16:11 ` DENG Qingfang
2021-07-29 16:50 ` Vladimir Oltean
2021-07-30 15:45 ` DENG Qingfang
2021-07-30 15:58 ` DENG Qingfang
2021-07-30 16:18 ` Vladimir Oltean
2021-07-30 17:21 ` DENG Qingfang
2021-07-30 17:35 ` Vladimir Oltean
2021-07-30 17:51 ` DENG Qingfang
2021-07-28 18:08 ` [RFC net-next 0/2] mt7530 software fallback bridging fix Vladimir Oltean
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210728183705.4gea64qlbe64kkpl@skbuf \
--to=olteanv@gmail.com \
--cc=Landen.Chao@mediatek.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=dqfext@gmail.com \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=sean.wang@mediatek.com \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).