public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: "Arınç ÜNAL" <arinc.unal@arinc9.com>
To: Vladimir Oltean <olteanv@gmail.com>,
	DENG Qingfang <dqfext@gmail.com>, Greg Ungerer <gerg@kernel.org>
Cc: Daniel Golle <daniel@makrotopia.org>,
	Richard van Schagen <richard@routerhints.com>,
	Richard van Schagen <vschagen@cs.com>,
	Frank Wunderlich <frank-w@public-files.de>,
	erkin.bozoglu@xeront.com, bartel.eerdekens@constell8.be,
	netdev <netdev@vger.kernel.org>
Subject: MT7530 bug, forward broadcast and unknown frames to the correct CPU port
Date: Sun, 23 Apr 2023 18:22:41 +0300	[thread overview]
Message-ID: <8a955c34-5724-af9d-d828-a8786bcc08b0@arinc9.com> (raw)

Hey there folks,

On mt753x_cpu_port_enable() there's code [0] that sets which port to 
forward the broadcast, unknown multicast, and unknown unicast frames to. 
Since mt753x_cpu_port_enable() runs twice when both CPU ports are 
enabled, port 6 becomes the port to forward the frames to. But port 5 is 
the active port, so no broadcast frames received from the user ports 
will be forwarded to port 5. This breaks network connectivity when 
multiple ports are being used as CPU ports.

My testing shows that only after receiving a broadcast ARP frame from 
port 5 then forwarding it to the user port, the unicast frames received 
from that user port will be forwarded to port 5. I tested this with ping.

Forwarding broadcast and unknown unicast frames to the CPU port was done 
with commit 5a30833b9a16 ("net: dsa: mt7530: support MDB and bridge flag 
operations"). I suppose forwarding the broadcast frames only to the CPU 
port is what "disable flooding" here means.

It’s a mystery to me how the switch classifies multicast and unicast 
frames as unknown. Bartel's testing showed LLDP frames fall under this 
category.

Until the driver supports changing the DSA conduit, unknown frames 
should be forwarded to the active CPU port, not the numerically greater 
one. Any ideas how to address this and the "disable flooding" case?

There's also this "set the CPU number" code that runs only for MT7621. 
I'm not sure why this is needed or why it's only needed for MT7621. 
Greg, could you shed some light on this since you added this code with 
commit ddda1ac116c8 ("net: dsa: mt7530: support the 7530 switch on the 
Mediatek MT7621 SoC")?

There're more things to discuss after supporting changing the DSA 
conduit, such as which CPU port to forward the unknown frames to, when 
user ports under different conduits receive unknown frames. What makes 
sense to me is, if there are multiple CPU ports being used, forward the 
unknown frames to port 6. This is already the case except the code runs 
twice. If not, set it to whatever 'int port' is, which is the default 
behaviour already.

[0] 
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.c#n1005

Arınç

             reply	other threads:[~2023-04-23 15:23 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-23 15:22 Arınç ÜNAL [this message]
2023-04-26 20:54 ` MT7530 bug, forward broadcast and unknown frames to the correct CPU port Vladimir Oltean
2023-04-28 13:31   ` Arınç ÜNAL
2023-04-29 13:03     ` Arınç ÜNAL
2023-04-29 17:35       ` Vladimir Oltean
2023-04-29 18:39         ` Arınç ÜNAL
2023-04-29 18:44           ` Arınç ÜNAL
2023-04-29 18:56           ` Vladimir Oltean
2023-04-29 19:52             ` Arınç ÜNAL
2023-05-01 10:09               ` Vladimir Oltean
2023-05-01 10:31                 ` Daniel Golle
2023-05-01 10:43                   ` Arınç ÜNAL
2023-05-10  8:59                     ` Arınç ÜNAL
2023-05-10 14:02                       ` Vladimir Oltean
2023-05-16 20:01                         ` Arınç ÜNAL
2023-05-17 15:52                           ` 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=8a955c34-5724-af9d-d828-a8786bcc08b0@arinc9.com \
    --to=arinc.unal@arinc9.com \
    --cc=bartel.eerdekens@constell8.be \
    --cc=daniel@makrotopia.org \
    --cc=dqfext@gmail.com \
    --cc=erkin.bozoglu@xeront.com \
    --cc=frank-w@public-files.de \
    --cc=gerg@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=richard@routerhints.com \
    --cc=vschagen@cs.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