[net-next,0/5] ATU and FDB synchronization on locked ports

  This patch set makes it possible to have synchronized dynamic ATU and FDB
entries on locked ports. As locked ports are not able to automatically
learn, they depend on userspace added entries, where userspace can add
static or dynamic entries. The lifetime of static entries are completely
dependent on userspace intervention, and thus not of interest here. We
are only concerned with dynamic entries, which can be added with a
command like:

bridge fdb replace ADDR dev <DEV> master dynamic

We choose only to support this feature on locked ports, as it involves
utilizing the CPU to handle ATU related switchcore events (typically
interrupts) and thus can result in significant performance loss if
exposed to heavy traffic.

On locked ports it is important for userspace to know when an authorized
station has become silent, hence not breaking the communication of a
station that has been authorized based on the MAC-Authentication Bypass
(MAB) scheme. Thus if the station keeps being active after authorization,
it will continue to have an open port as long as it is active. Only after
a silent period will it have to be reauthorized. As the ageing process in
the ATU is dependent on incoming traffic to the switchcore port, it is
necessary for the ATU to signal that an entry has aged out, so that the
FDB can be updated at the correct time.

This patch set includes a solution for the Marvell mv88e6xxx driver, where
for this driver we use the Hold-At-One feature so that an age-out
violation interrupt occurs when a station has been silent for the
system-set age time. The age out violation interrupt allows the switchcore
driver to remove both the ATU and the FDB entry at the same time.

It is up to the maintainers of other switchcore drivers to implement the
feature for their specific driver.

Hans J. Schultz (5):
  net: bridge: add dynamic flag to switchdev notifier
  net: dsa: propagate flags down towards drivers
  drivers: net: dsa: add fdb entry flags incoming to switchcore drivers
  net: bridge: ensure FDB offloaded flag is handled as needed
  net: dsa: mv88e6xxx: implementation of dynamic ATU entries

 drivers/net/dsa/b53/b53_common.c        | 12 ++++-
 drivers/net/dsa/b53/b53_priv.h          |  4 +-
 drivers/net/dsa/hirschmann/hellcreek.c  | 12 ++++-
 drivers/net/dsa/lan9303-core.c          | 12 ++++-
 drivers/net/dsa/lantiq_gswip.c          | 12 ++++-
 drivers/net/dsa/microchip/ksz9477.c     |  8 ++--
 drivers/net/dsa/microchip/ksz9477.h     |  8 ++--
 drivers/net/dsa/microchip/ksz_common.c  | 14 ++++--
 drivers/net/dsa/mt7530.c                | 12 ++++-
 drivers/net/dsa/mv88e6xxx/chip.c        | 24 ++++++++--
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 21 +++++++++
 drivers/net/dsa/mv88e6xxx/port.c        |  6 ++-
 drivers/net/dsa/mv88e6xxx/switchdev.c   | 61 +++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/switchdev.h   |  5 ++
 drivers/net/dsa/mv88e6xxx/trace.h       |  5 ++
 drivers/net/dsa/ocelot/felix.c          | 12 ++++-
 drivers/net/dsa/qca/qca8k-common.c      | 12 ++++-
 drivers/net/dsa/qca/qca8k.h             |  4 +-
 drivers/net/dsa/rzn1_a5psw.c            | 12 ++++-
 drivers/net/dsa/sja1105/sja1105_main.c  | 19 ++++++--
 include/net/dsa.h                       |  6 ++-
 include/net/switchdev.h                 |  1 +
 net/bridge/br_fdb.c                     |  5 +-
 net/bridge/br_switchdev.c               |  2 +
 net/dsa/port.c                          | 28 +++++++-----
 net/dsa/port.h                          |  8 ++--
 net/dsa/slave.c                         | 17 +++++--
 net/dsa/switch.c                        | 30 ++++++++----
 net/dsa/switch.h                        |  1 +
 29 files changed, 299 insertions(+), 74 deletions(-)
  

On Mon, Jan 30, 2023 at 06:34:24PM +0100, Hans J. Schultz wrote:
> This patch set makes it possible to have synchronized dynamic ATU and FDB
> entries on locked ports. As locked ports are not able to automatically
> learn, they depend on userspace added entries, where userspace can add
> static or dynamic entries. The lifetime of static entries are completely
> dependent on userspace intervention, and thus not of interest here. We
> are only concerned with dynamic entries, which can be added with a
> command like:
> 
> bridge fdb replace ADDR dev <DEV> master dynamic
> 
> We choose only to support this feature on locked ports, as it involves
> utilizing the CPU to handle ATU related switchcore events (typically
> interrupts) and thus can result in significant performance loss if
> exposed to heavy traffic.

Not sure I understand this reasoning. I was under the impression that
hostapd is installing dynamic entries instead of static ones since the
latter are not flushed when carrier is lost. Therefore, with static
entries it is possible to unplug a host (potentially plugging a
different one) and not lose authentication.

> 
> On locked ports it is important for userspace to know when an authorized
> station has become silent, hence not breaking the communication of a
> station that has been authorized based on the MAC-Authentication Bypass
> (MAB) scheme. Thus if the station keeps being active after authorization,
> it will continue to have an open port as long as it is active. Only after
> a silent period will it have to be reauthorized. As the ageing process in
> the ATU is dependent on incoming traffic to the switchcore port, it is
> necessary for the ATU to signal that an entry has aged out, so that the
> FDB can be updated at the correct time.

Why mention MAB at all? Don't you want user space to always use dynamic
entries to authenticate hosts regardless of 802.1X/MAB?

> 
> This patch set includes a solution for the Marvell mv88e6xxx driver, where
> for this driver we use the Hold-At-One feature so that an age-out
> violation interrupt occurs when a station has been silent for the
> system-set age time. The age out violation interrupt allows the switchcore
> driver to remove both the ATU and the FDB entry at the same time.
> 
> It is up to the maintainers of other switchcore drivers to implement the
> feature for their specific driver.
> 
> Hans J. Schultz (5):
>   net: bridge: add dynamic flag to switchdev notifier
>   net: dsa: propagate flags down towards drivers
>   drivers: net: dsa: add fdb entry flags incoming to switchcore drivers
>   net: bridge: ensure FDB offloaded flag is handled as needed
>   net: dsa: mv88e6xxx: implementation of dynamic ATU entries

Will try to review tomorrow, but it looks like this set is missing
selftests. What about extending bridge_locked_port.sh?
  

On 2023-01-31 20:25, Ido Schimmel wrote:
> 
> Will try to review tomorrow, but it looks like this set is missing
> selftests. What about extending bridge_locked_port.sh?

I knew you would take this up. :-)
But I am not sure that it's so easy to have selftests here as it is 
timing based and it would take the 5+ minutes just waiting to test in 
the stadard case, and there is opnly support for mv88e6xxx driver with 
this patch set.
  

On Thu, Feb 02, 2023 at 08:37:08AM +0100, netdev@kapio-technology.com wrote:
> On 2023-01-31 20:25, Ido Schimmel wrote:
> > 
> > Will try to review tomorrow, but it looks like this set is missing
> > selftests. What about extending bridge_locked_port.sh?
> 
> I knew you would take this up. :-)
> But I am not sure that it's so easy to have selftests here as it is timing
> based and it would take the 5+ minutes just waiting to test in the stadard
> case, and there is opnly support for mv88e6xxx driver with this patch set.

The ageing time is configurable: See commit 081197591769 ("selftests:
net: bridge: Parameterize ageing timeout"). Please add test cases in the
next version.
  

On 2023-02-02 16:43, Ido Schimmel wrote:
> On Thu, Feb 02, 2023 at 08:37:08AM +0100, netdev@kapio-technology.com 
> wrote:
>> On 2023-01-31 20:25, Ido Schimmel wrote:
>> >
>> > Will try to review tomorrow, but it looks like this set is missing
>> > selftests. What about extending bridge_locked_port.sh?
>> 
>> I knew you would take this up. :-)
>> But I am not sure that it's so easy to have selftests here as it is 
>> timing
>> based and it would take the 5+ minutes just waiting to test in the 
>> stadard
>> case, and there is opnly support for mv88e6xxx driver with this patch 
>> set.
> 
> The ageing time is configurable: See commit 081197591769 ("selftests:
> net: bridge: Parameterize ageing timeout"). Please add test cases in 
> the
> next version.

When I was looking at configuring the ageing time last time, my finding 
was that the ageing time could not be set very low as there was some 
part in the DSA layer etc, and confusion wrt units. I think the minimum 
secured was like around 2 min. (not validated), which is not that much 
of an improvement for fast testing. If you know what would be a good low 
timeout to set, I would like to know.
  

On Thu, Feb 02, 2023 at 05:19:07PM +0100, netdev@kapio-technology.com wrote:
> On 2023-02-02 16:43, Ido Schimmel wrote:
> > On Thu, Feb 02, 2023 at 08:37:08AM +0100, netdev@kapio-technology.com
> > wrote:
> > > On 2023-01-31 20:25, Ido Schimmel wrote:
> > > >
> > > > Will try to review tomorrow, but it looks like this set is missing
> > > > selftests. What about extending bridge_locked_port.sh?
> > > 
> > > I knew you would take this up. :-)
> > > But I am not sure that it's so easy to have selftests here as it is
> > > timing
> > > based and it would take the 5+ minutes just waiting to test in the
> > > stadard
> > > case, and there is opnly support for mv88e6xxx driver with this
> > > patch set.
> > 
> > The ageing time is configurable: See commit 081197591769 ("selftests:
> > net: bridge: Parameterize ageing timeout"). Please add test cases in the
> > next version.
> 
> When I was looking at configuring the ageing time last time, my finding was
> that the ageing time could not be set very low as there was some part in the
> DSA layer etc, and confusion wrt units. I think the minimum secured was like
> around 2 min. (not validated), which is not that much of an improvement for
> fast testing. If you know what would be a good low timeout to set, I would
> like to know.

My point is that the ageing time is parametrized via 'LOW_AGEING_TIME'
in forwarding.config so just use '$LOW_AGEING_TIME' in the selftest and
set it as high as it needs to be for mv88e6xxx in your own
forwarding.config.
  

On 2023-01-31 20:25, Ido Schimmel wrote:
>> command like:
>> 
>> bridge fdb replace ADDR dev <DEV> master dynamic
>> 
>> We choose only to support this feature on locked ports, as it involves
>> utilizing the CPU to handle ATU related switchcore events (typically
>> interrupts) and thus can result in significant performance loss if
>> exposed to heavy traffic.
> 
> Not sure I understand this reasoning. I was under the impression that
> hostapd is installing dynamic entries instead of static ones since the
> latter are not flushed when carrier is lost. Therefore, with static
> entries it is possible to unplug a host (potentially plugging a
> different one) and not lose authentication.
> 

Both auth schemes 802.1X and MAB install dynamic entries as you point 
out, and both use locked ports.
In the case of non locked ports, they just learn normally and age and 
refresh their entries, so the use case of a userspace added dynamic FDB 
entry is hard for me to see. And having userspace being notified of an 
ordinary event that a FDB entry has been aged out could maybe be used, 
but for the reasons mentioned it is not supported here.

>> 
>> On locked ports it is important for userspace to know when an 
>> authorized
>> station has become silent, hence not breaking the communication of a
>> station that has been authorized based on the MAC-Authentication 
>> Bypass
>> (MAB) scheme. Thus if the station keeps being active after 
>> authorization,
>> it will continue to have an open port as long as it is active. Only 
>> after
>> a silent period will it have to be reauthorized. As the ageing process 
>> in
>> the ATU is dependent on incoming traffic to the switchcore port, it is
>> necessary for the ATU to signal that an entry has aged out, so that 
>> the
>> FDB can be updated at the correct time.
> 
> Why mention MAB at all? Don't you want user space to always use dynamic
> entries to authenticate hosts regardless of 802.1X/MAB?

Yes, you are right about that. I guess it came about as this was 
developed much in the same time and with the code of MAB.
  

On Thu, Feb 02, 2023 at 06:36:14PM +0200, Ido Schimmel wrote:
> On Thu, Feb 02, 2023 at 05:19:07PM +0100, netdev@kapio-technology.com wrote:
> > On 2023-02-02 16:43, Ido Schimmel wrote:
> > > On Thu, Feb 02, 2023 at 08:37:08AM +0100, netdev@kapio-technology.com wrote:
> > > > On 2023-01-31 20:25, Ido Schimmel wrote:
> > > > >
> > > > > Will try to review tomorrow, but it looks like this set is missing
> > > > > selftests. What about extending bridge_locked_port.sh?
> > > > 
> > > > I knew you would take this up. :-)
> > > > But I am not sure that it's so easy to have selftests here as it is timing
> > > > based and it would take the 5+ minutes just waiting to test in the stadard
> > > > case, and there is opnly support for mv88e6xxx driver with this
> > > > patch set.
> > > 
> > > The ageing time is configurable: See commit 081197591769 ("selftests:
> > > net: bridge: Parameterize ageing timeout"). Please add test cases in the
> > > next version.
> > 
> > When I was looking at configuring the ageing time last time, my finding was
> > that the ageing time could not be set very low as there was some part in the
> > DSA layer etc, and confusion wrt units. I think the minimum secured was like
> > around 2 min. (not validated), which is not that much of an improvement for
> > fast testing. If you know what would be a good low timeout to set, I would
> > like to know.
> 
> My point is that the ageing time is parametrized via 'LOW_AGEING_TIME'
> in forwarding.config so just use '$LOW_AGEING_TIME' in the selftest and
> set it as high as it needs to be for mv88e6xxx in your own
> forwarding.config.

FWIW, we have a forwarding.config file in tools/testing/selftests/drivers/net/dsa/.
So you could cd to that folder, edit the file with your variable, and run the symlinked
script from there.

> as there was some part in the DSA layer etc

	if (ds->ageing_time_min && ageing_time < ds->ageing_time_min)
		return -ERANGE;

High tech, advanced software.....

You could print the ds->ageing_time_min variable. For mv88e6xxx, my 6390
and 6190 report 3750. I have to admit the ageing time units are confusing,
but Tobias Waldekranz kindly explained in one of those commit messages
that Ido linked to that these represent "centiseconds" (or 37.5 seconds).
And I think we discussed the units with you before. And in general, it's
not hard to find the answer if you search for it, I know I could find it.

Please stop trying to find silly excuses to always go through the path
of minimal resistance.