Linux Ethernet Bridging - Aug 2022 - [Bridge] [PATCH v5 net-next 6/6] selftests: forwarding: add test of MAC-Auth Bypass to locked port tests

On 2022-08-27 20:21, Ido Schimmel wrote:
> On Fri, Aug 26, 2022 at 01:45:38PM +0200, Hans Schultz wrote:
>> +locked_port_mab()
>> +{
>> +	RET=0
>> +	check_locked_port_support || return 0
>> +
>> +	ping_do $h1 192.0.2.2
>> +	check_err $? "MAB: Ping did not work before locking port"
>> +
>> +	bridge link set dev $swp1 locked on
>> +	bridge link set dev $swp1 learning on
> 
> "locked on learning on" is counter intuitive and IMO very much a
> misconfiguration that we should have disallowed when the "locked"
> option
> was introduced. It is my understanding that the only reason we are even
> talking about it is because mv88e6xxx needs it for MAB for some reason.
As the way mv88e6xxx implements "learning off" is to remove port 
association for ingress packets on a port, but that breaks many other 
things such as refreshing ATU entries and violation interrupts, so it is 
needed and the question is then what is the worst to have 'learning on' 
on a locked port or to have the locked port enabling learning in the 
driver silently?

Opinions seem to differ. Note that even on locked ports without MAB, 
port association on ingress is still needed in future as I have a 
dynamic ATU patch set coming, that uses age out violation and hardware 
refreshing to let the hardware keep the dynamic entries as long as the 
authorized station is sending, but will age the entry out if the station 
keeps silent for the ageing time. But that patch set is dependent on 
this patch set, and I don't think I can send it before this is 
accepted...
> Please avoid leaking this implementation detail to user space and
> instead use the "MAB" flag to enable learning if you need it in
> mv88e6xxx.
>

On Sun, Aug 28, 2022 at 02:00:29PM +0200, netdev at kapio-technology.com
wrote:
> On 2022-08-27 20:21, Ido Schimmel wrote:
> > On Fri, Aug 26, 2022 at 01:45:38PM +0200, Hans Schultz wrote:
> > > +locked_port_mab()
> > > +{
> > > +	RET=0
> > > +	check_locked_port_support || return 0
> > > +
> > > +	ping_do $h1 192.0.2.2
> > > +	check_err $? "MAB: Ping did not work before locking
port"
> > > +
> > > +	bridge link set dev $swp1 locked on
> > > +	bridge link set dev $swp1 learning on
> > 
> > "locked on learning on" is counter intuitive and IMO very
much a
> > misconfiguration that we should have disallowed when the
"locked" option
> > was introduced. It is my understanding that the only reason we are
even
> > talking about it is because mv88e6xxx needs it for MAB for some
reason.
> 
> As the way mv88e6xxx implements "learning off" is to remove port
association
> for ingress packets on a port, but that breaks many other things such as
> refreshing ATU entries and violation interrupts, so it is needed and the
> question is then what is the worst to have 'learning on' on a
locked port or
> to have the locked port enabling learning in the driver silently?
> 
> Opinions seem to differ. Note that even on locked ports without MAB, port
> association on ingress is still needed in future as I have a dynamic ATU
> patch set coming, that uses age out violation and hardware refreshing to
let
> the hardware keep the dynamic entries as long as the authorized station is
> sending, but will age the entry out if the station keeps silent for the
> ageing time. But that patch set is dependent on this patch set, and I
don't
> think I can send it before this is accepted...
Can you explain how you envision user space to work once everything is
merged? I want to make sure we have the full picture before more stuff
is merged. From what you describe, I expect the following:

1. Create topology, assuming two unauthorized ports:

# ip link add name br0 type bridge no_linklocal_learn 1 (*)
# ip link set dev swp1 master br0
# ip link set dev swp2 master br0
# bridge link set dev swp1 learning on locked on
# bridge link set dev swp2 learning on locked on
# ip link set dev swp1 up
# ip link set dev swp2 up
# ip link set dev br0 up

2. Assuming h1 behind swp1 was authorized using 802.1X:

# bridge fdb replace $H1_MAC dev swp1 master dynamic

3. Assuming 802.1X authentication failed for h2 behind swp2, enable MAB:

# bridge link set dev swp2 mab on

4. Assuming $H2_MAC is in our allow list:

# bridge fdb replace $H2_MAC dev swp2 master dynamic

Learning is on in order to refresh the dynamic entries that user space
installed.

(*) Need to add support for this option in iproute2. Already exposed
over netlink (see 'IFLA_BR_MULTI_BOOLOPT').