[PATCH bpf 1/2] xdp, bonding: Fix feature flags when there are no slave devs anymore

[PATCH bpf 1/2] xdp, bonding: Fix feature flags when there are no slave devs anymore

[Daniel Borkmann]

Commit 9b0ed890ac2a ("bonding: do not report NETDEV_XDP_ACT_XSK_ZEROCOPY")
changed the driver from reporting everything as supported before a device
was bonded into having the driver report that no XDP feature is supported
until a real device is bonded as it seems to be more truthful given
eventually real underlying devices decide what XDP features are supported.

The change however did not take into account when all slave devices get
removed from the bond device. In this case after 9b0ed890ac2a, the driver
keeps reporting a feature mask of 0x77, that is, NETDEV_XDP_ACT_MASK &
~NETDEV_XDP_ACT_XSK_ZEROCOPY whereas it should have reported a feature
mask of 0.

Fix it by resetting XDP feature flags in the same way as if no XDP program
is attached to the bond device. This was uncovered by the XDP bond selftest
which let BPF CI fail. After adjusting the starting masks on the latter
to 0 instead of NETDEV_XDP_ACT_MASK the test passes again together with
this fix.

Fixes: 9b0ed890ac2a ("bonding: do not report NETDEV_XDP_ACT_XSK_ZEROCOPY")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Cc: Prashant Batra <prbatra.mail@gmail.com>
Cc: Toke Høiland-Jørgensen <toke@redhat.com>
Cc: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/bonding/bond_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a11748b8d69b..cd0683bcca03 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1811,7 +1811,7 @@ void bond_xdp_set_features(struct net_device *bond_dev)
 
 	ASSERT_RTNL();
 
