Linux Ethernet Bridging - Oct 2022 - [Bridge] [RFC PATCH net-next 10/16] mlxsw: spectrum_switchdev: Add support for locked FDB notifications

On Tue, Oct 25, 2022 at 01:00:18PM +0300, Ido Schimmel
wrote:
> In Spectrum, learning happens in parallel to the security checks.
> Therefore, regardless of the result of the security checks, a learning
> notification will be generated by the device and polled later on by the
> driver.
> 
> Currently, the driver reacts to learning notifications by programming
> corresponding FDB entries to the device. When a port is locked (i.e.,
> has security checks enabled), this can no longer happen, as otherwise
> any host will blindly gain authorization.
> 
> Instead, notify the learned entry as a locked entry to the bridge driver
> that will in turn notify it to user space, in case MAB is enabled. User
> space can then decide to authorize the host by clearing the
"locked"
> flag, which will cause the entry to be programmed to the device.
> 
> Signed-off-by: Ido Schimmel <idosch at nvidia.com>
> ---
So for mlxsw, the hardware/driver always gets learning notifications
if learning is enabled (and regardless of MAB being enabled; with the
mention that BR_PORT_MAB implies BR_LEARNING and so, with MAB, these
notifications always come), and the driver always calls
SWITCHDEV_FDB_ADD_TO_BRIDGE,
letting the bridge figure out if it should create a BR_FDB_LOCKED entry
or to throw the notification away?

Hans' case is different; he needs to configure the HW differently
(MAB is more resource intensive). I suppose at some point, in his patch
series, he will need to also offload BR_PORT_MAB, something which you
didn't need. Ok.

The thing is that it will become tricky to know, when adding BR_PORT_MAB
to BR_PORT_FLAGS_HW_OFFLOAD, which drivers can offload MAB and which
can't, without some prior knowledge. For example, Hans will need to
patch mlxsw_sp_port_attr_br_pre_flags_set() to not reject BR_PORT_MAB,
even if mlxsw will need to do nothing based on the flag, right?

On Thu, Oct 27, 2022 at 11:39:40PM +0000, Vladimir Oltean
wrote:
> On Tue, Oct 25, 2022 at 01:00:18PM +0300, Ido Schimmel wrote:
> > In Spectrum, learning happens in parallel to the security checks.
> > Therefore, regardless of the result of the security checks, a learning
> > notification will be generated by the device and polled later on by
the
> > driver.
> > 
> > Currently, the driver reacts to learning notifications by programming
> > corresponding FDB entries to the device. When a port is locked (i.e.,
> > has security checks enabled), this can no longer happen, as otherwise
> > any host will blindly gain authorization.
> > 
> > Instead, notify the learned entry as a locked entry to the bridge
driver
> > that will in turn notify it to user space, in case MAB is enabled.
User
> > space can then decide to authorize the host by clearing the
"locked"
> > flag, which will cause the entry to be programmed to the device.
> > 
> > Signed-off-by: Ido Schimmel <idosch at nvidia.com>
> > ---
> 
> So for mlxsw, the hardware/driver always gets learning notifications
> if learning is enabled (and regardless of MAB being enabled; with the
> mention that BR_PORT_MAB implies BR_LEARNING and so, with MAB, these
> notifications always come), and the driver always calls
SWITCHDEV_FDB_ADD_TO_BRIDGE,
> letting the bridge figure out if it should create a BR_FDB_LOCKED entry
> or to throw the notification away?
Yes, correct.
> 
> Hans' case is different; he needs to configure the HW differently
> (MAB is more resource intensive). I suppose at some point, in his patch
> series, he will need to also offload BR_PORT_MAB, something which you
> didn't need. Ok.
> 
> The thing is that it will become tricky to know, when adding BR_PORT_MAB
> to BR_PORT_FLAGS_HW_OFFLOAD, which drivers can offload MAB and which
> can't, without some prior knowledge. For example, Hans will need to
> patch mlxsw_sp_port_attr_br_pre_flags_set() to not reject BR_PORT_MAB,
> even if mlxsw will need to do nothing based on the flag, right?
Right. I'm quite reluctant to add the MAB flag to
BR_PORT_FLAGS_HW_OFFLOAD as part of this patchset for the simple reason
that it is not really needed. I'm not worried about someone adding it
later when it is actually needed. We will probably catch the omission
during code review. Worst case, we have a selftest that will break,
notifying us that a bug fix is needed.