[PATCH net] bonding: check xdp prog when set bond mode

Wang Liang posted 1 patch 9 months, 1 week ago
There is a newer version of this series
drivers/net/bonding/bond_options.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH net] bonding: check xdp prog when set bond mode
Posted by Wang Liang 9 months, 1 week ago
Following operations can trigger a warning[1]:

    ip netns add ns1
    ip netns exec ns1 ip link add bond0 type bond mode balance-rr
    ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
    ip netns exec ns1 ip link set bond0 type bond mode broadcast
    ip netns del ns1

When delete the namespace, dev_xdp_uninstall() is called to remove xdp
program on bond dev, and bond_xdp_set() will check the bond mode. If bond
mode is changed after attaching xdp program, the warning may occur.

Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
with xdp program attached is not good. Add check for xdp program when set
bond mode.

    [1]
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
    Modules linked in:
    CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
    Workqueue: netns cleanup_net
    RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
    Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
    RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
    RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
    RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
    RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
    R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
    R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
    FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
    Call Trace:
     <TASK>
     ? __warn+0x83/0x130
     ? unregister_netdevice_many_notify+0x8d9/0x930
     ? report_bug+0x18e/0x1a0
     ? handle_bug+0x54/0x90
     ? exc_invalid_op+0x18/0x70
     ? asm_exc_invalid_op+0x1a/0x20
     ? unregister_netdevice_many_notify+0x8d9/0x930
     ? bond_net_exit_batch_rtnl+0x5c/0x90
     cleanup_net+0x237/0x3d0
     process_one_work+0x163/0x390
     worker_thread+0x293/0x3b0
     ? __pfx_worker_thread+0x10/0x10
     kthread+0xec/0x1e0
     ? __pfx_kthread+0x10/0x10
     ? __pfx_kthread+0x10/0x10
     ret_from_fork+0x2f/0x50
     ? __pfx_kthread+0x10/0x10
     ret_from_fork_asm+0x1a/0x30
     </TASK>
    ---[ end trace 0000000000000000 ]---

Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver")
Signed-off-by: Wang Liang <wangliang74@huawei.com>
---
 drivers/net/bonding/bond_options.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 327b6ecdc77e..127181866829 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -868,6 +868,9 @@ static bool bond_set_xfrm_features(struct bonding *bond)
 static int bond_option_mode_set(struct bonding *bond,
 				const struct bond_opt_value *newval)
 {
+	if (bond->xdp_prog)
+		return -EOPNOTSUPP;
+
 	if (!bond_mode_uses_arp(newval->value)) {
 		if (bond->params.arp_interval) {
 			netdev_dbg(bond->dev, "%s mode is incompatible with arp monitoring, start mii monitoring\n",
-- 
2.34.1
Re: [PATCH net] bonding: check xdp prog when set bond mode
Posted by Toke Høiland-Jørgensen 9 months, 1 week ago
Wang Liang <wangliang74@huawei.com> writes:

> Following operations can trigger a warning[1]:
>
>     ip netns add ns1
>     ip netns exec ns1 ip link add bond0 type bond mode balance-rr
>     ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
>     ip netns exec ns1 ip link set bond0 type bond mode broadcast
>     ip netns del ns1
>
> When delete the namespace, dev_xdp_uninstall() is called to remove xdp
> program on bond dev, and bond_xdp_set() will check the bond mode. If bond
> mode is changed after attaching xdp program, the warning may occur.
>
> Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
> with xdp program attached is not good. Add check for xdp program when set
> bond mode.
>
>     [1]
>     ------------[ cut here ]------------
>     WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
>     Modules linked in:
>     CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
>     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
>     Workqueue: netns cleanup_net
>     RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
>     Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
>     RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
>     RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
>     RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
>     RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
>     R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
>     R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
>     FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
>     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>     CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
>     Call Trace:
>      <TASK>
>      ? __warn+0x83/0x130
>      ? unregister_netdevice_many_notify+0x8d9/0x930
>      ? report_bug+0x18e/0x1a0
>      ? handle_bug+0x54/0x90
>      ? exc_invalid_op+0x18/0x70
>      ? asm_exc_invalid_op+0x1a/0x20
>      ? unregister_netdevice_many_notify+0x8d9/0x930
>      ? bond_net_exit_batch_rtnl+0x5c/0x90
>      cleanup_net+0x237/0x3d0
>      process_one_work+0x163/0x390
>      worker_thread+0x293/0x3b0
>      ? __pfx_worker_thread+0x10/0x10
>      kthread+0xec/0x1e0
>      ? __pfx_kthread+0x10/0x10
>      ? __pfx_kthread+0x10/0x10
>      ret_from_fork+0x2f/0x50
>      ? __pfx_kthread+0x10/0x10
>      ret_from_fork_asm+0x1a/0x30
>      </TASK>
>     ---[ end trace 0000000000000000 ]---
>
> Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver")
> Signed-off-by: Wang Liang <wangliang74@huawei.com>
> ---
>  drivers/net/bonding/bond_options.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> index 327b6ecdc77e..127181866829 100644
> --- a/drivers/net/bonding/bond_options.c
> +++ b/drivers/net/bonding/bond_options.c
> @@ -868,6 +868,9 @@ static bool bond_set_xfrm_features(struct bonding *bond)
>  static int bond_option_mode_set(struct bonding *bond,
>  				const struct bond_opt_value *newval)
>  {
> +	if (bond->xdp_prog)
> +		return -EOPNOTSUPP;
> +

Should we allow changing as long as the new mode also supports XDP?

-Toke
Re: [PATCH net] bonding: check xdp prog when set bond mode
Posted by Nikolay Aleksandrov 9 months, 1 week ago
On 3/14/25 12:13 PM, Toke Høiland-Jørgensen wrote:
> Wang Liang <wangliang74@huawei.com> writes:
> 
>> Following operations can trigger a warning[1]:
>>
>>     ip netns add ns1
>>     ip netns exec ns1 ip link add bond0 type bond mode balance-rr
>>     ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
>>     ip netns exec ns1 ip link set bond0 type bond mode broadcast
>>     ip netns del ns1
>>
>> When delete the namespace, dev_xdp_uninstall() is called to remove xdp
>> program on bond dev, and bond_xdp_set() will check the bond mode. If bond
>> mode is changed after attaching xdp program, the warning may occur.
>>
>> Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
>> with xdp program attached is not good. Add check for xdp program when set
>> bond mode.
>>
>>     [1]
>>     ------------[ cut here ]------------
>>     WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
>>     Modules linked in:
>>     CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
>>     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
>>     Workqueue: netns cleanup_net
>>     RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
>>     Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
>>     RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
>>     RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
>>     RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
>>     RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
>>     R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
>>     R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
>>     FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
>>     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>     CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
>>     Call Trace:
>>      <TASK>
>>      ? __warn+0x83/0x130
>>      ? unregister_netdevice_many_notify+0x8d9/0x930
>>      ? report_bug+0x18e/0x1a0
>>      ? handle_bug+0x54/0x90
>>      ? exc_invalid_op+0x18/0x70
>>      ? asm_exc_invalid_op+0x1a/0x20
>>      ? unregister_netdevice_many_notify+0x8d9/0x930
>>      ? bond_net_exit_batch_rtnl+0x5c/0x90
>>      cleanup_net+0x237/0x3d0
>>      process_one_work+0x163/0x390
>>      worker_thread+0x293/0x3b0
>>      ? __pfx_worker_thread+0x10/0x10
>>      kthread+0xec/0x1e0
>>      ? __pfx_kthread+0x10/0x10
>>      ? __pfx_kthread+0x10/0x10
>>      ret_from_fork+0x2f/0x50
>>      ? __pfx_kthread+0x10/0x10
>>      ret_from_fork_asm+0x1a/0x30
>>      </TASK>
>>     ---[ end trace 0000000000000000 ]---
>>
>> Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver")
>> Signed-off-by: Wang Liang <wangliang74@huawei.com>
>> ---
>>  drivers/net/bonding/bond_options.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>> index 327b6ecdc77e..127181866829 100644
>> --- a/drivers/net/bonding/bond_options.c
>> +++ b/drivers/net/bonding/bond_options.c
>> @@ -868,6 +868,9 @@ static bool bond_set_xfrm_features(struct bonding *bond)
>>  static int bond_option_mode_set(struct bonding *bond,
>>  				const struct bond_opt_value *newval)
>>  {
>> +	if (bond->xdp_prog)
>> +		return -EOPNOTSUPP;
>> +
> 
> Should we allow changing as long as the new mode also supports XDP?
> 
> -Toke
> 
> 

+1
I think we should allow it, the best way probably is to add a new
BOND_VALFLAG_XDP_UNSUPP (for example) as a bond option flag and to set
it in bond_options.c for each mode that doesn't support XDP, then you
can do the check in a generic way (for any option) in
bond_opt_check_deps. Any bond option that can't be changed with XDP prog
should have that flag set.

Cheers,
 Nik

Re: [PATCH net] bonding: check xdp prog when set bond mode
Posted by Nikolay Aleksandrov 9 months, 1 week ago
On 3/14/25 12:22 PM, Nikolay Aleksandrov wrote:
> On 3/14/25 12:13 PM, Toke Høiland-Jørgensen wrote:
>> Wang Liang <wangliang74@huawei.com> writes:
>>
>>> Following operations can trigger a warning[1]:
>>>
>>>     ip netns add ns1
>>>     ip netns exec ns1 ip link add bond0 type bond mode balance-rr
>>>     ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
>>>     ip netns exec ns1 ip link set bond0 type bond mode broadcast
>>>     ip netns del ns1
>>>
>>> When delete the namespace, dev_xdp_uninstall() is called to remove xdp
>>> program on bond dev, and bond_xdp_set() will check the bond mode. If bond
>>> mode is changed after attaching xdp program, the warning may occur.
>>>
>>> Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
>>> with xdp program attached is not good. Add check for xdp program when set
>>> bond mode.
>>>
>>>     [1]
>>>     ------------[ cut here ]------------
>>>     WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
>>>     Modules linked in:
>>>     CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
>>>     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
>>>     Workqueue: netns cleanup_net
>>>     RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
>>>     Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
>>>     RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
>>>     RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
>>>     RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
>>>     RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
>>>     R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
>>>     R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
>>>     FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
>>>     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>     CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
>>>     Call Trace:
>>>      <TASK>
>>>      ? __warn+0x83/0x130
>>>      ? unregister_netdevice_many_notify+0x8d9/0x930
>>>      ? report_bug+0x18e/0x1a0
>>>      ? handle_bug+0x54/0x90
>>>      ? exc_invalid_op+0x18/0x70
>>>      ? asm_exc_invalid_op+0x1a/0x20
>>>      ? unregister_netdevice_many_notify+0x8d9/0x930
>>>      ? bond_net_exit_batch_rtnl+0x5c/0x90
>>>      cleanup_net+0x237/0x3d0
>>>      process_one_work+0x163/0x390
>>>      worker_thread+0x293/0x3b0
>>>      ? __pfx_worker_thread+0x10/0x10
>>>      kthread+0xec/0x1e0
>>>      ? __pfx_kthread+0x10/0x10
>>>      ? __pfx_kthread+0x10/0x10
>>>      ret_from_fork+0x2f/0x50
>>>      ? __pfx_kthread+0x10/0x10
>>>      ret_from_fork_asm+0x1a/0x30
>>>      </TASK>
>>>     ---[ end trace 0000000000000000 ]---
>>>
>>> Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver")
>>> Signed-off-by: Wang Liang <wangliang74@huawei.com>
>>> ---
>>>  drivers/net/bonding/bond_options.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>> index 327b6ecdc77e..127181866829 100644
>>> --- a/drivers/net/bonding/bond_options.c
>>> +++ b/drivers/net/bonding/bond_options.c
>>> @@ -868,6 +868,9 @@ static bool bond_set_xfrm_features(struct bonding *bond)
>>>  static int bond_option_mode_set(struct bonding *bond,
>>>  				const struct bond_opt_value *newval)
>>>  {
>>> +	if (bond->xdp_prog)
>>> +		return -EOPNOTSUPP;
>>> +
>>
>> Should we allow changing as long as the new mode also supports XDP?
>>
>> -Toke
>>
>>
> 
> +1
> I think we should allow it, the best way probably is to add a new option 
> BOND_VALFLAG_XDP_UNSUPP (for example) as a bond option flag and to set
> it in bond_options.c for each mode that doesn't support XDP, then you
> can do the check in a generic way (for any option) in
> bond_opt_check_deps. Any bond option that can't be changed with XDP prog

err, I meant any bond option's value that isn't supported with XDP, for
a whole option it would be a bit different

> should have that flag set.
> 
> Cheers,
>  Nik
> 

Re: [PATCH net] bonding: check xdp prog when set bond mode
Posted by Wang Liang 9 months ago
在 2025/3/14 18:44, Nikolay Aleksandrov 写道:
> On 3/14/25 12:22 PM, Nikolay Aleksandrov wrote:
>> On 3/14/25 12:13 PM, Toke Høiland-Jørgensen wrote:
>>> Wang Liang <wangliang74@huawei.com> writes:
>>>
>>>> Following operations can trigger a warning[1]:
>>>>
>>>>      ip netns add ns1
>>>>      ip netns exec ns1 ip link add bond0 type bond mode balance-rr
>>>>      ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
>>>>      ip netns exec ns1 ip link set bond0 type bond mode broadcast
>>>>      ip netns del ns1
>>>>
>>>> When delete the namespace, dev_xdp_uninstall() is called to remove xdp
>>>> program on bond dev, and bond_xdp_set() will check the bond mode. If bond
>>>> mode is changed after attaching xdp program, the warning may occur.
>>>>
>>>> Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
>>>> with xdp program attached is not good. Add check for xdp program when set
>>>> bond mode.
>>>>
>>>>      [1]
>>>>      ------------[ cut here ]------------
>>>>      WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
>>>>      Modules linked in:
>>>>      CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
>>>>      Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
>>>>      Workqueue: netns cleanup_net
>>>>      RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
>>>>      Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
>>>>      RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
>>>>      RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
>>>>      RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
>>>>      RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
>>>>      R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
>>>>      R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
>>>>      FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
>>>>      CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>      CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
>>>>      Call Trace:
>>>>       <TASK>
>>>>       ? __warn+0x83/0x130
>>>>       ? unregister_netdevice_many_notify+0x8d9/0x930
>>>>       ? report_bug+0x18e/0x1a0
>>>>       ? handle_bug+0x54/0x90
>>>>       ? exc_invalid_op+0x18/0x70
>>>>       ? asm_exc_invalid_op+0x1a/0x20
>>>>       ? unregister_netdevice_many_notify+0x8d9/0x930
>>>>       ? bond_net_exit_batch_rtnl+0x5c/0x90
>>>>       cleanup_net+0x237/0x3d0
>>>>       process_one_work+0x163/0x390
>>>>       worker_thread+0x293/0x3b0
>>>>       ? __pfx_worker_thread+0x10/0x10
>>>>       kthread+0xec/0x1e0
>>>>       ? __pfx_kthread+0x10/0x10
>>>>       ? __pfx_kthread+0x10/0x10
>>>>       ret_from_fork+0x2f/0x50
>>>>       ? __pfx_kthread+0x10/0x10
>>>>       ret_from_fork_asm+0x1a/0x30
>>>>       </TASK>
>>>>      ---[ end trace 0000000000000000 ]---
>>>>
>>>> Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver")
>>>> Signed-off-by: Wang Liang <wangliang74@huawei.com>
>>>> ---
>>>>   drivers/net/bonding/bond_options.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>>> index 327b6ecdc77e..127181866829 100644
>>>> --- a/drivers/net/bonding/bond_options.c
>>>> +++ b/drivers/net/bonding/bond_options.c
>>>> @@ -868,6 +868,9 @@ static bool bond_set_xfrm_features(struct bonding *bond)
>>>>   static int bond_option_mode_set(struct bonding *bond,
>>>>   				const struct bond_opt_value *newval)
>>>>   {
>>>> +	if (bond->xdp_prog)
>>>> +		return -EOPNOTSUPP;
>>>> +
>>> Should we allow changing as long as the new mode also supports XDP?
>>>
>>> -Toke
>>>
>>>
>> +1
>> I think we should allow it, the best way probably is to add a new option
>> BOND_VALFLAG_XDP_UNSUPP (for example) as a bond option flag and to set
>> it in bond_options.c for each mode that doesn't support XDP, then you
>> can do the check in a generic way (for any option) in
>> bond_opt_check_deps. Any bond option that can't be changed with XDP prog
> err, I meant any bond option's value that isn't supported with XDP, for
> a whole option it would be a bit different
Thanks for your suggestions!

When install xdp prog, bond_xdp_set() use bond_xdp_check() to check 
whether the bond mode support xdp.

When uninstall xdp prog, the paramter prog of bond_xdp_set() is NULL. 
How about not call bond_xdp_check() to avoid the warning when the prog 
is NULL, like:

static int bond_xdp_set(struct net_device *dev, struct bpf_prog *prog,
             struct netlink_ext_ack *extack)
     ...
     if (prog && !bond_xdp_check(bond))
>> should have that flag set.
>>
>> Cheers,
>>   Nik
>>
>
Re: [PATCH net] bonding: check xdp prog when set bond mode
Posted by Nikolay Aleksandrov 9 months ago
On 3/17/25 06:07, Wang Liang wrote:
> 
> 在 2025/3/14 18:44, Nikolay Aleksandrov 写道:
>> On 3/14/25 12:22 PM, Nikolay Aleksandrov wrote:
>>> On 3/14/25 12:13 PM, Toke Høiland-Jørgensen wrote:
>>>> Wang Liang <wangliang74@huawei.com> writes:
>>>>
>>>>> Following operations can trigger a warning[1]:
>>>>>
>>>>>      ip netns add ns1
>>>>>      ip netns exec ns1 ip link add bond0 type bond mode balance-rr
>>>>>      ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
>>>>>      ip netns exec ns1 ip link set bond0 type bond mode broadcast
>>>>>      ip netns del ns1
>>>>>
>>>>> When delete the namespace, dev_xdp_uninstall() is called to remove xdp
>>>>> program on bond dev, and bond_xdp_set() will check the bond mode. If bond
>>>>> mode is changed after attaching xdp program, the warning may occur.
>>>>>
>>>>> Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
>>>>> with xdp program attached is not good. Add check for xdp program when set
>>>>> bond mode.
>>>>>
>>>>>      [1]
>>>>>      ------------[ cut here ]------------
>>>>>      WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
>>>>>      Modules linked in:
>>>>>      CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
>>>>>      Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
>>>>>      Workqueue: netns cleanup_net
>>>>>      RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
>>>>>      Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
>>>>>      RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
>>>>>      RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
>>>>>      RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
>>>>>      RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
>>>>>      R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
>>>>>      R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
>>>>>      FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
>>>>>      CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>>      CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
>>>>>      Call Trace:
>>>>>       <TASK>
>>>>>       ? __warn+0x83/0x130
>>>>>       ? unregister_netdevice_many_notify+0x8d9/0x930
>>>>>       ? report_bug+0x18e/0x1a0
>>>>>       ? handle_bug+0x54/0x90
>>>>>       ? exc_invalid_op+0x18/0x70
>>>>>       ? asm_exc_invalid_op+0x1a/0x20
>>>>>       ? unregister_netdevice_many_notify+0x8d9/0x930
>>>>>       ? bond_net_exit_batch_rtnl+0x5c/0x90
>>>>>       cleanup_net+0x237/0x3d0
>>>>>       process_one_work+0x163/0x390
>>>>>       worker_thread+0x293/0x3b0
>>>>>       ? __pfx_worker_thread+0x10/0x10
>>>>>       kthread+0xec/0x1e0
>>>>>       ? __pfx_kthread+0x10/0x10
>>>>>       ? __pfx_kthread+0x10/0x10
>>>>>       ret_from_fork+0x2f/0x50
>>>>>       ? __pfx_kthread+0x10/0x10
>>>>>       ret_from_fork_asm+0x1a/0x30
>>>>>       </TASK>
>>>>>      ---[ end trace 0000000000000000 ]---
>>>>>
>>>>> Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver")
>>>>> Signed-off-by: Wang Liang <wangliang74@huawei.com>
>>>>> ---
>>>>>   drivers/net/bonding/bond_options.c | 3 +++
>>>>>   1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>>>> index 327b6ecdc77e..127181866829 100644
>>>>> --- a/drivers/net/bonding/bond_options.c
>>>>> +++ b/drivers/net/bonding/bond_options.c
>>>>> @@ -868,6 +868,9 @@ static bool bond_set_xfrm_features(struct bonding *bond)
>>>>>   static int bond_option_mode_set(struct bonding *bond,
>>>>>                   const struct bond_opt_value *newval)
>>>>>   {
>>>>> +    if (bond->xdp_prog)
>>>>> +        return -EOPNOTSUPP;
>>>>> +
>>>> Should we allow changing as long as the new mode also supports XDP?
>>>>
>>>> -Toke
>>>>
>>>>
>>> +1
>>> I think we should allow it, the best way probably is to add a new option
>>> BOND_VALFLAG_XDP_UNSUPP (for example) as a bond option flag and to set
>>> it in bond_options.c for each mode that doesn't support XDP, then you
>>> can do the check in a generic way (for any option) in
>>> bond_opt_check_deps. Any bond option that can't be changed with XDP prog
>> err, I meant any bond option's value that isn't supported with XDP, for
>> a whole option it would be a bit different
> Thanks for your suggestions!
> 
> When install xdp prog, bond_xdp_set() use bond_xdp_check() to check whether the bond mode support xdp.
> 
> When uninstall xdp prog, the paramter prog of bond_xdp_set() is NULL. How about not call bond_xdp_check() to avoid the warning when the prog is NULL, like:
> 
> static int bond_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>             struct netlink_ext_ack *extack)
>     ...
>     if (prog && !bond_xdp_check(bond))

No, this could cause other problems. Actually, for -net I think the best would be to stick to
a simpler fix and just do bond_xdp_check() if there's a XDP program attached when changing
the mode so it can be backported easier. The option value flag can be done in the future
if more option values (or options) need to be disabled for XDP.

Cheers,
 Nik

>>> should have that flag set.
>>>
>>> Cheers,
>>>   Nik
>>>
>>

Re: [PATCH net] bonding: check xdp prog when set bond mode
Posted by Wang Liang 9 months ago
在 2025/3/17 15:39, Nikolay Aleksandrov 写道:
> On 3/17/25 06:07, Wang Liang wrote:
>> 在 2025/3/14 18:44, Nikolay Aleksandrov 写道:
>>> On 3/14/25 12:22 PM, Nikolay Aleksandrov wrote:
>>>> On 3/14/25 12:13 PM, Toke Høiland-Jørgensen wrote:
>>>>> Wang Liang <wangliang74@huawei.com> writes:
>>>>>
>>>>>> Following operations can trigger a warning[1]:
>>>>>>
>>>>>>       ip netns add ns1
>>>>>>       ip netns exec ns1 ip link add bond0 type bond mode balance-rr
>>>>>>       ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
>>>>>>       ip netns exec ns1 ip link set bond0 type bond mode broadcast
>>>>>>       ip netns del ns1
>>>>>>
>>>>>> When delete the namespace, dev_xdp_uninstall() is called to remove xdp
>>>>>> program on bond dev, and bond_xdp_set() will check the bond mode. If bond
>>>>>> mode is changed after attaching xdp program, the warning may occur.
>>>>>>
>>>>>> Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
>>>>>> with xdp program attached is not good. Add check for xdp program when set
>>>>>> bond mode.
>>>>>>
>>>>>>       [1]
>>>>>>       ------------[ cut here ]------------
>>>>>>       WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
>>>>>>       Modules linked in:
>>>>>>       CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
>>>>>>       Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
>>>>>>       Workqueue: netns cleanup_net
>>>>>>       RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
>>>>>>       Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
>>>>>>       RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
>>>>>>       RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
>>>>>>       RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
>>>>>>       RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
>>>>>>       R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
>>>>>>       R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
>>>>>>       FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
>>>>>>       CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>>>       CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
>>>>>>       Call Trace:
>>>>>>        <TASK>
>>>>>>        ? __warn+0x83/0x130
>>>>>>        ? unregister_netdevice_many_notify+0x8d9/0x930
>>>>>>        ? report_bug+0x18e/0x1a0
>>>>>>        ? handle_bug+0x54/0x90
>>>>>>        ? exc_invalid_op+0x18/0x70
>>>>>>        ? asm_exc_invalid_op+0x1a/0x20
>>>>>>        ? unregister_netdevice_many_notify+0x8d9/0x930
>>>>>>        ? bond_net_exit_batch_rtnl+0x5c/0x90
>>>>>>        cleanup_net+0x237/0x3d0
>>>>>>        process_one_work+0x163/0x390
>>>>>>        worker_thread+0x293/0x3b0
>>>>>>        ? __pfx_worker_thread+0x10/0x10
>>>>>>        kthread+0xec/0x1e0
>>>>>>        ? __pfx_kthread+0x10/0x10
>>>>>>        ? __pfx_kthread+0x10/0x10
>>>>>>        ret_from_fork+0x2f/0x50
>>>>>>        ? __pfx_kthread+0x10/0x10
>>>>>>        ret_from_fork_asm+0x1a/0x30
>>>>>>        </TASK>
>>>>>>       ---[ end trace 0000000000000000 ]---
>>>>>>
>>>>>> Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver")
>>>>>> Signed-off-by: Wang Liang <wangliang74@huawei.com>
>>>>>> ---
>>>>>>    drivers/net/bonding/bond_options.c | 3 +++
>>>>>>    1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>>>>> index 327b6ecdc77e..127181866829 100644
>>>>>> --- a/drivers/net/bonding/bond_options.c
>>>>>> +++ b/drivers/net/bonding/bond_options.c
>>>>>> @@ -868,6 +868,9 @@ static bool bond_set_xfrm_features(struct bonding *bond)
>>>>>>    static int bond_option_mode_set(struct bonding *bond,
>>>>>>                    const struct bond_opt_value *newval)
>>>>>>    {
>>>>>> +    if (bond->xdp_prog)
>>>>>> +        return -EOPNOTSUPP;
>>>>>> +
>>>>> Should we allow changing as long as the new mode also supports XDP?
>>>>>
>>>>> -Toke
>>>>>
>>>>>
>>>> +1
>>>> I think we should allow it, the best way probably is to add a new option
>>>> BOND_VALFLAG_XDP_UNSUPP (for example) as a bond option flag and to set
>>>> it in bond_options.c for each mode that doesn't support XDP, then you
>>>> can do the check in a generic way (for any option) in
>>>> bond_opt_check_deps. Any bond option that can't be changed with XDP prog
>>> err, I meant any bond option's value that isn't supported with XDP, for
>>> a whole option it would be a bit different
>> Thanks for your suggestions!
>>
>> When install xdp prog, bond_xdp_set() use bond_xdp_check() to check whether the bond mode support xdp.
>>
>> When uninstall xdp prog, the paramter prog of bond_xdp_set() is NULL. How about not call bond_xdp_check() to avoid the warning when the prog is NULL, like:
>>
>> static int bond_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>>              struct netlink_ext_ack *extack)
>>      ...
>>      if (prog && !bond_xdp_check(bond))
> No, this could cause other problems. Actually, for -net I think the best would be to stick to
> a simpler fix and just do bond_xdp_check() if there's a XDP program attached when changing
> the mode so it can be backported easier. The option value flag can be done in the future
> if more option values (or options) need to be disabled for XDP.
>
> Cheers,
>   Nik
Excuse me again. I hope that I haven't misunderstood your suggestions.

When I try to do bond_xdp_check() if there's a XDP program attached when
changing the mode in function bond_option_mode_set(). bond_xdp_check()
visit bond->params.mode and bond->params.xmit_policy, should I create a
new function, whose paramter is int mode?

Looking forward to getting more replies. Thanks.
>>>> should have that flag set.
>>>>
>>>> Cheers,
>>>>    Nik
>>>>
>
Re: [PATCH net] bonding: check xdp prog when set bond mode
Posted by Nikolay Aleksandrov 9 months ago
On 3/18/25 08:50, Wang Liang wrote:
> 
> 在 2025/3/17 15:39, Nikolay Aleksandrov 写道:
>> On 3/17/25 06:07, Wang Liang wrote:
>>> 在 2025/3/14 18:44, Nikolay Aleksandrov 写道:
>>>> On 3/14/25 12:22 PM, Nikolay Aleksandrov wrote:
>>>>> On 3/14/25 12:13 PM, Toke Høiland-Jørgensen wrote:
>>>>>> Wang Liang <wangliang74@huawei.com> writes:
>>>>>>
>>>>>>> Following operations can trigger a warning[1]:
>>>>>>>
>>>>>>>       ip netns add ns1
>>>>>>>       ip netns exec ns1 ip link add bond0 type bond mode balance-rr
>>>>>>>       ip netns exec ns1 ip link set dev bond0 xdp obj af_xdp_kern.o sec xdp
>>>>>>>       ip netns exec ns1 ip link set bond0 type bond mode broadcast
>>>>>>>       ip netns del ns1
>>>>>>>
>>>>>>> When delete the namespace, dev_xdp_uninstall() is called to remove xdp
>>>>>>> program on bond dev, and bond_xdp_set() will check the bond mode. If bond
>>>>>>> mode is changed after attaching xdp program, the warning may occur.
>>>>>>>
>>>>>>> Some bond modes (broadcast, etc.) do not support native xdp. Set bond mode
>>>>>>> with xdp program attached is not good. Add check for xdp program when set
>>>>>>> bond mode.
>>>>>>>
>>>>>>>       [1]
>>>>>>>       ------------[ cut here ]------------
>>>>>>>       WARNING: CPU: 0 PID: 11 at net/core/dev.c:9912 unregister_netdevice_many_notify+0x8d9/0x930
>>>>>>>       Modules linked in:
>>>>>>>       CPU: 0 UID: 0 PID: 11 Comm: kworker/u4:0 Not tainted 6.14.0-rc4 #107
>>>>>>>       Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
>>>>>>>       Workqueue: netns cleanup_net
>>>>>>>       RIP: 0010:unregister_netdevice_many_notify+0x8d9/0x930
>>>>>>>       Code: 00 00 48 c7 c6 6f e3 a2 82 48 c7 c7 d0 b3 96 82 e8 9c 10 3e ...
>>>>>>>       RSP: 0018:ffffc90000063d80 EFLAGS: 00000282
>>>>>>>       RAX: 00000000ffffffa1 RBX: ffff888004959000 RCX: 00000000ffffdfff
>>>>>>>       RDX: 0000000000000000 RSI: 00000000ffffffea RDI: ffffc90000063b48
>>>>>>>       RBP: ffffc90000063e28 R08: ffffffff82d39b28 R09: 0000000000009ffb
>>>>>>>       R10: 0000000000000175 R11: ffffffff82d09b40 R12: ffff8880049598e8
>>>>>>>       R13: 0000000000000001 R14: dead000000000100 R15: ffffc90000045000
>>>>>>>       FS:  0000000000000000(0000) GS:ffff888007a00000(0000) knlGS:0000000000000000
>>>>>>>       CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>>>>       CR2: 000000000d406b60 CR3: 000000000483e000 CR4: 00000000000006f0
>>>>>>>       Call Trace:
>>>>>>>        <TASK>
>>>>>>>        ? __warn+0x83/0x130
>>>>>>>        ? unregister_netdevice_many_notify+0x8d9/0x930
>>>>>>>        ? report_bug+0x18e/0x1a0
>>>>>>>        ? handle_bug+0x54/0x90
>>>>>>>        ? exc_invalid_op+0x18/0x70
>>>>>>>        ? asm_exc_invalid_op+0x1a/0x20
>>>>>>>        ? unregister_netdevice_many_notify+0x8d9/0x930
>>>>>>>        ? bond_net_exit_batch_rtnl+0x5c/0x90
>>>>>>>        cleanup_net+0x237/0x3d0
>>>>>>>        process_one_work+0x163/0x390
>>>>>>>        worker_thread+0x293/0x3b0
>>>>>>>        ? __pfx_worker_thread+0x10/0x10
>>>>>>>        kthread+0xec/0x1e0
>>>>>>>        ? __pfx_kthread+0x10/0x10
>>>>>>>        ? __pfx_kthread+0x10/0x10
>>>>>>>        ret_from_fork+0x2f/0x50
>>>>>>>        ? __pfx_kthread+0x10/0x10
>>>>>>>        ret_from_fork_asm+0x1a/0x30
>>>>>>>        </TASK>
>>>>>>>       ---[ end trace 0000000000000000 ]---
>>>>>>>
>>>>>>> Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver")
>>>>>>> Signed-off-by: Wang Liang <wangliang74@huawei.com>
>>>>>>> ---
>>>>>>>    drivers/net/bonding/bond_options.c | 3 +++
>>>>>>>    1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>>>>>>> index 327b6ecdc77e..127181866829 100644
>>>>>>> --- a/drivers/net/bonding/bond_options.c
>>>>>>> +++ b/drivers/net/bonding/bond_options.c
>>>>>>> @@ -868,6 +868,9 @@ static bool bond_set_xfrm_features(struct bonding *bond)
>>>>>>>    static int bond_option_mode_set(struct bonding *bond,
>>>>>>>                    const struct bond_opt_value *newval)
>>>>>>>    {
>>>>>>> +    if (bond->xdp_prog)
>>>>>>> +        return -EOPNOTSUPP;
>>>>>>> +
>>>>>> Should we allow changing as long as the new mode also supports XDP?
>>>>>>
>>>>>> -Toke
>>>>>>
>>>>>>
>>>>> +1
>>>>> I think we should allow it, the best way probably is to add a new option
>>>>> BOND_VALFLAG_XDP_UNSUPP (for example) as a bond option flag and to set
>>>>> it in bond_options.c for each mode that doesn't support XDP, then you
>>>>> can do the check in a generic way (for any option) in
>>>>> bond_opt_check_deps. Any bond option that can't be changed with XDP prog
>>>> err, I meant any bond option's value that isn't supported with XDP, for
>>>> a whole option it would be a bit different
>>> Thanks for your suggestions!
>>>
>>> When install xdp prog, bond_xdp_set() use bond_xdp_check() to check whether the bond mode support xdp.
>>>
>>> When uninstall xdp prog, the paramter prog of bond_xdp_set() is NULL. How about not call bond_xdp_check() to avoid the warning when the prog is NULL, like:
>>>
>>> static int bond_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>>>              struct netlink_ext_ack *extack)
>>>      ...
>>>      if (prog && !bond_xdp_check(bond))
>> No, this could cause other problems. Actually, for -net I think the best would be to stick to
>> a simpler fix and just do bond_xdp_check() if there's a XDP program attached when changing
>> the mode so it can be backported easier. The option value flag can be done in the future
>> if more option values (or options) need to be disabled for XDP.
>>
>> Cheers,
>>   Nik
> Excuse me again. I hope that I haven't misunderstood your suggestions.
> 
> When I try to do bond_xdp_check() if there's a XDP program attached when
> changing the mode in function bond_option_mode_set(). bond_xdp_check()
> visit bond->params.mode and bond->params.xmit_policy, should I create a
> new function, whose paramter is int mode?
> 
> Looking forward to getting more replies. Thanks.

No need to create a new function, you can modify the existing and pass
mode as an argument, then use it to do this check when changing the mode.

>>>>> should have that flag set.
>>>>>
>>>>> Cheers,
>>>>>    Nik
>>>>>
>>