* [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program
@ 2024-08-23 8:42 Feng zhou
2024-08-23 11:55 ` Jiri Pirko
0 siblings, 1 reply; 6+ messages in thread
From: Feng zhou @ 2024-08-23 8:42 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, ast, daniel, hawk, john.fastabend,
jiri, bigeasy, lorenzo
Cc: netdev, linux-kernel, bpf, yangzhenze, wangdongdong.6,
zhoufeng.zf, Toke Høiland-Jørgensen
From: Feng Zhou <zhoufeng.zf@bytedance.com>
Cannot attach when an upper device already has a program, This
restriction is only for bond's slave devices or team port, and
should not be accidentally injured for devices like eth0 and vxlan0.
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
---
Changelog:
v1->v2: Addressed comments from Paolo Abeni, Jiri Pirko
- Use "netif_is_lag_port" relace of "netif_is_bond_slave"
Details in here:
https://lore.kernel.org/netdev/3bf84d23-a561-47ae-84a4-e99488fc762b@bytedance.com/T/
net/core/dev.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index f66e61407883..49144e62172e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9502,10 +9502,12 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
}
/* don't allow if an upper device already has a program */
- netdev_for_each_upper_dev_rcu(dev, upper, iter) {
- if (dev_xdp_prog_count(upper) > 0) {
- NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
- return -EEXIST;
+ if (netif_is_lag_port(dev)) {
+ netdev_for_each_upper_dev_rcu(dev, upper, iter) {
+ if (dev_xdp_prog_count(upper) > 0) {
+ NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
+ return -EEXIST;
+ }
}
}
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program
2024-08-23 8:42 [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program Feng zhou
@ 2024-08-23 11:55 ` Jiri Pirko
2024-08-23 12:07 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2024-08-23 11:55 UTC (permalink / raw)
To: Feng zhou
Cc: davem, edumazet, kuba, pabeni, ast, daniel, hawk, john.fastabend,
bigeasy, lorenzo, netdev, linux-kernel, bpf, yangzhenze,
wangdongdong.6, Toke Høiland-Jørgensen
Fri, Aug 23, 2024 at 10:42:04AM CEST, zhoufeng.zf@bytedance.com wrote:
>From: Feng Zhou <zhoufeng.zf@bytedance.com>
>
>Cannot attach when an upper device already has a program, This
>restriction is only for bond's slave devices or team port, and
>should not be accidentally injured for devices like eth0 and vxlan0.
What if I attach xdp program to solo netdev and then I enslave it
to bond/team netdev that already has xdp program attached?
What prevents me from doing that?
>
>Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
>Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
>---
>Changelog:
>v1->v2: Addressed comments from Paolo Abeni, Jiri Pirko
>- Use "netif_is_lag_port" relace of "netif_is_bond_slave"
>Details in here:
>https://lore.kernel.org/netdev/3bf84d23-a561-47ae-84a4-e99488fc762b@bytedance.com/T/
>
> net/core/dev.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index f66e61407883..49144e62172e 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -9502,10 +9502,12 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
> }
>
> /* don't allow if an upper device already has a program */
>- netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>- if (dev_xdp_prog_count(upper) > 0) {
>- NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>- return -EEXIST;
>+ if (netif_is_lag_port(dev)) {
>+ netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>+ if (dev_xdp_prog_count(upper) > 0) {
>+ NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>+ return -EEXIST;
>+ }
> }
> }
>
>--
>2.30.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program
2024-08-23 11:55 ` Jiri Pirko
@ 2024-08-23 12:07 ` Daniel Borkmann
2024-08-23 13:29 ` Jiri Pirko
2024-08-27 8:03 ` Feng Zhou
0 siblings, 2 replies; 6+ messages in thread
From: Daniel Borkmann @ 2024-08-23 12:07 UTC (permalink / raw)
To: Jiri Pirko, Feng zhou
Cc: davem, edumazet, kuba, pabeni, ast, hawk, john.fastabend, bigeasy,
lorenzo, netdev, linux-kernel, bpf, yangzhenze, wangdongdong.6,
Toke Høiland-Jørgensen
On 8/23/24 1:55 PM, Jiri Pirko wrote:
> Fri, Aug 23, 2024 at 10:42:04AM CEST, zhoufeng.zf@bytedance.com wrote:
>> From: Feng Zhou <zhoufeng.zf@bytedance.com>
>>
>> Cannot attach when an upper device already has a program, This
>> restriction is only for bond's slave devices or team port, and
>> should not be accidentally injured for devices like eth0 and vxlan0.
>
> What if I attach xdp program to solo netdev and then I enslave it
> to bond/team netdev that already has xdp program attached?
> What prevents me from doing that?
In that case the enslaving of the device to bond(/team) must fail as
otherwise the latter won't be able to propagate the XDP prog downwards.
Feng, did you double check if we have net or BPF selftest coverage for
that? If not might be good to add.
>> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
>> ---
>> Changelog:
>> v1->v2: Addressed comments from Paolo Abeni, Jiri Pirko
>> - Use "netif_is_lag_port" relace of "netif_is_bond_slave"
>> Details in here:
>> https://lore.kernel.org/netdev/3bf84d23-a561-47ae-84a4-e99488fc762b@bytedance.com/T/
>>
>> net/core/dev.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index f66e61407883..49144e62172e 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -9502,10 +9502,12 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
>> }
>>
>> /* don't allow if an upper device already has a program */
>> - netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>> - if (dev_xdp_prog_count(upper) > 0) {
>> - NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>> - return -EEXIST;
>> + if (netif_is_lag_port(dev)) {
>> + netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>> + if (dev_xdp_prog_count(upper) > 0) {
>> + NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>> + return -EEXIST;
>> + }
>> }
>> }
>>
>> --
>> 2.30.2
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program
2024-08-23 12:07 ` Daniel Borkmann
@ 2024-08-23 13:29 ` Jiri Pirko
2024-08-27 8:02 ` [External] " Feng Zhou
2024-08-27 8:03 ` Feng Zhou
1 sibling, 1 reply; 6+ messages in thread
From: Jiri Pirko @ 2024-08-23 13:29 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Feng zhou, davem, edumazet, kuba, pabeni, ast, hawk,
john.fastabend, bigeasy, lorenzo, netdev, linux-kernel, bpf,
yangzhenze, wangdongdong.6, Toke Høiland-Jørgensen
Fri, Aug 23, 2024 at 02:07:45PM CEST, daniel@iogearbox.net wrote:
>On 8/23/24 1:55 PM, Jiri Pirko wrote:
>> Fri, Aug 23, 2024 at 10:42:04AM CEST, zhoufeng.zf@bytedance.com wrote:
>> > From: Feng Zhou <zhoufeng.zf@bytedance.com>
>> >
>> > Cannot attach when an upper device already has a program, This
>> > restriction is only for bond's slave devices or team port, and
>> > should not be accidentally injured for devices like eth0 and vxlan0.
>>
>> What if I attach xdp program to solo netdev and then I enslave it
>> to bond/team netdev that already has xdp program attached?
>> What prevents me from doing that?
>
>In that case the enslaving of the device to bond(/team) must fail as
>otherwise the latter won't be able to propagate the XDP prog downwards.
Yep, I don't see that in the code though.
>
>Feng, did you double check if we have net or BPF selftest coverage for
>that? If not might be good to add.
>
>> > Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> > Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
>> > ---
>> > Changelog:
>> > v1->v2: Addressed comments from Paolo Abeni, Jiri Pirko
>> > - Use "netif_is_lag_port" relace of "netif_is_bond_slave"
>> > Details in here:
>> > https://lore.kernel.org/netdev/3bf84d23-a561-47ae-84a4-e99488fc762b@bytedance.com/T/
>> >
>> > net/core/dev.c | 10 ++++++----
>> > 1 file changed, 6 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/net/core/dev.c b/net/core/dev.c
>> > index f66e61407883..49144e62172e 100644
>> > --- a/net/core/dev.c
>> > +++ b/net/core/dev.c
>> > @@ -9502,10 +9502,12 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
>> > }
>> >
>> > /* don't allow if an upper device already has a program */
>> > - netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>> > - if (dev_xdp_prog_count(upper) > 0) {
>> > - NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>> > - return -EEXIST;
>> > + if (netif_is_lag_port(dev)) {
>> > + netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>> > + if (dev_xdp_prog_count(upper) > 0) {
>> > + NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>> > + return -EEXIST;
>> > + }
>> > }
>> > }
>> >
>> > --
>> > 2.30.2
>> >
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [External] Re: [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program
2024-08-23 13:29 ` Jiri Pirko
@ 2024-08-27 8:02 ` Feng Zhou
0 siblings, 0 replies; 6+ messages in thread
From: Feng Zhou @ 2024-08-27 8:02 UTC (permalink / raw)
To: Jiri Pirko, Daniel Borkmann
Cc: davem, edumazet, kuba, pabeni, ast, hawk, john.fastabend, bigeasy,
lorenzo, netdev, linux-kernel, bpf, yangzhenze, wangdongdong.6,
Toke Høiland-Jørgensen
在 2024/8/23 21:29, Jiri Pirko 写道:
> Fri, Aug 23, 2024 at 02:07:45PM CEST, daniel@iogearbox.net wrote:
>> On 8/23/24 1:55 PM, Jiri Pirko wrote:
>>> Fri, Aug 23, 2024 at 10:42:04AM CEST, zhoufeng.zf@bytedance.com wrote:
>>>> From: Feng Zhou <zhoufeng.zf@bytedance.com>
>>>>
>>>> Cannot attach when an upper device already has a program, This
>>>> restriction is only for bond's slave devices or team port, and
>>>> should not be accidentally injured for devices like eth0 and vxlan0.
>>>
>>> What if I attach xdp program to solo netdev and then I enslave it
>>> to bond/team netdev that already has xdp program attached?
>>> What prevents me from doing that?
>>
>> In that case the enslaving of the device to bond(/team) must fail as
>> otherwise the latter won't be able to propagate the XDP prog downwards.
>
> Yep, I don't see that in the code though.
>
>
Thanks for your suggestion, I will complete it.
>>
>> Feng, did you double check if we have net or BPF selftest coverage for
>> that? If not might be good to add.
>>
>>>> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>>> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
>>>> ---
>>>> Changelog:
>>>> v1->v2: Addressed comments from Paolo Abeni, Jiri Pirko
>>>> - Use "netif_is_lag_port" relace of "netif_is_bond_slave"
>>>> Details in here:
>>>> https://lore.kernel.org/netdev/3bf84d23-a561-47ae-84a4-e99488fc762b@bytedance.com/T/
>>>>
>>>> net/core/dev.c | 10 ++++++----
>>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>>> index f66e61407883..49144e62172e 100644
>>>> --- a/net/core/dev.c
>>>> +++ b/net/core/dev.c
>>>> @@ -9502,10 +9502,12 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
>>>> }
>>>>
>>>> /* don't allow if an upper device already has a program */
>>>> - netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>>>> - if (dev_xdp_prog_count(upper) > 0) {
>>>> - NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>>>> - return -EEXIST;
>>>> + if (netif_is_lag_port(dev)) {
>>>> + netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>>>> + if (dev_xdp_prog_count(upper) > 0) {
>>>> + NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>>>> + return -EEXIST;
>>>> + }
>>>> }
>>>> }
>>>>
>>>> --
>>>> 2.30.2
>>>>
>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [External] Re: [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program
2024-08-23 12:07 ` Daniel Borkmann
2024-08-23 13:29 ` Jiri Pirko
@ 2024-08-27 8:03 ` Feng Zhou
1 sibling, 0 replies; 6+ messages in thread
From: Feng Zhou @ 2024-08-27 8:03 UTC (permalink / raw)
To: Daniel Borkmann, Jiri Pirko
Cc: davem, edumazet, kuba, pabeni, ast, hawk, john.fastabend, bigeasy,
lorenzo, netdev, linux-kernel, bpf, yangzhenze, wangdongdong.6,
Toke Høiland-Jørgensen
在 2024/8/23 20:07, Daniel Borkmann 写道:
> On 8/23/24 1:55 PM, Jiri Pirko wrote:
>> Fri, Aug 23, 2024 at 10:42:04AM CEST, zhoufeng.zf@bytedance.com wrote:
>>> From: Feng Zhou <zhoufeng.zf@bytedance.com>
>>>
>>> Cannot attach when an upper device already has a program, This
>>> restriction is only for bond's slave devices or team port, and
>>> should not be accidentally injured for devices like eth0 and vxlan0.
>>
>> What if I attach xdp program to solo netdev and then I enslave it
>> to bond/team netdev that already has xdp program attached?
>> What prevents me from doing that?
>
> In that case the enslaving of the device to bond(/team) must fail as
> otherwise the latter won't be able to propagate the XDP prog downwards.
>
> Feng, did you double check if we have net or BPF selftest coverage for
> that? If not might be good to add.
>
Will do, thanks.
>>> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>> Signed-off-by: Feng Zhou <zhoufeng.zf@bytedance.com>
>>> ---
>>> Changelog:
>>> v1->v2: Addressed comments from Paolo Abeni, Jiri Pirko
>>> - Use "netif_is_lag_port" relace of "netif_is_bond_slave"
>>> Details in here:
>>> https://lore.kernel.org/netdev/3bf84d23-a561-47ae-84a4-e99488fc762b@bytedance.com/T/
>>>
>>> net/core/dev.c | 10 ++++++----
>>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index f66e61407883..49144e62172e 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -9502,10 +9502,12 @@ static int dev_xdp_attach(struct net_device
>>> *dev, struct netlink_ext_ack *extack
>>> }
>>>
>>> /* don't allow if an upper device already has a program */
>>> - netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>>> - if (dev_xdp_prog_count(upper) > 0) {
>>> - NL_SET_ERR_MSG(extack, "Cannot attach when an upper
>>> device already has a program");
>>> - return -EEXIST;
>>> + if (netif_is_lag_port(dev)) {
>>> + netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>>> + if (dev_xdp_prog_count(upper) > 0) {
>>> + NL_SET_ERR_MSG(extack, "Cannot attach when an upper
>>> device already has a program");
>>> + return -EEXIST;
>>> + }
>>> }
>>> }
>>>
>>> --
>>> 2.30.2
>>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-27 8:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-23 8:42 [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program Feng zhou
2024-08-23 11:55 ` Jiri Pirko
2024-08-23 12:07 ` Daniel Borkmann
2024-08-23 13:29 ` Jiri Pirko
2024-08-27 8:02 ` [External] " Feng Zhou
2024-08-27 8:03 ` Feng Zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox