Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: DENG Qingfang <dqfext@gmail.com>, Hauke Mehrtens <hauke@hauke-m.de>
Cc: "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>,
	"Russell King" <linux@armlinux.org.uk>,
	"open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	"Ansuel Smith" <ansuelsmth@gmail.com>,
	"Jonathan McDowell" <noodles@earth.li>,
	"Michal Vokáč" <vokac.m@gmail.com>,
	"Christian Lamparter" <chunkeey@gmail.com>,
	"Nishka Dasgupta" <nishkadg.linux@gmail.com>,
	"Xiaofei Shen" <xiaofeis@codeaurora.org>,
	"John Crispin" <john@phrozen.org>,
	"Stefan Lippers-Hollmann" <s.l-h@gmx.de>,
	"Hannu Nyman" <hannu.nyman@iki.fi>,
	"Imran Khan" <gururug@gmail.com>,
	"Frank Wunderlich" <frank-w@public-files.de>,
	"Nick Lowe" <nick.lowe@gmail.com>,
	"André Valentin" <avalentin@vmh.kalnet.hooya.de>
Subject: Re: [RFC net-next 3/3] net: dsa: tag_qca: set offload_fwd_mark
Date: Sun, 8 Aug 2021 01:57:21 +0300	[thread overview]
Message-ID: <20210807225721.xk5q6osyqoqjmhmp@skbuf> (raw)
In-Reply-To: <20210807120726.1063225-4-dqfext@gmail.com>

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.

  reply	other threads:[~2021-08-07 22:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-07 12:07 [RFC net-next 0/3] qca8k bridge flags offload DENG Qingfang
2021-08-07 12:07 ` [RFC net-next 1/3] net: dsa: qca8k: offload bridge flags DENG Qingfang
2021-08-07 20:45   ` Vladimir Oltean
2021-08-07 12:07 ` [RFC net-next 2/3] net: dsa: qca8k: enable assisted learning on CPU port DENG Qingfang
2021-08-07 22:25   ` Vladimir Oltean
2021-08-08 16:05     ` DENG Qingfang
2021-08-08 21:10       ` Vladimir Oltean
2021-08-10 17:27       ` Andre Valentin
2021-08-10 17:53         ` Vladimir Oltean
2021-08-10 21:09           ` Andre Valentin
2021-08-10 23:33             ` Vladimir Oltean
2021-08-07 12:07 ` [RFC net-next 3/3] net: dsa: tag_qca: set offload_fwd_mark DENG Qingfang
2021-08-07 22:57   ` Vladimir Oltean [this message]
2021-08-08 16:12     ` DENG Qingfang
2021-08-08 21:14       ` Vladimir Oltean
2021-08-10  6:57 ` [RFC net-next 0/3] qca8k bridge flags offload Jonathan McDowell

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=20210807225721.xk5q6osyqoqjmhmp@skbuf \
    --to=olteanv@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=ansuelsmth@gmail.com \
    --cc=avalentin@vmh.kalnet.hooya.de \
    --cc=chunkeey@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=f.fainelli@gmail.com \
    --cc=frank-w@public-files.de \
    --cc=gururug@gmail.com \
    --cc=hannu.nyman@iki.fi \
    --cc=hauke@hauke-m.de \
    --cc=john@phrozen.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=nick.lowe@gmail.com \
    --cc=nishkadg.linux@gmail.com \
    --cc=noodles@earth.li \
    --cc=s.l-h@gmx.de \
    --cc=vivien.didelot@gmail.com \
    --cc=vokac.m@gmail.com \
    --cc=xiaofeis@codeaurora.org \
    --subject='Re: [RFC net-next 3/3] net: dsa: tag_qca: set offload_fwd_mark' \
    /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
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).