Re: [PATCH v8 net-next 10/12] net: dsa: mv88e6xxx: mac-auth/MAB implementation

From: netdev
Date: Fri Oct 21 2022 - 09:17:32 EST


On 2022-10-21 13:22, Vladimir Oltean wrote:
On Fri, Oct 21, 2022 at 08:47:42AM +0200, netdev@xxxxxxxxxxxxxxxxxxxx wrote:
On 2022-10-21 00:57, Vladimir Oltean wrote:
> On Thu, Oct 20, 2022 at 10:20:50PM +0200, netdev@xxxxxxxxxxxxxxxxxxxx
> wrote:
> > In general locked ports block traffic from a host based on if there
> > is a
> > FDB entry or not. In the non-offloaded case, there is only CPU
> > assisted
> > learning, so the normal learning mechanism has to be disabled as any
> > learned entry will open the port for the learned MAC,vlan.
>
> Does it have to be that way? Why can't BR_LEARNING on a BR_PORT_LOCKED
> cause the learned FDB entries to have BR_FDB_LOCKED, and everything
> would be ok in that case (the port will not be opened for the learned
> MAC/VLAN)?

I suppose you are right that basing it solely on BR_FDB_LOCKED is possible.

The question is then maybe if the common case where you don't need learned
entries for the scheme to work, e.g. with EAPOL link local packets, requires
less CPU load to work and is cleaner than if using BR_FDB_LOCKED entries?

I suppose the real question is what does the bridge currently do with
BR_LEARNING + BR_PORT_LOCKED, and if that is sane and useful in any case?
It isn't a configuration that's rejected, for sure. The configuration
could be rejected via a bug fix patch, then in net-next it could be made
to learn these addresses with the BR_FDB_LOCKED flag.

To your question regarding the common case (no MAB): that can be supported
just fine when BR_LEARNING is off and BR_PORT_LOCKED is on, no?
No BR_FDB_LOCKED entries will be learned.

As it is now in the bridge, the locked port part is handled before learning
in the ingress data path, so with BR_LEARNING and BR_PORT_LOCKED, I think it
will work as it does now except link local packages.

If your suggestion of BR_LEARNING causing BR_FDB_LOCKED on a locked port, I
guess it would be implemented under br_fdb_update() and BR_LEARNING +
BR_PORT_LOCKED would go together, forcing BR_LEARNING in this case, thus also
for all drivers?