Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
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>

  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 \
    --subject='Re: [RFC net-next 1/2] net: dsa: tag_mtk: skip address learning on transmit to standalone ports' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox