Re: [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program

[Daniel Borkmann <daniel@iogearbox.net>]

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
>>
>