Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed

[Daniel Borkmann <daniel@iogearbox.net>]

On 07/10/2019 01:16 PM, Maxim Mikityanskiy wrote:
> On 2019-06-12 19:14, Maxim Mikityanskiy wrote:
>> dev_change_xdp_fd doesn't perform any checks in case it uninstalls an
>> XDP program. It means that the driver's ndo_bpf can be called with
>> XDP_SETUP_PROG asking to set it to NULL even if it's already NULL. This
>> case happens if the user runs `ip link set eth0 xdp off` when there is
>> no XDP program attached.
>>
>> The drivers typically perform some heavy operations on XDP_SETUP_PROG,
>> so they all have to handle this case internally to return early if it
>> happens. This patch puts this check into the kernel code, so that all
>> drivers will benefit from it.
>>
>> Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
>> ---
>> Björn, please take a look at this, Saeed told me you were doing
>> something related, but I couldn't find it. If this fix is already
>> covered by your work, please tell about that.
>>
>>   net/core/dev.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 66f7508825bd..68b3e3320ceb 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -8089,6 +8089,9 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
>>   			bpf_prog_put(prog);
>>   			return -EINVAL;
>>   		}
>> +	} else {
>> +		if (!__dev_xdp_query(dev, bpf_op, query))
>> +			return 0;
>>   	}
>>   
>>   	err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
>>
> 
> Alexei, so what about this patch? It's marked as "Changed Requested" in 
> patchwork, but Jakub's point looks resolved - I don't see any changes 
> required from my side.

I believe part of Jakub's feedback was that if we make this generic that this
does not generally address the case where both prog pointers are equal (whether
NULL or non-NULL).

Thanks,
Daniel