| Message ID | 75fbd18f79badee2ba4303e48ce0e7922e5421d1.1722357745.git.pabeni@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: introduce TX H/W shaping API | expand |
On 7/30/24 22:39, Paolo Abeni wrote: > Leverage a basic/dummy netdevsim implementation to do functional > coverage for NL interface. > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> FTR, it looks like the CI build went wild around this patch, but the failures look unrelated to the actual changes here. i.e.: https://netdev.bots.linux.dev/static/nipa/875223/13747883/build_clang/stderr Per-file breakdown --- /tmp/tmp.z9wI8zevoA 2024-07-30 18:26:37.281153512 -0700 +++ /tmp/tmp.pwD35f06q6 2024-07-30 18:26:37.285153598 -0700 @@ -0,0 +1,13 @@ + 4 ../drivers/block/drbd/drbd_bitmap.c + 4 ../drivers/block/drbd/drbd_main.c + 2 ../drivers/firmware/broadcom/bcm47xx_sprom.c + 6 ../drivers/most/most_usb.c + 1 ../drivers/net/ethernet/sfc/ptp.c + 1 ../drivers/net/ethernet/sfc/siena/ptp.c + 7 ../include/linux/fortify-string.h + 2 ../include/linux/kern_levels.h + 2 ../include/linux/printk.h + 2 ../kernel/audit.c + 1 ../lib/vsprintf.c + 3 ../net/ipv4/tcp_lp.c + 2 ../security/apparmor/lsm.c Still we hit a similar issue on the previous iteration, so perhaps there is some correlation I don't see?!?
On Wed, 31 Jul 2024 09:52:38 +0200 Paolo Abeni wrote: > On 7/30/24 22:39, Paolo Abeni wrote: > > Leverage a basic/dummy netdevsim implementation to do functional > > coverage for NL interface. > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > FTR, it looks like the CI build went wild around this patch, but the > failures look unrelated to the actual changes here. i.e.: > > https://netdev.bots.linux.dev/static/nipa/875223/13747883/build_clang/stderr Could you dig deeper? The scripts are doing incremental builds, and changes to Kconfig confuse them. You should be able to run the build script as a normal bash script, directly, it only needs a small handful of exported env variables. I have been trying to massage this for a while, my last change is: https://github.com/linux-netdev/nipa/commit/5bcb890cbfecd3c1727cec2f026360646a4afc62
On Wed, Jul 31, 2024 at 06:55:11PM -0700, Jakub Kicinski wrote: > On Wed, 31 Jul 2024 09:52:38 +0200 Paolo Abeni wrote: > > On 7/30/24 22:39, Paolo Abeni wrote: > > > Leverage a basic/dummy netdevsim implementation to do functional > > > coverage for NL interface. > > > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > > > > FTR, it looks like the CI build went wild around this patch, but the > > failures look unrelated to the actual changes here. i.e.: > > > > https://netdev.bots.linux.dev/static/nipa/875223/13747883/build_clang/stderr > > Could you dig deeper? > > The scripts are doing incremental builds, and changes to Kconfig > confuse them. You should be able to run the build script as a normal > bash script, directly, it only needs a small handful of exported > env variables. > > I have been trying to massage this for a while, my last change is: > https://github.com/linux-netdev/nipa/commit/5bcb890cbfecd3c1727cec2f026360646a4afc62 > Thanks Jakub, I am looking into this. So far I believe it relate to a Kconfig change activating new code. But reproducing the problem is proving a little tricky.
On Mon, 5 Aug 2024 15:22:53 +0100 Simon Horman wrote: > On Wed, Jul 31, 2024 at 06:55:11PM -0700, Jakub Kicinski wrote: > > On Wed, 31 Jul 2024 09:52:38 +0200 Paolo Abeni wrote: > > > FTR, it looks like the CI build went wild around this patch, but the > > > failures look unrelated to the actual changes here. i.e.: > > > > > > https://netdev.bots.linux.dev/static/nipa/875223/13747883/build_clang/stderr > > > > Could you dig deeper? > > > > The scripts are doing incremental builds, and changes to Kconfig > > confuse them. You should be able to run the build script as a normal > > bash script, directly, it only needs a small handful of exported > > env variables. > > > > I have been trying to massage this for a while, my last change is: > > https://github.com/linux-netdev/nipa/commit/5bcb890cbfecd3c1727cec2f026360646a4afc62 > > > > Thanks Jakub, > > I am looking into this. > So far I believe it relate to a Kconfig change activating new code. > But reproducing the problem is proving a little tricky. Have you tried twiddling / exporting FIRST_IN_SERIES ? See here for the 4 possible exports the test will look at: https://github.com/linux-netdev/nipa/blob/6112db7d472660450c69457c98ab37b431063301/core/test.py#L124
On Mon, Aug 05, 2024 at 12:36:55PM -0700, Jakub Kicinski wrote: > On Mon, 5 Aug 2024 15:22:53 +0100 Simon Horman wrote: > > On Wed, Jul 31, 2024 at 06:55:11PM -0700, Jakub Kicinski wrote: > > > On Wed, 31 Jul 2024 09:52:38 +0200 Paolo Abeni wrote: > > > > FTR, it looks like the CI build went wild around this patch, but the > > > > failures look unrelated to the actual changes here. i.e.: > > > > > > > > https://netdev.bots.linux.dev/static/nipa/875223/13747883/build_clang/stderr > > > > > > Could you dig deeper? > > > > > > The scripts are doing incremental builds, and changes to Kconfig > > > confuse them. You should be able to run the build script as a normal > > > bash script, directly, it only needs a small handful of exported > > > env variables. > > > > > > I have been trying to massage this for a while, my last change is: > > > https://github.com/linux-netdev/nipa/commit/5bcb890cbfecd3c1727cec2f026360646a4afc62 > > > > > > > Thanks Jakub, > > > > I am looking into this. > > So far I believe it relate to a Kconfig change activating new code. > > But reproducing the problem is proving a little tricky. > > Have you tried twiddling / exporting FIRST_IN_SERIES ? > > See here for the 4 possible exports the test will look at: > > https://github.com/linux-netdev/nipa/blob/6112db7d472660450c69457c98ab37b431063301/core/test.py#L124 Thanks, I will look into that.
On Tue, Aug 06, 2024 at 04:22:03PM +0100, Simon Horman wrote: > On Mon, Aug 05, 2024 at 12:36:55PM -0700, Jakub Kicinski wrote: > > On Mon, 5 Aug 2024 15:22:53 +0100 Simon Horman wrote: > > > On Wed, Jul 31, 2024 at 06:55:11PM -0700, Jakub Kicinski wrote: > > > > On Wed, 31 Jul 2024 09:52:38 +0200 Paolo Abeni wrote: > > > > > FTR, it looks like the CI build went wild around this patch, but the > > > > > failures look unrelated to the actual changes here. i.e.: > > > > > > > > > > https://netdev.bots.linux.dev/static/nipa/875223/13747883/build_clang/stderr > > > > > > > > Could you dig deeper? > > > > > > > > The scripts are doing incremental builds, and changes to Kconfig > > > > confuse them. You should be able to run the build script as a normal > > > > bash script, directly, it only needs a small handful of exported > > > > env variables. > > > > > > > > I have been trying to massage this for a while, my last change is: > > > > https://github.com/linux-netdev/nipa/commit/5bcb890cbfecd3c1727cec2f026360646a4afc62 > > > > > > > > > > Thanks Jakub, > > > > > > I am looking into this. > > > So far I believe it relate to a Kconfig change activating new code. > > > But reproducing the problem is proving a little tricky. > > > > Have you tried twiddling / exporting FIRST_IN_SERIES ? > > > > See here for the 4 possible exports the test will look at: > > > > https://github.com/linux-netdev/nipa/blob/6112db7d472660450c69457c98ab37b431063301/core/test.py#L124 > > Thanks, I will look into that. Hi Jakub, Thanks again for the information. I have now taken another look at this problem. Firstly, my analysis is that the cause of the problem is a combination of the way the patchset is constricted, and the way that the build tests (I have focussed on build_allmodconfig_warn.sh [1]). [1] https://github.com/linux-netdev/nipa/blob/main/tests/patch/build_allmodconfig_warn/build_allmodconfig.sh What I believe happens is this: The patches 01/12 - 07/12 modify some header files, adds a new Kconfig entry, and does a bunch of other normal stuff. Each of those patches is tested in turn, and everything seems fine. Then we get to patch 08/12. The key thing about this patch is that it enables the CONFIG_NET_SHAPER Kconfig option, in the context of an allmodconfig build. That in turn modifies the headers include/linux/netdevice.h and net/core/dev.h (and net/Makefile). Not in the in terms of their on-disk contents changing, but rather in the case of the header files, in terms of preprocessor output. And this is, I believe, where everything goes wrong. NIPA arrives at running build_allmodconfig_warn.sh for patch 08/12 with the tree built for the previous patch, 07/12. It then: * touches $output_dir/include/generated/utsrelease.h * checks out HEAD~ (patch 07/12) * prepares the kernel config * builds kernel and records incumbent errors (49) The thing to note here is that the tree has been little perturbed since build tests were run for patch 07/12, and thus few files are rebuilt. Moving on, simplifying things, the following then runs: * touches $output_dir/include/generated/utsrelease.h * checks out $HEAD (patch 08/12) * prepares the kernel config * builds kernel and records current errors (4219) The key to understanding why the large delta between 49 and 4219 is that vastly more files have been rebuilt. Because the preprocessor output of netdevice.h and dev.h have changes since the last build, and those headers are included, directly or indirectly, by a lot of files (and compilation results in warnings for many of those files). I was able to reproduce the result by running build_allmodconfig_warn.sh over patch 07/12 and then 07/12 with FIRST_IN_SERIES=0. I was able to get the desired result no new compiler warnings by doing the same again, but with FIRST_IN_SERIES=1 for the invocation of build_allmodconfig_warn.sh for 08/12. I believe this is entirely due to a baseline rebuild being run due to the FIRST_IN_SERIES=1 parameter. And, FWIIW, I believe the invocation of build_allmodconfig_warn.sh for 07/12 ensures reproducibility. My suggestion is that while we may consider reorganising the patch-set, that is really only a work around. And it would be best to make the CI more robust in the presence of such constructions. It may be a bit heavy handed, but my tested solution is to invoke a baseline rebuild if a Kconfig change is made. At the very last it does address the problem at hand. (In precisely the same way as manually setting FIRST_IN_SERIES=1.) The patch implementing this for build_allmodconfig.sh which I tested is below. If we want to go ahead with this approach then I expect it is best to add it to other build tests too. But this seems to be a good point to report my findings, so here we are. --- build_allmodconfig.sh.orig 2024-08-08 07:30:56.599372164 +0000 +++ build_allmodconfig.sh 2024-08-08 09:58:22.692206313 +0000 @@ -34,8 +34,10 @@ echo "Tree base:" git log -1 --pretty='%h ("%s")' HEAD~ -if [ x$FIRST_IN_SERIES == x0 ]; then - echo "Skip baseline build, not the first patch" +if [ x$FIRST_IN_SERIES == x0 ] && \ + ! git diff --name-only HEAD~ | grep -q -E "Kconfig$" +then + echo "Skip baseline build, not the first patch and no Kconfig updates" else echo "Baseline building the tree"
On Thu, 8 Aug 2024 13:20:42 +0100 Simon Horman wrote: > Thanks again for the information. > > I have now taken another look at this problem. > > Firstly, my analysis is that the cause of the problem is a combination of > the way the patchset is constricted, and the way that the build tests (I > have focussed on build_allmodconfig_warn.sh [1]). > > [1] https://github.com/linux-netdev/nipa/blob/main/tests/patch/build_allmodconfig_warn/build_allmodconfig.sh > > What I believe happens is this: The patches 01/12 - 07/12 modify some > header files, adds a new Kconfig entry, and does a bunch of other normal > stuff. Each of those patches is tested in turn, and everything seems fine. > > Then we get to patch 08/12. The key thing about this patch is that it > enables the CONFIG_NET_SHAPER Kconfig option, in the context of an > allmodconfig build. That in turn modifies the headers > include/linux/netdevice.h and net/core/dev.h (and net/Makefile). Not in the > in terms of their on-disk contents changing, but rather in the case of the > header files, in terms of preprocessor output. And this is, I believe, > where everything goes wrong. That's strange, make does not understand preprocessor, does it? Either file has been modified or it has not. I guess it doesn't matter, given your solution > NIPA arrives at running build_allmodconfig_warn.sh for patch 08/12 with the tree > built for the previous patch, 07/12. It then: > > * touches $output_dir/include/generated/utsrelease.h > * checks out HEAD~ (patch 07/12) > * prepares the kernel config > * builds kernel and records incumbent errors (49) > > The thing to note here is that the tree has been little perturbed since build > tests were run for patch 07/12, and thus few files are rebuilt. > > Moving on, simplifying things, the following then runs: > > * touches $output_dir/include/generated/utsrelease.h > * checks out $HEAD (patch 08/12) > * prepares the kernel config > * builds kernel and records current errors (4219) > > The key to understanding why the large delta between 49 and 4219 is > that vastly more files have been rebuilt. Because the preprocessor output > of netdevice.h and dev.h have changes since the last build, and those > headers are included, directly or indirectly, by a lot of files (and > compilation results in warnings for many of those files). > > > I was able to reproduce the result by running build_allmodconfig_warn.sh > over patch 07/12 and then 07/12 with FIRST_IN_SERIES=0. > > I was able to get the desired result no new compiler warnings > by doing the same again, but with FIRST_IN_SERIES=1 for the > invocation of build_allmodconfig_warn.sh for 08/12. > > I believe this is entirely due to a baseline rebuild being run due to the > FIRST_IN_SERIES=1 parameter. And, FWIIW, I believe the invocation of > build_allmodconfig_warn.sh for 07/12 ensures reproducibility. > > My suggestion is that while we may consider reorganising the patch-set, > that is really only a work around. And it would be best to make the CI more > robust in the presence of such constructions. > > It may be a bit heavy handed, but my tested solution is to invoke a > baseline rebuild if a Kconfig change is made. At the very last it does > address the problem at hand. (In precisely the same way as manually setting > FIRST_IN_SERIES=1.) > > The patch implementing this for build_allmodconfig.sh which I tested is > below. If we want to go ahead with this approach then I expect it is best > to add it to other build tests too. But this seems to be a good point > to report my findings, so here we are. > > --- build_allmodconfig.sh.orig 2024-08-08 07:30:56.599372164 +0000 > +++ build_allmodconfig.sh 2024-08-08 09:58:22.692206313 +0000 > @@ -34,8 +34,10 @@ > echo "Tree base:" > git log -1 --pretty='%h ("%s")' HEAD~ > > -if [ x$FIRST_IN_SERIES == x0 ]; then > - echo "Skip baseline build, not the first patch" > +if [ x$FIRST_IN_SERIES == x0 ] && \ > + ! git diff --name-only HEAD~ | grep -q -E "Kconfig$" > +then > + echo "Skip baseline build, not the first patch and no Kconfig updates" > else > echo "Baseline building the tree" Excellent idea, let's try it! Could you send a PR to NIPA? Note that the code is copied 3 times for each flavor of building :(
On Thu, Aug 08, 2024 at 07:17:54AM -0700, Jakub Kicinski wrote: > On Thu, 8 Aug 2024 13:20:42 +0100 Simon Horman wrote: > > Thanks again for the information. > > > > I have now taken another look at this problem. > > > > Firstly, my analysis is that the cause of the problem is a combination of > > the way the patchset is constricted, and the way that the build tests (I > > have focussed on build_allmodconfig_warn.sh [1]). > > > > [1] https://github.com/linux-netdev/nipa/blob/main/tests/patch/build_allmodconfig_warn/build_allmodconfig.sh > > > > What I believe happens is this: The patches 01/12 - 07/12 modify some > > header files, adds a new Kconfig entry, and does a bunch of other normal > > stuff. Each of those patches is tested in turn, and everything seems fine. > > > > Then we get to patch 08/12. The key thing about this patch is that it > > enables the CONFIG_NET_SHAPER Kconfig option, in the context of an > > allmodconfig build. That in turn modifies the headers > > include/linux/netdevice.h and net/core/dev.h (and net/Makefile). Not in the > > in terms of their on-disk contents changing, but rather in the case of the > > header files, in terms of preprocessor output. And this is, I believe, > > where everything goes wrong. > > That's strange, make does not understand preprocessor, does it? > Either file has been modified or it has not. True, that is a good point. I would say there is something deeper going on than I have been able to discover at this time: probably something very obvious. > I guess it doesn't matter, given your solution Right, in any case I think the script needs to be enhanced. My solution may prove heavy handed, but it can be improved over time. > > NIPA arrives at running build_allmodconfig_warn.sh for patch 08/12 with the tree > > built for the previous patch, 07/12. It then: > > > > * touches $output_dir/include/generated/utsrelease.h > > * checks out HEAD~ (patch 07/12) > > * prepares the kernel config > > * builds kernel and records incumbent errors (49) > > > > The thing to note here is that the tree has been little perturbed since build > > tests were run for patch 07/12, and thus few files are rebuilt. > > > > Moving on, simplifying things, the following then runs: > > > > * touches $output_dir/include/generated/utsrelease.h > > * checks out $HEAD (patch 08/12) > > * prepares the kernel config > > * builds kernel and records current errors (4219) > > > > The key to understanding why the large delta between 49 and 4219 is > > that vastly more files have been rebuilt. Because the preprocessor output > > of netdevice.h and dev.h have changes since the last build, and those > > headers are included, directly or indirectly, by a lot of files (and > > compilation results in warnings for many of those files). > > > > > > I was able to reproduce the result by running build_allmodconfig_warn.sh > > over patch 07/12 and then 07/12 with FIRST_IN_SERIES=0. Correction: this should have read "07/12 and then 08/12" > > I was able to get the desired result no new compiler warnings > > by doing the same again, but with FIRST_IN_SERIES=1 for the > > invocation of build_allmodconfig_warn.sh for 08/12. > > > > I believe this is entirely due to a baseline rebuild being run due to the > > FIRST_IN_SERIES=1 parameter. And, FWIIW, I believe the invocation of > > build_allmodconfig_warn.sh for 07/12 ensures reproducibility. > > > > My suggestion is that while we may consider reorganising the patch-set, > > that is really only a work around. And it would be best to make the CI more > > robust in the presence of such constructions. > > > > It may be a bit heavy handed, but my tested solution is to invoke a > > baseline rebuild if a Kconfig change is made. At the very last it does > > address the problem at hand. (In precisely the same way as manually setting > > FIRST_IN_SERIES=1.) > > > > The patch implementing this for build_allmodconfig.sh which I tested is > > below. If we want to go ahead with this approach then I expect it is best > > to add it to other build tests too. But this seems to be a good point > > to report my findings, so here we are. > > > > --- build_allmodconfig.sh.orig 2024-08-08 07:30:56.599372164 +0000 > > +++ build_allmodconfig.sh 2024-08-08 09:58:22.692206313 +0000 > > @@ -34,8 +34,10 @@ > > echo "Tree base:" > > git log -1 --pretty='%h ("%s")' HEAD~ > > > > -if [ x$FIRST_IN_SERIES == x0 ]; then > > - echo "Skip baseline build, not the first patch" > > +if [ x$FIRST_IN_SERIES == x0 ] && \ > > + ! git diff --name-only HEAD~ | grep -q -E "Kconfig$" > > +then > > + echo "Skip baseline build, not the first patch and no Kconfig updates" > > else > > echo "Baseline building the tree" > > Excellent idea, let's try it! Could you send a PR to NIPA? Yes, can do. > Note that the code is copied 3 times for each flavor of building :( I assumed so :)
On Thu, Aug 08, 2024 at 03:34:43PM +0100, Simon Horman wrote: > On Thu, Aug 08, 2024 at 07:17:54AM -0700, Jakub Kicinski wrote: > > On Thu, 8 Aug 2024 13:20:42 +0100 Simon Horman wrote: ... > > > It may be a bit heavy handed, but my tested solution is to invoke a > > > baseline rebuild if a Kconfig change is made. At the very last it does > > > address the problem at hand. (In precisely the same way as manually setting > > > FIRST_IN_SERIES=1.) > > > > > > The patch implementing this for build_allmodconfig.sh which I tested is > > > below. If we want to go ahead with this approach then I expect it is best > > > to add it to other build tests too. But this seems to be a good point > > > to report my findings, so here we are. > > > > > > --- build_allmodconfig.sh.orig 2024-08-08 07:30:56.599372164 +0000 > > > +++ build_allmodconfig.sh 2024-08-08 09:58:22.692206313 +0000 > > > @@ -34,8 +34,10 @@ > > > echo "Tree base:" > > > git log -1 --pretty='%h ("%s")' HEAD~ > > > > > > -if [ x$FIRST_IN_SERIES == x0 ]; then > > > - echo "Skip baseline build, not the first patch" > > > +if [ x$FIRST_IN_SERIES == x0 ] && \ > > > + ! git diff --name-only HEAD~ | grep -q -E "Kconfig$" > > > +then > > > + echo "Skip baseline build, not the first patch and no Kconfig updates" > > > else > > > echo "Baseline building the tree" > > > > Excellent idea, let's try it! Could you send a PR to NIPA? > > Yes, can do. For reference, the PR is here: https://github.com/linux-netdev/nipa/pull/35
On Sun, 11 Aug 2024 13:40:53 +0100 Simon Horman wrote: > > > Excellent idea, let's try it! Could you send a PR to NIPA? > > > > Yes, can do. > > For reference, the PR is here: > https://github.com/linux-netdev/nipa/pull/35 Deployed now, thank you!
On 8/8/24 16:17, Jakub Kicinski wrote: > On Thu, 8 Aug 2024 13:20:42 +0100 Simon Horman wrote: >> Thanks again for the information. >> >> I have now taken another look at this problem. >> >> Firstly, my analysis is that the cause of the problem is a combination of >> the way the patchset is constricted, and the way that the build tests (I >> have focussed on build_allmodconfig_warn.sh [1]). >> >> [1] https://github.com/linux-netdev/nipa/blob/main/tests/patch/build_allmodconfig_warn/build_allmodconfig.sh >> >> What I believe happens is this: The patches 01/12 - 07/12 modify some >> header files, adds a new Kconfig entry, and does a bunch of other normal >> stuff. Each of those patches is tested in turn, and everything seems fine. >> >> Then we get to patch 08/12. The key thing about this patch is that it >> enables the CONFIG_NET_SHAPER Kconfig option, in the context of an >> allmodconfig build. That in turn modifies the headers >> include/linux/netdevice.h and net/core/dev.h (and net/Makefile). Not in the >> in terms of their on-disk contents changing, but rather in the case of the >> header files, in terms of preprocessor output. And this is, I believe, >> where everything goes wrong. > > That's strange, make does not understand preprocessor, does it? AFICS kbuild creates a file for each enabled knob under include/config/. Then, for each kernel object target, it creates a .cmd file including the list of all dependencies. Such list comprises all the included files _and_ the relevant, mentioned "knob" file under include/config/ scripts/basic/fixdep is responsible for including the "knob files" in the dependency list. To test the above: make drivers/net/ethernet/intel/ice/ice_main.o touch include/config/NET_SHAPER make V=2 drivers/net/ethernet/intel/ice/ice_main.o CALL scripts/checksyscalls.sh - due to target is PHONY DESCEND objtool INSTALL libsubcmd_headers CC drivers/net/ethernet/intel/ice/ice_main.o - due to: include/config/NET_SHAPER Cheers, Paolo
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 9920b3a68ed1..1fd5acdc73c6 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -641,6 +641,7 @@ config NETDEVSIM depends on PTP_1588_CLOCK_MOCK || PTP_1588_CLOCK_MOCK=n select NET_DEVLINK select PAGE_POOL + select NET_SHAPER help This driver is a developer testing tool and software model that can be used to test various control path networking APIs, especially diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c index 017a6102be0a..ab9c3d5055e7 100644 --- a/drivers/net/netdevsim/netdev.c +++ b/drivers/net/netdevsim/netdev.c @@ -22,6 +22,7 @@ #include <net/netdev_queues.h> #include <net/page_pool/helpers.h> #include <net/netlink.h> +#include <net/net_shaper.h> #include <net/pkt_cls.h> #include <net/rtnetlink.h> #include <net/udp_tunnel.h> @@ -475,6 +476,41 @@ static int nsim_stop(struct net_device *dev) return 0; } +static int nsim_shaper_set(struct net_device *dev, + const struct net_shaper_info *shaper, + struct netlink_ext_ack *extack) +{ + return 0; +} + +static int nsim_shaper_del(struct net_device *dev, u32 handles, + struct netlink_ext_ack *extack) +{ + return 0; +} + +static int nsim_shaper_group(struct net_device *dev, int nr_inputs, + const struct net_shaper_info *inputs, + const struct net_shaper_info *output, + struct netlink_ext_ack *extack) +{ + return 0; +} + +static int nsim_shaper_cap(struct net_device *dev, enum net_shaper_scope scope, + unsigned long *flags) +{ + *flags = ULONG_MAX; + return 0; +} + +static const struct net_shaper_ops nsim_shaper_ops = { + .set = nsim_shaper_set, + .delete = nsim_shaper_del, + .group = nsim_shaper_group, + .capabilities = nsim_shaper_cap, +}; + static const struct net_device_ops nsim_netdev_ops = { .ndo_start_xmit = nsim_start_xmit, .ndo_set_rx_mode = nsim_set_rx_mode, @@ -496,6 +532,7 @@ static const struct net_device_ops nsim_netdev_ops = { .ndo_bpf = nsim_bpf, .ndo_open = nsim_open, .ndo_stop = nsim_stop, + .net_shaper_ops = &nsim_shaper_ops, }; static const struct net_device_ops nsim_vf_netdev_ops = { diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile index e54f382bcb02..432306f11664 100644 --- a/tools/testing/selftests/drivers/net/Makefile +++ b/tools/testing/selftests/drivers/net/Makefile @@ -6,6 +6,7 @@ TEST_PROGS := \ ping.py \ queues.py \ stats.py \ + shaper.py # end of TEST_PROGS include ../../lib.mk diff --git a/tools/testing/selftests/drivers/net/shaper.py b/tools/testing/selftests/drivers/net/shaper.py new file mode 100755 index 000000000000..ed34573bb4f6 --- /dev/null +++ b/tools/testing/selftests/drivers/net/shaper.py @@ -0,0 +1,267 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 + +from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_true, KsftSkipEx +from lib.py import ShaperFamily +from lib.py import NetDrvEnv +from lib.py import NlError +from lib.py import cmd +import glob +import sys + +def get_shapers(cfg, nl_shaper) -> None: + try: + shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True) + except NlError as e: + if e.error == 95: + raise KsftSkipEx("shapers not supported by the device") + raise + + # default configuration, no shapers configured + ksft_eq(len(shapers), 0) + +def get_caps(cfg, nl_shaper) -> None: + try: + caps = nl_shaper.cap_get({'ifindex': cfg.ifindex}, dump=True) + except NlError as e: + if e.error == 95: + raise KsftSkipEx("shapers not supported by the device") + raise + + # each device implementing shaper support must support some + # features in at least a scope + ksft_true(len(caps)> 0) + +def set_qshapers(cfg, nl_shaper) -> None: + try: + caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'queue'}) + except NlError as e: + if e.error == 95: + cfg.queues = False; + raise KsftSkipEx("shapers not supported by the device") + raise + if not 'support-bw-max' in caps or not 'support-metric-bps' in caps: + raise KsftSkipEx("device does not support queue scope shapers with bw_max and metric bps") + + nl_shaper.set({'ifindex': cfg.ifindex, + 'shaper': { 'handle': { 'scope': 'queue', 'id': 1 }, 'metric': 'bps', 'bw-max': 10000 }}) + nl_shaper.set({'ifindex': cfg.ifindex, + 'shaper': { 'handle': { 'scope': 'queue', 'id': 2 }, 'metric': 'bps', 'bw-max': 20000 }}) + + # querying a specific shaper not yet configured must fail + raised = False + try: + shaper_q0 = nl_shaper.get({'ifindex': cfg.ifindex, 'handle': { 'scope': 'queue', 'id': 0}}) + except (NlError): + raised = True + ksft_eq(raised, True) + + shaper_q1 = nl_shaper.get({'ifindex': cfg.ifindex, 'handle': { 'scope': 'queue', 'id': 1 }}) + ksft_eq(shaper_q1, { 'parent': { 'scope': 'netdev', 'id': 0 }, + 'handle': { 'scope': 'queue', 'id': 1 }, + 'metric': 'bps', + 'bw-min': 0, + 'bw-max': 10000, + 'burst': 0, + 'priority': 0, + 'weight': 0 }) + + shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True) + ksft_eq(shapers, [{'parent': { 'scope': 'netdev', 'id': 0 }, + 'handle': { 'scope': 'queue', 'id': 1 }, + 'metric': 'bps', + 'bw-min': 0, + 'bw-max': 10000, + 'burst': 0, + 'priority': 0, + 'weight': 0 }, + {'parent': { 'scope': 'netdev', 'id': 0 }, + 'handle': { 'scope': 'queue', 'id': 2 }, + 'metric': 'bps', + 'bw-min': 0, + 'bw-max': 20000, + 'burst': 0, + 'priority': 0, + 'weight': 0 }]) + +def del_qshapers(cfg, nl_shaper) -> None: + if not cfg.queues: + raise KsftSkipEx("queue shapers not supported by device, skipping delete") + + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'queue', 'id': 2}}) + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'queue', 'id': 1}}) + shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True) + ksft_eq(len(shapers), 0) + +def set_nshapers(cfg, nl_shaper) -> None: + # check required features + try: + caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'netdev'}) + except NlError as e: + if e.error == 95: + cfg.netdev = False; + raise KsftSkipEx("shapers not supported by the device") + raise + if not 'support-bw-max' in caps or not 'support-metric-bps' in caps: + raise KsftSkipEx("device does not support nested netdev scope shapers with weight") + + nl_shaper.set({'ifindex': cfg.ifindex, + 'shaper': { 'handle': { 'scope': 'netdev', 'id': 0 }, 'bw-max': 100000 }}) + + shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True) + ksft_eq(shapers, [{'parent': { 'scope': 'port', 'id': 0 }, + 'handle': { 'scope': 'netdev', 'id': 0 }, + 'metric': 'bps', + 'bw-min': 0, + 'bw-max': 100000, + 'burst': 0, + 'priority': 0, + 'weight': 0 }]) + +def del_nshapers(cfg, nl_shaper) -> None: + if not cfg.netdev: + raise KsftSkipEx("netdev shaper not supported by device, skipping delete") + + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'netdev'}}) + shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True) + ksft_eq(len(shapers), 0) + +def basic_groups(cfg, nl_shaper) -> None: + if not cfg.netdev: + raise KsftSkipEx("netdev shaper not supported by the device") + try: + caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'queue'}) + except NlError as e: + if e.error == 95: + cfg.queues = False; + raise KsftSkipEx("shapers not supported by the device") + raise + if not 'support-weight' in caps: + raise KsftSkipEx("device does not support queue scope shapers with weight") + + output_handle = nl_shaper.group({'ifindex': cfg.ifindex, + 'inputs':[{ 'handle': { 'scope': 'queue', 'id': 1 },'weight': 1 }, + { 'handle': { 'scope': 'queue', 'id': 2 }, 'weight': 2 }], + 'output': { 'handle': {'scope':'netdev'}, 'metric': 'bps', 'bw-max': 10000}}) + output_id = output_handle['handle']['id'] + + shaper = nl_shaper.get({'ifindex': cfg.ifindex, 'handle': { 'scope': 'queue', 'id': 1 }}) + ksft_eq(shaper, {'parent': { 'scope': 'netdev', 'id': 0 }, + 'handle': { 'scope': 'queue', 'id': 1 }, + 'metric': 'bps', + 'bw-min': 0, + 'bw-max': 0, + 'burst': 0, + 'priority': 0, + 'weight': 1 }) + + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'queue', 'id': 2}}) + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'queue', 'id': 1}}) + + # deleting all the inputs shaper does not affect the output one + # when the latter has 'netdev' scope + shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True) + ksft_eq(len(shapers), 1) + + nl_shaper.delete({'ifindex': cfg.ifindex, 'handle': { 'scope': 'netdev'}}) + +def qgroups(cfg, nl_shaper) -> None: + try: + caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'detached'}) + except NlError as e: + if e.error == 95: + raise KsftSkipEx("shapers not supported by the device") + raise + if not 'support-bw-max' in caps or not 'support-metric-bps' in caps: + raise KsftSkipEx("device does not support detached scope shapers with bw_max and metric bps") + try: + caps = nl_shaper.cap_get({'ifindex': cfg.ifindex, 'scope':'queue'}) + except NlError as e: + if e.error == 95: + raise KsftSkipEx("shapers not supported by the device") + raise + if not 'support-nesting' in caps or not 'support-weight' in caps or not 'support-metric-bps' in caps: + raise KsftSkipEx("device does not support nested queue scope shapers with weight") + + output_handle = nl_shaper.group({'ifindex': cfg.ifindex, + 'inputs':[{ 'handle': { 'scope': 'queue', 'id': 1 }, 'metric': 'bps', 'weight': 3 }, + { 'handle': { 'scope': 'queue', 'id': 2 }, 'metric': 'bps', 'weight': 2 }], + 'output': { 'handle': {'scope':'detached'}, 'metric': 'bps', 'bw-max': 10000}}) + output_id = output_handle['handle']['id'] + + shaper = nl_shaper.get({'ifindex': cfg.ifindex, 'handle': { 'scope': 'queue', 'id': 1 }}) + ksft_eq(shaper, {'parent': { 'scope': 'detached', 'id': output_id }, + 'handle': { 'scope': 'queue', 'id': 1 }, + 'metric': 'bps', + 'bw-min': 0, + 'bw-max': 0, + 'burst': 0, + 'priority': 0, + 'weight': 3 }) + + # grouping to a specified, not existing detached scope shaper must fail + raised = False + try: + nl_shaper.group({'ifindex': cfg.ifindex, + 'inputs':[ { 'handle': { 'scope': 'queue', 'id': 3 }, 'metric': 'bps', 'weight': 3 }], + 'output': { 'handle': {'scope':'detached', 'id': output_id + 1 }, 'metric': 'bps', 'bw-max': 10000}}) + except (NlError): + raised = True + ksft_eq(raised, True) + + output_handle = nl_shaper.group({'ifindex': cfg.ifindex, + 'inputs':[ { 'handle': { 'scope': 'queue', 'id': 3 }, 'metric': 'bps', 'weight': 4 }], + 'output': { 'handle': {'scope':'detached', 'id': output_id} }}) + + shaper = nl_shaper.get({'ifindex': cfg.ifindex, 'handle': { 'scope': 'queue', 'id': 3 }}) + ksft_eq(shaper, {'parent': { 'scope': 'detached', 'id': 0 }, + 'handle': { 'scope': 'queue', 'id': 3 }, + 'metric': 'bps', + 'bw-min': 0, + 'bw-max': 0, + 'burst': 0, + 'priority': 0, + 'weight': 4 }) + + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'queue', 'id': 2}}) + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'queue', 'id': 1}}) + + # deleting a non empty group mast fail + raised = False + try: + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'detached', 'id': output_id }}) + except (NlError): + raised = True + ksft_eq(raised, True) + nl_shaper.delete({'ifindex': cfg.ifindex, + 'handle': { 'scope': 'queue', 'id': 3}}) + + # the detached scope shaper deletion is implicit + shapers = nl_shaper.get({'ifindex': cfg.ifindex}, dump=True) + ksft_eq(len(shapers), 0) + +def main() -> None: + with NetDrvEnv(__file__, queue_count=4) as cfg: + cfg.queues = True + cfg.netdev = True + ksft_run([get_shapers, + get_caps, + set_qshapers, + del_qshapers, + set_nshapers, + del_nshapers, + basic_groups, + qgroups], args=(cfg, ShaperFamily())) + ksft_exit() + + +if __name__ == "__main__": + main() diff --git a/tools/testing/selftests/net/lib/py/__init__.py b/tools/testing/selftests/net/lib/py/__init__.py index b6d498d125fe..ef1aa52f4761 100644 --- a/tools/testing/selftests/net/lib/py/__init__.py +++ b/tools/testing/selftests/net/lib/py/__init__.py @@ -6,3 +6,4 @@ from .netns import NetNS from .nsim import * from .utils import * from .ynl import NlError, YnlFamily, EthtoolFamily, NetdevFamily, RtnlFamily +from .ynl import ShaperFamily diff --git a/tools/testing/selftests/net/lib/py/ynl.py b/tools/testing/selftests/net/lib/py/ynl.py index 1ace58370c06..42e825905ade 100644 --- a/tools/testing/selftests/net/lib/py/ynl.py +++ b/tools/testing/selftests/net/lib/py/ynl.py @@ -47,3 +47,8 @@ class NetdevFamily(YnlFamily): def __init__(self): super().__init__((SPEC_PATH / Path('netdev.yaml')).as_posix(), schema='') + +class ShaperFamily(YnlFamily): + def __init__(self): + super().__init__((SPEC_PATH / Path('shaper.yaml')).as_posix(), + schema='')
Leverage a basic/dummy netdevsim implementation to do functional coverage for NL interface. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- rfc v1 -> v2: - added more test-cases WRT nesting and grouping --- drivers/net/Kconfig | 1 + drivers/net/netdevsim/netdev.c | 37 +++ tools/testing/selftests/drivers/net/Makefile | 1 + tools/testing/selftests/drivers/net/shaper.py | 267 ++++++++++++++++++ .../testing/selftests/net/lib/py/__init__.py | 1 + tools/testing/selftests/net/lib/py/ynl.py | 5 + 6 files changed, 312 insertions(+) create mode 100755 tools/testing/selftests/drivers/net/shaper.py