From: "Hans J. Schultz" <netdev@kapio-technology.com>

This patch set extends the locked port feature for devices
that are behind a locked port, but do not have the ability to
authorize themselves as a supplicant using IEEE 802.1X.
Such devices can be printers, meters or anything related to
fixed installations. Instead of 802.1X authorization, devices
can get access based on their MAC addresses being whitelisted.

For an authorization daemon to detect that a device is trying
to get access through a locked port, the bridge will add the
MAC address of the device to the FDB with a locked flag to it.
Thus the authorization daemon can catch the FDB add event and
check if the MAC address is in the whitelist and if so replace
the FDB entry without the locked flag enabled, and thus open
the port for the device.

This feature is known as MAC-Auth or MAC Authentication Bypass
(MAB) in Cisco terminology, where the full MAB concept involves
additional Cisco infrastructure for authorization. There is no
real authentication process, as the MAC address of the device
is the only input the authorization daemon, in the general
case, has to base the decision if to unlock the port or not.

With this patch set, an implementation of the offloaded case is
supplied for the mv88e6xxx driver. When a packet ingresses on
a locked port, an ATU miss violation event will occur. When
handling such ATU miss violation interrupts, the MAC address of
the device is added to the FDB with a zero destination port
vector (DPV) and the MAC address is communicated through the
switchdev layer to the bridge, so that a FDB entry with the
locked flag enabled can be added.

Log:
        v3:     Added timers and lists in the driver (mv88e6xxx)
                to keep track of and remove locked entries.

        v4:     Leave out enforcing a limit to the number of
                locked entries in the bridge.
                Removed the timers in the driver and use the
                worker only. Add locked FDB flag to all drivers
                using port_fdb_add() from the dsa api and let
                all drivers ignore entries with this flag set.
                Change how to get the ageing timeout of locked
                entries. See global1_atu.c and switchdev.c.
                Use struct mv88e6xxx_port for locked entries
                variables instead of struct dsa_port.

        v5:    	Added 'mab' flag to enable MAB/MacAuth feature,
        	in a similar way to the locked feature flag.

        	In these implementations for the mv88e6xxx, the
        	switchport must be configured with learning on.

        	To tell userspace about the behavior of the
        	locked entries in the driver, a 'blackhole'
        	FDB flag has been added, which locked FDB
        	entries coming from the driver gets. Also the
        	'sticky' flag comes with those locked entries,
        	as the drivers locked entries cannot roam.

        	Fixed issues with taking mutex locks, and added
        	a function to read the fid, that supports all
        	versions of the chipset family.

        v6:     Added blackhole FDB flag instead of using sticky
		flag, as the blackhole flag corresponds to the
		behaviour of the zero-DPV locked entries in the
		driver.

		Userspace can add blackhole FDB entries with:
		# bridge fdb add MAC dev br0 blackhole

		Added FDB flags towards driver in DSA layer as u16.

