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

On Sun, Sep 11, 2022 at 11:23:55AM +0200, netdev at kapio-technology.com
wrote:
> On 2022-09-11 02:13, Vladimir Oltean wrote:
> > On Fri, Sep 09, 2022 at 03:11:56PM +0200, netdev at
kapio-technology.com
> > wrote:
> > > > > > On Wed, Sep 07, 2022 at 11:10:07PM +0200, netdev
at kapio-technology.com wrote:
> > > > > > > I am at the blackhole driver implementation
now, as I suppose that the
> > > > > > > iproute2 command should work with the
mv88e6xxx driver when adding blackhole
> > > > > > > entries (with a added selftest)?
> > > > > > > I decided to add the blackhole feature as new
ops for drivers with functions
> > > > > > > blackhole_fdb_add() and blackhole_fdb_del().
Do you agree with that approach?
> > > > > >
> > > > > > I assume you are talking about extending
'dsa_switch_ops'?
> > > > >
> > > > > Yes, that is the idea.
> > > > >
> > > > > > If so, it's up to the DSA maintainers to
decide.
> > > >
> > > > What will be the usefulness of adding a blackhole FDB entry
from user
> > > > space?
> > > 
> > > With the software bridge it could be used to signal a untrusted
host
> > > in
> > > connection with a locked port entry attempt. I don't see so
much use
> > > other
> > > that test purposes with the driver though.
> > 
> > Not a huge selling point, to be honest. Can't the blackhole flag
remain
> > settable only in the device -> bridge direction, with user space
just
> > reading it?
> 
> That is possible, but it would of course not make sense to have selftests
of
> the feature as that would not work unless there is a driver with this
> capability (now just mv88e6xxx).
The new "blackhole" flag requires changes in the bridge driver and
without allowing user space to add such entries, the only way to test
these changes is with mv88e6xxx which many of us do not have...

On 2022-09-12 11:08, Ido Schimmel wrote:
> On Sun, Sep 11, 2022 at 11:23:55AM +0200, netdev at kapio-technology.com 
> wrote:
>> On 2022-09-11 02:13, Vladimir Oltean wrote:
>> > On Fri, Sep 09, 2022 at 03:11:56PM +0200, netdev at
kapio-technology.com
>> > wrote:
>> > > > > > On Wed, Sep 07, 2022 at 11:10:07PM +0200,
netdev at kapio-technology.com wrote:
>> > > > > > > I am at the blackhole driver
implementation now, as I suppose that the
>> > > > > > > iproute2 command should work with the
mv88e6xxx driver when adding blackhole
>> > > > > > > entries (with a added selftest)?
>> > > > > > > I decided to add the blackhole feature as
new ops for drivers with functions
>> > > > > > > blackhole_fdb_add() and
blackhole_fdb_del(). Do you agree with that approach?
>> > > > > >
>> > > > > > I assume you are talking about extending
'dsa_switch_ops'?
>> > > > >
>> > > > > Yes, that is the idea.
>> > > > >
>> > > > > > If so, it's up to the DSA maintainers to
decide.
>> > > >
>> > > > What will be the usefulness of adding a blackhole FDB
entry from user
>> > > > space?
>> > >
>> > > With the software bridge it could be used to signal a
untrusted host
>> > > in
>> > > connection with a locked port entry attempt. I don't see
so much use
>> > > other
>> > > that test purposes with the driver though.
>> >
>> > Not a huge selling point, to be honest. Can't the blackhole
flag remain
>> > settable only in the device -> bridge direction, with user
space just
>> > reading it?
>> 
>> That is possible, but it would of course not make sense to have 
>> selftests of
>> the feature as that would not work unless there is a driver with this
>> capability (now just mv88e6xxx).
> 
> The new "blackhole" flag requires changes in the bridge driver
and
> without allowing user space to add such entries, the only way to test
> these changes is with mv88e6xxx which many of us do not have...
I am now building from new system (comp), and the kernel selftests are 
not being installed correctly, so I haven't been able to run the 
selftests yet.

I have made a blackhole selftest, which looks like this:

test_blackhole_fdb()
{
         RET=0

         check_blackhole_fdb_support || return 0

         tcpdump_start $h2
         $MZ $h1 -q -t udp -a $h1 -b $h2
         tcpdump_stop
         tcpdump_show | grep -q udp
         check_err $? "test_blackhole_fdb: No packet seen on initial"
         tcpdump_cleanup

         bridge fdb add `mac_get $h2` dev br0 blackhole
         bridge fdb show dev br0 | grep -q "blackhole"
         check_err $? "test_blackhole_fdb: No blackhole FDB entry
found"

         tcpdump_start $h2
         $MZ $h1 -q -t udp -a $h1 -b $h2
         tcpdump_stop
         tcpdump_show | grep -q udp
         check_fail $? "test_blackhole_fdb: packet seen with blackhole 
fdb entry"
         tcpdump_cleanup

         bridge fdb del `mac_get $h2` dev br0 blackhole
         bridge fdb show dev br0 | grep -q "blackhole"
         check_fail $? "test_blackhole_fdb: Blackhole FDB entry not 
deleted"

         tcpdump_start $h2
         $MZ $h1 -q -t udp -a $h1 -b $h2
         tcpdump_stop
         tcpdump_show | grep -q udp
         check_err $? "test_blackhole_fdb: No packet seen after removing 
blackhole FDB entry"
         tcpdump_cleanup

         log_test "Blackhole FDB entry test"
}

the setup is simple and is the same as in bridge_sticky_fdb.sh.

Does the test look sound or is there obvious mistakes?

On 2022-09-12 11:08, Ido Schimmel wrote:
> 
> The new "blackhole" flag requires changes in the bridge driver
and
> without allowing user space to add such entries, the only way to test
> these changes is with mv88e6xxx which many of us do not have...
There seems to be a little inconvenience when adding/deleting blackhole 
entries, and that is since all slaves are the listeners to 
SWITCHDEV_FDB_ADD(DEL)_TO_DEVICE events and blackhole entries are not to 
any slave devices, the ops will be called for every slave device as 
there is no way to distinguish. This said, the add and del operations 
work.