-	if (!bond_xdp_check(bond)) {
+	if (!bond_xdp_check(bond) || !bond_has_slaves(bond)) {
 		xdp_clear_features_flag(bond_dev);
 		return;
 	}
-- 
2.34.1

[PATCH bpf 2/2] selftests/bpf: Fix up xdp bonding test wrt feature flags

[Daniel Borkmann]

Adjust the XDP feature flags for the bond device when no bond slave
devices are attached. After 9b0ed890ac2a ("bonding: do not report
NETDEV_XDP_ACT_XSK_ZEROCOPY"), the empty bond device must report 0
as flags instead of NETDEV_XDP_ACT_MASK.

  # ./vmtest.sh -- ./test_progs -t xdp_bond
  [...]
  [    3.983311] bond1 (unregistering): (slave veth1_1): Releasing backup interface
  [    3.995434] bond1 (unregistering): Released all slaves
  [    4.022311] bond2: (slave veth2_1): Releasing backup interface
  #507/1   xdp_bonding/xdp_bonding_attach:OK
  #507/2   xdp_bonding/xdp_bonding_nested:OK
  #507/3   xdp_bonding/xdp_bonding_features:OK
  #507/4   xdp_bonding/xdp_bonding_roundrobin:OK
  #507/5   xdp_bonding/xdp_bonding_activebackup:OK
  #507/6   xdp_bonding/xdp_bonding_xor_layer2:OK
  #507/7   xdp_bonding/xdp_bonding_xor_layer23:OK
  #507/8   xdp_bonding/xdp_bonding_xor_layer34:OK
  #507/9   xdp_bonding/xdp_bonding_redirect_multi:OK
  #507     xdp_bonding:OK
  Summary: 1/9 PASSED, 0 SKIPPED, 0 FAILED
  [    4.185255] bond2 (unregistering): Released all slaves
  [...]

Fixes: 9b0ed890ac2a ("bonding: do not report NETDEV_XDP_ACT_XSK_ZEROCOPY")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tools/testing/selftests/bpf/prog_tests/xdp_bonding.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
index c3b45745cbcc..6d8b54124cb3 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_bonding.c
@@ -511,7 +511,7 @@ static void test_xdp_bonding_features(struct skeletons *skeletons)
 	if (!ASSERT_OK(err, "bond bpf_xdp_query"))
 		goto out;
 
-	if (!ASSERT_EQ(query_opts.feature_flags, NETDEV_XDP_ACT_MASK,
+	if (!ASSERT_EQ(query_opts.feature_flags, 0,
 		       "bond query_opts.feature_flags"))
 		goto out;
 
@@ -601,7 +601,7 @@ static void test_xdp_bonding_features(struct skeletons *skeletons)
 	if (!ASSERT_OK(err, "bond bpf_xdp_query"))
 		goto out;
 
-	ASSERT_EQ(query_opts.feature_flags, NETDEV_XDP_ACT_MASK,
+	ASSERT_EQ(query_opts.feature_flags, 0,
 		  "bond query_opts.feature_flags");
 out:
 	bpf_link__destroy(link);
-- 
2.34.1

Re: [PATCH bpf 2/2] selftests/bpf: Fix up xdp bonding test wrt feature flags

[Toke Høiland-Jørgensen]

Daniel Borkmann <daniel@iogearbox.net> writes:

> Adjust the XDP feature flags for the bond device when no bond slave
> devices are attached. After 9b0ed890ac2a ("bonding: do not report
> NETDEV_XDP_ACT_XSK_ZEROCOPY"), the empty bond device must report 0
> as flags instead of NETDEV_XDP_ACT_MASK.
>
>   # ./vmtest.sh -- ./test_progs -t xdp_bond
>   [...]
>   [    3.983311] bond1 (unregistering): (slave veth1_1): Releasing backup interface
>   [    3.995434] bond1 (unregistering): Released all slaves
>   [    4.022311] bond2: (slave veth2_1): Releasing backup interface
>   #507/1   xdp_bonding/xdp_bonding_attach:OK
>   #507/2   xdp_bonding/xdp_bonding_nested:OK
>   #507/3   xdp_bonding/xdp_bonding_features:OK
>   #507/4   xdp_bonding/xdp_bonding_roundrobin:OK
>   #507/5   xdp_bonding/xdp_bonding_activebackup:OK
>   #507/6   xdp_bonding/xdp_bonding_xor_layer2:OK
>   #507/7   xdp_bonding/xdp_bonding_xor_layer23:OK
>   #507/8   xdp_bonding/xdp_bonding_xor_layer34:OK
>   #507/9   xdp_bonding/xdp_bonding_redirect_multi:OK
>   #507     xdp_bonding:OK
>   Summary: 1/9 PASSED, 0 SKIPPED, 0 FAILED
>   [    4.185255] bond2 (unregistering): Released all slaves
>   [...]
>
> Fixes: 9b0ed890ac2a ("bonding: do not report NETDEV_XDP_ACT_XSK_ZEROCOPY")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>

Re: [PATCH bpf 1/2] xdp, bonding: Fix feature flags when there are no slave devs anymore

[Toke Høiland-Jørgensen]

Daniel Borkmann <daniel@iogearbox.net> writes:

> Commit 9b0ed890ac2a ("bonding: do not report NETDEV_XDP_ACT_XSK_ZEROCOPY")
> changed the driver from reporting everything as supported before a device
> was bonded into having the driver report that no XDP feature is supported
> until a real device is bonded as it seems to be more truthful given
> eventually real underlying devices decide what XDP features are supported.
>
> The change however did not take into account when all slave devices get
> removed from the bond device. In this case after 9b0ed890ac2a, the driver
> keeps reporting a feature mask of 0x77, that is, NETDEV_XDP_ACT_MASK &
> ~NETDEV_XDP_ACT_XSK_ZEROCOPY whereas it should have reported a feature
> mask of 0.
>
> Fix it by resetting XDP feature flags in the same way as if no XDP program
> is attached to the bond device. This was uncovered by the XDP bond selftest
> which let BPF CI fail. After adjusting the starting masks on the latter
> to 0 instead of NETDEV_XDP_ACT_MASK the test passes again together with
> this fix.
>
> Fixes: 9b0ed890ac2a ("bonding: do not report NETDEV_XDP_ACT_XSK_ZEROCOPY")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Magnus Karlsson <magnus.karlsson@intel.com>
> Cc: Prashant Batra <prbatra.mail@gmail.com>
> Cc: Toke Høiland-Jørgensen <toke@redhat.com>
> Cc: Jakub Kicinski <kuba@kernel.org>

Ah yes, makes sense, of course we should reset the mask when devices are
removed!

Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>

Re: [PATCH bpf 1/2] xdp, bonding: Fix feature flags when there are no slave devs anymore

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Tue,  5 Mar 2024 10:08:28 +0100 you wrote:
> Commit 9b0ed890ac2a ("bonding: do not report NETDEV_XDP_ACT_XSK_ZEROCOPY")
> changed the driver from reporting everything as supported before a device
> was bonded into having the driver report that no XDP feature is supported
> until a real device is bonded as it seems to be more truthful given
> eventually real underlying devices decide what XDP features are supported.
> 
> The change however did not take into account when all slave devices get
> removed from the bond device. In this case after 9b0ed890ac2a, the driver
> keeps reporting a feature mask of 0x77, that is, NETDEV_XDP_ACT_MASK &
> ~NETDEV_XDP_ACT_XSK_ZEROCOPY whereas it should have reported a feature
> mask of 0.
> 
> [...]

Here is the summary with links:
  - [bpf,1/2] xdp, bonding: Fix feature flags when there are no slave devs anymore
    https://git.kernel.org/bpf/bpf/c/f267f2628150
  - [bpf,2/2] selftests/bpf: Fix up xdp bonding test wrt feature flags
    https://git.kernel.org/bpf/bpf/c/0bfc0336e134

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH bpf 1/2] xdp, bonding: Fix feature flags when there are no slave devs anymore

[Pu Lehui]

On 2024/3/6 8:30, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This series was applied to bpf/bpf.git (master)
> by Alexei Starovoitov <ast@kernel.org>:
> 
> On Tue,  5 Mar 2024 10:08:28 +0100 you wrote:
>> Commit 9b0ed890ac2a ("bonding: do not report NETDEV_XDP_ACT_XSK_ZEROCOPY")
>> changed the driver from reporting everything as supported before a device
>> was bonded into having the driver report that no XDP feature is supported
>> until a real device is bonded as it seems to be more truthful given
>> eventually real underlying devices decide what XDP features are supported.
>>
>> The change however did not take into account when all slave devices get
>> removed from the bond device. In this case after 9b0ed890ac2a, the driver
>> keeps reporting a feature mask of 0x77, that is, NETDEV_XDP_ACT_MASK &
>> ~NETDEV_XDP_ACT_XSK_ZEROCOPY whereas it should have reported a feature
>> mask of 0.
>>
>> [...]
> 
> Here is the summary with links:
>    - [bpf,1/2] xdp, bonding: Fix feature flags when there are no slave devs anymore
>      https://git.kernel.org/bpf/bpf/c/f267f2628150
>    - [bpf,2/2] selftests/bpf: Fix up xdp bonding test wrt feature flags
>      https://git.kernel.org/bpf/bpf/c/0bfc0336e134

I had encountered the same issue during riscv bpf selftest regression. 
Happy to see this fixes.

> 
> You are awesome, thank you!