Hans J. Schultz (9):
  net: bridge: add locked entry fdb flag to extend locked port feature
  net: bridge: add blackhole fdb entry flag
  net: switchdev: add support for offloading of the FDB locked flag
  net: switchdev: support offloading of the FDB blackhole flag
  drivers: net: dsa: add fdb entry flags to drivers
  net: dsa: mv88e6xxx: allow reading FID when handling ATU violations
  net: dsa: mv88e6xxx: mac-auth/MAB implementation
  net: dsa: mv88e6xxx: add blackhole ATU entries
  selftests: forwarding: add test of MAC-Auth Bypass to locked port
    tests

 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/Makefile            |   1 +
 drivers/net/dsa/mv88e6xxx/chip.c              | 158 +++++++++-
 drivers/net/dsa/mv88e6xxx/chip.h              |  19 ++
 drivers/net/dsa/mv88e6xxx/global1.h           |   1 +
 drivers/net/dsa/mv88e6xxx/global1_atu.c       |  72 ++++-
 drivers/net/dsa/mv88e6xxx/port.c              |  15 +-
 drivers/net/dsa/mv88e6xxx/port.h              |   6 +
 drivers/net/dsa/mv88e6xxx/switchdev.c         | 284 ++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/switchdev.h         |  37 +++
 drivers/net/dsa/ocelot/felix.c                |  12 +-
 drivers/net/dsa/qca/qca8k-common.c            |  10 +-
 drivers/net/dsa/qca/qca8k.h                   |   4 +-
 drivers/net/dsa/sja1105/sja1105_main.c        |  14 +-
 include/linux/if_bridge.h                     |   1 +
 include/net/dsa.h                             |   7 +-
 include/net/switchdev.h                       |   2 +
 include/uapi/linux/if_link.h                  |   1 +
 include/uapi/linux/neighbour.h                |  11 +-
 net/bridge/br.c                               |   5 +-
 net/bridge/br_fdb.c                           |  77 ++++-
 net/bridge/br_input.c                         |  20 +-
 net/bridge/br_netlink.c                       |  12 +-
 net/bridge/br_private.h                       |   5 +-
 net/bridge/br_switchdev.c                     |   4 +-
 net/core/rtnetlink.c                          |   9 +
 net/dsa/dsa_priv.h                            |  10 +-
 net/dsa/port.c                                |  32 +-
 net/dsa/slave.c                               |  16 +-
 net/dsa/switch.c                              |  24 +-
 .../net/forwarding/bridge_blackhole_fdb.sh    | 102 +++++++
 .../net/forwarding/bridge_locked_port.sh      | 106 ++++++-
 .../net/forwarding/bridge_sticky_fdb.sh       |  21 +-
 tools/testing/selftests/net/forwarding/lib.sh |  18 ++
 42 files changed, 1093 insertions(+), 117 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.h
 create mode 100755 tools/testing/selftests/net/forwarding/bridge_blackhole_fdb.sh

-- 
2.34.1

On Wed, 28 Sep 2022 17:02:47 +0200 Hans Schultz wrote:
> From: "Hans J. Schultz" <netdev@kapio-technology.com>
> 
> This patch set extends the locked port feature for devices
> that are behind a locked port, but do not have the ability to
> authorize themselves as a supplicant using IEEE 802.1X.
> Such devices can be printers, meters or anything related to
> fixed installations. Instead of 802.1X authorization, devices
> can get access based on their MAC addresses being whitelisted.

Try a allmodconfig build on latest net-next, seems broken.

On 2022-09-29 18:10, Jakub Kicinski wrote:
> On Wed, 28 Sep 2022 17:02:47 +0200 Hans Schultz wrote:
>> From: "Hans J. Schultz" <netdev@kapio-technology.com>
>> 
>> This patch set extends the locked port feature for devices
>> that are behind a locked port, but do not have the ability to
>> authorize themselves as a supplicant using IEEE 802.1X.
>> Such devices can be printers, meters or anything related to
>> fixed installations. Instead of 802.1X authorization, devices
>> can get access based on their MAC addresses being whitelisted.
> 
> Try a allmodconfig build on latest net-next, seems broken.

I have all different switch drivers enabled and I see no compile 
warnings or errors. I guess I will get a robot update if that is the 
case, but please be specific as to what does not build.

On Thu, 29 Sep 2022 18:37:09 +0200 netdev@kapio-technology.com wrote:
> On 2022-09-29 18:10, Jakub Kicinski wrote:
> > On Wed, 28 Sep 2022 17:02:47 +0200 Hans Schultz wrote:  
> >> From: "Hans J. Schultz" <netdev@kapio-technology.com>
> >> 
> >> This patch set extends the locked port feature for devices
> >> that are behind a locked port, but do not have the ability to
> >> authorize themselves as a supplicant using IEEE 802.1X.
> >> Such devices can be printers, meters or anything related to
> >> fixed installations. Instead of 802.1X authorization, devices
> >> can get access based on their MAC addresses being whitelisted.  
> > 
> > Try a allmodconfig build on latest net-next, seems broken.  
> 
> I have all different switch drivers enabled and I see no compile 
> warnings or errors. 

Just do what I told you - rebase on net-next, allmodconfig.

> I guess I will get a robot update if that is the 
> case but please be specific as to what does not build.

The maintainers simply don't have time to hold everyone by the hand.
Sometimes I wish it was still okay to yell at people who post code
which does not build. Oh well.

../drivers/net/dsa/qca/qca8k-common.c:810:5: error: conflicting types for ‘qca8k_port_fdb_del’
 int qca8k_port_fdb_del(struct dsa_switch *ds, int port,
     ^~~~~~~~~~~~~~~~~~
In file included from ../drivers/net/dsa/qca/qca8k-common.c:13:
../drivers/net/dsa/qca/qca8k.h:483:5: note: previous declaration of ‘qca8k_port_fdb_del’ was here
 int qca8k_port_fdb_del(struct dsa_switch *ds, int port,
     ^~~~~~~~~~~~~~~~~~
../drivers/net/dsa/qca/qca8k-common.c: In function ‘qca8k_port_fdb_del’:
../drivers/net/dsa/qca/qca8k-common.c:818:6: error: ‘fdb_flags’ undeclared (first use in this function); did you mean ‘tsq_flags’?
  if (fdb_flags)
      ^~~~~~~~~
      tsq_flags
../drivers/net/dsa/qca/qca8k-common.c:818:6: note: each undeclared identifier is reported only once for each function it appears in
make[5]: *** [../scripts/Makefile.build:249: drivers/net/dsa/qca/qca8k-common.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [../scripts/Makefile.build:465: drivers/net/dsa/qca] Error 2
make[4]: *** Waiting for unfinished jobs....
../drivers/net/dsa/sja1105/sja1105_main.c: In function ‘sja1105_fast_age’:
../drivers/net/dsa/sja1105/sja1105_main.c:1941:61: error: incompatible type for argument 5 of ‘sja1105_fdb_del’
   rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
                                                             ^~
../drivers/net/dsa/sja1105/sja1105_main.c:1831:11: note: expected ‘u16’ {aka ‘short unsigned int’} but argument is of type ‘struct dsa_db’
       u16 fdb_flags, struct dsa_db db)
       ~~~~^~~~~~~~~
../drivers/net/dsa/sja1105/sja1105_main.c:1941:8: error: too few arguments to function ‘sja1105_fdb_del’
   rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
        ^~~~~~~~~~~~~~~
../drivers/net/dsa/sja1105/sja1105_main.c:1829:12: note: declared here
 static int sja1105_fdb_del(struct dsa_switch *ds, int port,
            ^~~~~~~~~~~~~~~
../drivers/net/dsa/sja1105/sja1105_main.c: In function ‘sja1105_mdb_del’:
../drivers/net/dsa/sja1105/sja1105_main.c:1962:56: error: incompatible type for argument 5 of ‘sja1105_fdb_del’
  return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid, db);
                                                        ^~
../drivers/net/dsa/sja1105/sja1105_main.c:1831:11: note: expected ‘u16’ {aka ‘short unsigned int’} but argument is of type ‘struct dsa_db’
       u16 fdb_flags, struct dsa_db db)
       ~~~~^~~~~~~~~
../drivers/net/dsa/sja1105/sja1105_main.c:1962:9: error: too few arguments to function ‘sja1105_fdb_del’
  return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid, db);
         ^~~~~~~~~~~~~~~
