public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Tobias Waldekranz <tobias@waldekranz.com>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	"UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
	Paolo Abeni <pabeni@redhat.com>, Roopa Prabhu <roopa@nvidia.com>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	Jiri Pirko <jiri@nvidia.com>, Ido Schimmel <idosch@nvidia.com>,
	Mattias Forsblad <mattias.forsblad@gmail.com>,
	Joachim Wiberg <troglobit@gmail.com>
Subject: Re: [PATCH net-next 0/6] Disable host flooding for DSA ports under a bridge
Date: Mon, 11 Apr 2022 17:24:02 +0300	[thread overview]
Message-ID: <20220411142402.vhuctrmlyezklg4n@skbuf> (raw)
In-Reply-To: <20220411135530.2nu4rlofyd6nphb2@skbuf>

On Mon, Apr 11, 2022 at 04:55:30PM +0300, Vladimir Oltean wrote:
> > > IFF_ALLMULTI would implicitly correspond to BR_MCAST_FLOOD (ok, also
> > > adjust for mcast_router ports).
> > 
> > IFF flags shouldn't correspond to any bridge flags. They describe higher
> > level features. Therefore, they should be passed down to the drivers,
> > who in many cases may decide to use hardware resources that are shared
> > with bridge flags (i.e. flood controls), but in some cases may be able
> > to do something better.
> > 
> > As an example of "something better": some ASICs have separate flooding
> > controls for IP and non-IP multicast.
> > 
> > So, I think
> > 
> >     ip link set dev br0 allmulticast on
> > 
> > Should be one way for userspace to tell the bridge to mark the host port
> > as a permanent multicast router port. This in turn would trigger a
> > 
> >     switchdev_port_attr_set(dev,
> >     	&{ .id = SWITCHDEV_ATTR_ID_BRIDGE_MROUTER ... }, extack);
> > 
> > At the DSA layer this info would be passed to the driver, which will
> > decide if that means the same thing as BR_MCAST_FLOOD or something
> > else.
> 
> Yeah, I don't think so. Doing nothing at all is way better than
> entangling the RX filtering logic even more with the forwarding logic,
> IMHO.

Thinking out loud.
I still maintain it is weird and uncalled for if "ip link set dev br0 allmulticast on"
marks the bridge as a multicast router (maybe I'm wrong, but I don't see why it is helpful).
But the other way around may not be so weird. That is, as long as the
bridge is a multicast router port or needs to receive unknown multicast
for any reason at all, it auto-enables IFF_ALLMULTI on its RX flags.

Then, we just need to take care of that "RX filter is a mirror" thing.
Not ideal, but maybe if we actually introduce a bridge flag "rx_filter_mirror"
where the default is 1 to keep backwards compatibility, we could actually
turn the bridge into something sane?
The bad part is that we can't make switchdev drivers reject a bridge
with rx_filter_mirror=1, unless we're ready to deal with the breakage
(although even that is tempting...).

What this effort would achieve is that the bridge would no longer become
promiscuous in the presence of uppers with different MAC address.
It would do this by implementing the filtering you talked about in
br_pass_frame_up().

As for offloading, the nice part is that the bridge RX filtering logic
could be left as invisible to switchdev, and we could concentrate only
on flooding towards the host via the logic that Joachim is working on.

      reply	other threads:[~2022-04-11 14:24 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-08 20:03 [PATCH net-next 0/6] Disable host flooding for DSA ports under a bridge Vladimir Oltean
2022-04-08 20:03 ` [PATCH net-next 1/6] net: refactor all NETDEV_CHANGE notifier calls to a single function Vladimir Oltean
2022-04-08 20:03 ` [PATCH net-next 2/6] net: emit NETDEV_CHANGE for changes to IFF_PROMISC | IFF_ALLMULTI Vladimir Oltean
2022-04-08 20:03 ` [PATCH net-next 3/6] net: dsa: walk through all changeupper notifier functions Vladimir Oltean
2022-04-08 20:03 ` [PATCH net-next 4/6] net: dsa: track whether bridges have foreign interfaces in them Vladimir Oltean
2022-04-08 20:03 ` [PATCH net-next 5/6] net: dsa: monitor changes to bridge promiscuity Vladimir Oltean
2022-04-08 20:03 ` [PATCH net-next 6/6] net: bridge: avoid uselessly making offloaded ports promiscuous Vladimir Oltean
2022-04-09 10:17   ` kernel test robot
2022-04-09 11:04     ` Vladimir Oltean
2022-04-14 13:53   ` kernel test robot
2022-04-08 20:14 ` [PATCH net-next 0/6] Disable host flooding for DSA ports under a bridge Vladimir Oltean
2022-04-08 20:52   ` Nikolay Aleksandrov
2022-04-09 19:46 ` Tobias Waldekranz
2022-04-09 20:45   ` Vladimir Oltean
2022-04-10 18:02     ` Tobias Waldekranz
2022-04-10 22:03       ` Vladimir Oltean
2022-04-11  9:33         ` Tobias Waldekranz
2022-04-11 10:55           ` Vladimir Oltean
2022-04-11 13:14             ` Tobias Waldekranz
2022-04-11 13:55               ` Vladimir Oltean
2022-04-11 14:24                 ` Vladimir Oltean [this message]

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=20220411142402.vhuctrmlyezklg4n@skbuf \
    --to=olteanv@gmail.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=idosch@nvidia.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=mattias.forsblad@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=roopa@nvidia.com \
    --cc=tobias@waldekranz.com \
    --cc=troglobit@gmail.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.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