../drivers/net/dsa/sja1105/sja1105_main.c:1829:12: note: declared here
 static int sja1105_fdb_del(struct dsa_switch *ds, int port,
            ^~~~~~~~~~~~~~~
../drivers/net/dsa/sja1105/sja1105_main.c:1963:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1: some warnings being treated as errors
make[5]: *** [../scripts/Makefile.build:249: drivers/net/dsa/sja1105/sja1105_main.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [../scripts/Makefile.build:465: drivers/net/dsa/sja1105] Error 2
make[3]: *** [../scripts/Makefile.build:465: drivers/net/dsa] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [../scripts/Makefile.build:465: drivers/net] Error 2
make[1]: *** [/home/kicinski/linux/Makefile:1852: drivers] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:222: __sub-make] Error 2

On 2022-09-29 20:27, Jakub Kicinski wrote:
> On Thu, 29 Sep 2022 18:37:09 +0200 netdev@kapio-technology.com wrote:
>> On 2022-09-29 18:10, Jakub Kicinski wrote:
>> > On Wed, 28 Sep 2022 17:02:47 +0200 Hans Schultz wrote:
>> >> From: "Hans J. Schultz" <netdev@kapio-technology.com>
>> >>
>> >> This patch set extends the locked port feature for devices
>> >> that are behind a locked port, but do not have the ability to
>> >> authorize themselves as a supplicant using IEEE 802.1X.
>> >> Such devices can be printers, meters or anything related to
>> >> fixed installations. Instead of 802.1X authorization, devices
>> >> can get access based on their MAC addresses being whitelisted.
>> >
>> > Try a allmodconfig build on latest net-next, seems broken.

Obviously my method of selecting all switchcore drivers with sub-options 
under menuconfig was not sufficient, and I didn't know of the 
allmodconfig option, otherwise I would have used it.

So the question is if I should repost the fixed patch-set or I need to 
make a new version?

Anyhow I hope that there will not be problems when running the 
selftests, as I have not been able to do so with my system, so there can 
be more that needs to be changed.

If anyone needs it, here is the compile fix patch:

diff --git a/drivers/net/dsa/qca/qca8k-common.c 
b/drivers/net/dsa/qca/qca8k-common.c
index 0c5f49de6729..e26a9a483955 100644
--- a/drivers/net/dsa/qca/qca8k-common.c
+++ b/drivers/net/dsa/qca/qca8k-common.c
@@ -809,7 +809,7 @@ int qca8k_port_fdb_add(struct dsa_switch *ds, int 
port,

  int qca8k_port_fdb_del(struct dsa_switch *ds, int port,
  		       const unsigned char *addr, u16 vid,
-		       struct dsa_db db)
+		       u16 fdb_flags, struct dsa_db db)
  {
  	struct qca8k_priv *priv = (struct qca8k_priv *)ds->priv;
  	u16 port_mask = BIT(port);
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c 
b/drivers/net/dsa/sja1105/sja1105_main.c
index 1f12a5b89c91..526177813d53 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1938,7 +1938,7 @@ static void sja1105_fast_age(struct dsa_switch 
*ds, int port)

  		u64_to_ether_addr(l2_lookup.macaddr, macaddr);

-		rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
+		rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, 0, db);
  		if (rc) {
  			dev_err(ds->dev,
  				"Failed to delete FDB entry %pM vid %lld: %pe\n",
@@ -1952,14 +1952,14 @@ static int sja1105_mdb_add(struct dsa_switch 
*ds, int port,
  			   const struct switchdev_obj_port_mdb *mdb,
  			   struct dsa_db db)
  {
-	return sja1105_fdb_add(ds, port, mdb->addr, mdb->vid, false, db);
+	return sja1105_fdb_add(ds, port, mdb->addr, mdb->vid, 0, db);
  }

  static int sja1105_mdb_del(struct dsa_switch *ds, int port,
  			   const struct switchdev_obj_port_mdb *mdb,
  			   struct dsa_db db)
  {
-	return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid, db);
+	return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid, 0, db);
  }

  /* Common function for unicast and broadcast flood configuration.

On Fri, Sep 30, 2022 at 07:42:37AM +0200, netdev@kapio-technology.com wrote:
> Obviously my method of selecting all switchcore drivers with sub-options
> under menuconfig was not sufficient, and I didn't know of the allmodconfig
> option, otherwise I would have used it.

You can see build issues on patchwork:

https://patchwork.kernel.org/project/netdevbpf/patch/20220928150256.115248-6-netdev@kapio-technology.com/

Also:

https://docs.kernel.org/next/process/maintainer-netdev.html#what-level-of-testing-is-expected-before-i-submit-my-change

https://docs.kernel.org/next/process/maintainer-netdev.html#can-i-reproduce-the-checks-from-patchwork-on-my-local-machine

https://docs.kernel.org/next/process/maintainer-netdev.html#running-all-the-builds-and-checks-locally-is-a-pain-can-i-post-my-patches-and-have-the-patchwork-bot-validate-them

> So the question is if I should repost the fixed patch-set or I need to make
> a new version?

A new fixed version (v7) is required, but wait for this version to be
reviewed first.

> Anyhow I hope that there will not be problems when running the selftests, as
> I have not been able to do so with my system, so there can be more that
> needs to be changed.

It's not really acceptable to post tests that you haven't run... What
exactly is the issue? You should be able to run the tests with veth
pairs in a VM.

On 2022-09-30 16:05, Ido Schimmel wrote:
> 
> It's not really acceptable to post tests that you haven't run... What
> exactly is the issue? You should be able to run the tests with veth
> pairs in a VM.

Sorry about that, I have now got something up and running by using live 
Ubuntu on a USB...

On 2022-09-30 16:05, Ido Schimmel wrote:
> What exactly is the issue? You should be able to run the tests with 
> veth
> pairs in a VM.

First there is an issue with alsa missing for some mixer tests, then 
there is several reports of sys/capability.h missing, and then just 
really many obscure problems that look like wrong lib versions are in 
place. Here is some of the long log of errors etc... :(


In file included from lib/elf.c:8:
include/test_util.h: In function ‘align_up’:
include/test_util.h:134:7: warning: format ‘%lu’ expects argument of 
type ‘long unsigned int’, but argument 6 has type ‘uint64_t’ {aka ‘long 
long unsigned int’} [-Wformat=]
   134 |       "size not a power of 2: %lu", size);
       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~
       |                                     |
       |                                     uint64_t {aka long long 
unsigned int}
include/test_util.h:54:43: note: in definition of macro ‘TEST_ASSERT’
    54 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
       |                                           ^~~
include/test_util.h:134:33: note: format string is defined here
   134 |       "size not a power of 2: %lu", size);
       |                               ~~^
       |                                 |
       |                                 long unsigned int
       |                               %llu
include/test_util.h: In function ‘align_ptr_up’:
include/test_util.h:150:9: warning: cast to pointer from integer of 
different size [-Wint-to-pointer-cast]
   150 |  return (void *)align_up((unsigned long)x, size);
       |         ^
In file included from include/kvm_util.h:10,
                  from lib/elf.c:13:
include/kvm_util_base.h: At top level:
include/kvm_util_base.h:93:26: error: field ‘stats_header’ has 
incomplete type
    93 |  struct kvm_stats_header stats_header;
       |                          ^~~~~~~~~~~~
In file included from ../../../include/linux/kernel.h:8,
                  from ../../../include/linux/list.h:7,
                  from ../../../include/linux/hashtable.h:10,
                  from include/kvm_util_base.h:13,
                  from include/kvm_util.h:10,
                  from lib/elf.c:13:
include/kvm_util_base.h: In function ‘kvm_vm_reset_dirty_ring’:
include/kvm_util_base.h:308:24: error: ‘KVM_RESET_DIRTY_RINGS’ 
undeclared (first use in this function); did you mean 
‘KVM_GET_DIRTY_LOG’?
   308 |  return __vm_ioctl(vm, KVM_RESET_DIRTY_RINGS, NULL);
       |                        ^~~~~~~~~~~~~~~~~~~~~
../../../include/linux/build_bug.h:79:56: note: in definition of macro 
‘__static_assert’
    79 | #define __static_assert(expr, msg, ...) _Static_assert(expr, 
msg)
       |                                                        ^~~~
include/kvm_util_base.h:193:2: note: in expansion of macro 
‘static_assert’
   193 |  static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == 
_IOC_SIZE(cmd), ""); \
       |  ^~~~~~~~~~~~~
include/kvm_util_base.h:216:2: note: in expansion of macro 
‘kvm_do_ioctl’
   216 |  kvm_do_ioctl((vm)->fd, cmd, arg);   \
       |  ^~~~~~~~~~~~
include/kvm_util_base.h:308:9: note: in expansion of macro ‘__vm_ioctl’
   308 |  return __vm_ioctl(vm, KVM_RESET_DIRTY_RINGS, NULL);
       |         ^~~~~~~~~~
include/kvm_util_base.h:308:24: note: each undeclared identifier is 
reported only once for each function it appears in
   308 |  return __vm_ioctl(vm, KVM_RESET_DIRTY_RINGS, NULL);
       |                        ^~~~~~~~~~~~~~~~~~~~~
../../../include/linux/build_bug.h:79:56: note: in definition of macro 
‘__static_assert’
    79 | #define __static_assert(expr, msg, ...) _Static_assert(expr, 
msg)
       |                                                        ^~~~
include/kvm_util_base.h:193:2: note: in expansion of macro 
‘static_assert’
   193 |  static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == 
_IOC_SIZE(cmd), ""); \
       |  ^~~~~~~~~~~~~
include/kvm_util_base.h:216:2: note: in expansion of macro 
‘kvm_do_ioctl’
   216 |  kvm_do_ioctl((vm)->fd, cmd, arg);   \
       |  ^~~~~~~~~~~~
include/kvm_util_base.h:308:9: note: in expansion of macro ‘__vm_ioctl’
   308 |  return __vm_ioctl(vm, KVM_RESET_DIRTY_RINGS, NULL);
       |         ^~~~~~~~~~
include/kvm_util_base.h:193:16: error: expression in static assertion is 
not an integer
   193 |  static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == 
_IOC_SIZE(cmd), ""); \
       |                ^
../../../include/linux/build_bug.h:79:56: note: in definition of macro 
‘__static_assert’
    79 | #define __static_assert(expr, msg, ...) _Static_assert(expr, 
msg)
       |                                                        ^~~~
include/kvm_util_base.h:193:2: note: in expansion of macro 
‘static_assert’
   193 |  static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == 
_IOC_SIZE(cmd), ""); \
       |  ^~~~~~~~~~~~~
include/kvm_util_base.h:216:2: note: in expansion of macro 
‘kvm_do_ioctl’
   216 |  kvm_do_ioctl((vm)->fd, cmd, arg);   \
       |  ^~~~~~~~~~~~
include/kvm_util_base.h:308:9: note: in expansion of macro ‘__vm_ioctl’
   308 |  return __vm_ioctl(vm, KVM_RESET_DIRTY_RINGS, NULL);
       |         ^~~~~~~~~~
include/kvm_util_base.h: In function ‘vm_get_stats_fd’:
include/kvm_util_base.h:313:26: error: ‘KVM_GET_STATS_FD’ undeclared 
(first use in this function); did you mean ‘KVM_GET_SREGS’?
   313 |  int fd = __vm_ioctl(vm, KVM_GET_STATS_FD, NULL);
       |                          ^~~~~~~~~~~~~~~~

On 2022-09-30 16:05, Ido Schimmel wrote:
> On Fri, Sep 30, 2022 at 07:42:37AM +0200, netdev@kapio-technology.com 
> wrote:
>> Obviously my method of selecting all switchcore drivers with 
>> sub-options
>> under menuconfig was not sufficient, and I didn't know of the 
>> allmodconfig
>> option, otherwise I would have used it.
> 
> You can see build issues on patchwork:
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20220928150256.115248-6-netdev@kapio-technology.com/
> 
> Also:
> 
> https://docs.kernel.org/next/process/maintainer-netdev.html#what-level-of-testing-is-expected-before-i-submit-my-change
> 
> https://docs.kernel.org/next/process/maintainer-netdev.html#can-i-reproduce-the-checks-from-patchwork-on-my-local-machine
> 
> https://docs.kernel.org/next/process/maintainer-netdev.html#running-all-the-builds-and-checks-locally-is-a-pain-can-i-post-my-patches-and-have-the-patchwork-bot-validate-them
> 
>> So the question is if I should repost the fixed patch-set or I need to 
>> make
>> a new version?
> 
> A new fixed version (v7) is required, but wait for this version to be
> reviewed first.
> 
>> Anyhow I hope that there will not be problems when running the 
>> selftests, as
>> I have not been able to do so with my system, so there can be more 
>> that
>> needs to be changed.
> 
> It's not really acceptable to post tests that you haven't run... What
> exactly is the issue? You should be able to run the tests with veth
> pairs in a VM.

It is only the blackhole test that I have not been able to run as is, 
but I have stepped it manually as far as I could.
My environment has changed lately and in that context the building of 
the selftests fails and I don't know why,I just get some error 
messagesabout missing header files, and setting up a whole system like 
f.ex. linuxfromscratch with the necessary libs and tools to run it in a 
VM is too time consuming a task at the moment.

If there is some freely available system for the purpose out there 
besides my own system based on Buildroot that does not work now, please 
let me know...

On Fri, 30 Sep 2022 17:05:20 +0300 Ido Schimmel wrote:
> You can see build issues on patchwork:

Overall a helpful response, but that part you got wrong.

Do not point people to patchwork checks, please. It will only encourage
people to post stuff they haven't build tested themselves.

It's documented:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#running-all-the-builds-and-checks-locally-is-a-pain-can-i-post-my-patches-and-have-the-patchwork-bot-validate-them

Only people who helped write the code and maintain the infra can decide
how to use it which means me, Kees, or Hangbin. Please and thank you :S

On Fri, Sep 30, 2022 at 07:52:04AM -0700, Jakub Kicinski wrote:
> On Fri, 30 Sep 2022 17:05:20 +0300 Ido Schimmel wrote:
> > You can see build issues on patchwork:
> 
> Overall a helpful response, but that part you got wrong.
> 
> Do not point people to patchwork checks, please. It will only encourage
> people to post stuff they haven't build tested themselves.
> 
> It's documented:
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#running-all-the-builds-and-checks-locally-is-a-pain-can-i-post-my-patches-and-have-the-patchwork-bot-validate-them

Did you read my reply? I specifically included this link so that you
won't tell me I'm encouraging people to build test their patches by
posting to netdev.

On Fri, 30 Sep 2022 18:04:12 +0300 Ido Schimmel wrote:
> On Fri, Sep 30, 2022 at 07:52:04AM -0700, Jakub Kicinski wrote:
> > On Fri, 30 Sep 2022 17:05:20 +0300 Ido Schimmel wrote:  
> > > You can see build issues on patchwork:  
> > 
> > Overall a helpful response, but that part you got wrong.
> > 
> > Do not point people to patchwork checks, please. It will only encourage
> > people to post stuff they haven't build tested themselves.
> > 
> > It's documented:
> > 
> > https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#running-all-the-builds-and-checks-locally-is-a-pain-can-i-post-my-patches-and-have-the-patchwork-bot-validate-them  
> 
> Did you read my reply? I specifically included this link so that you
> won't tell me I'm encouraging people to build test their patches by
> posting to netdev.

Yeah, I noticed the link after, but I think my point stands. 

You show someone the patchwork checks and then at the end of the "also"
section put a link on how it's not okay to abuse it. Not a clear enough
instruction.

*never* point people to the patchwork checks, *please*