netdev - Re: [PATCH net-next v8 06/16] net: Proxy net_mp_{open,close}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5f8a0b59-0ca8-43bd-8d28-99d306ff5bbf@davidwei.uk>
Date: Mon, 2 Feb 2026 07:09:25 +0900
From: David Wei <dw@...idwei.uk>
To: Jakub Kicinski <kuba@...nel.org>, Daniel Borkmann <daniel@...earbox.net>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
 razor@...ckwall.org, pabeni@...hat.com, willemb@...gle.com, sdf@...ichev.me,
 john.fastabend@...il.com, martin.lau@...nel.org, jordan@...fe.io,
 maciej.fijalkowski@...el.com, magnus.karlsson@...el.com, toke@...hat.com,
 yangzhenze@...edance.com, wangdongdong.6@...edance.com
Subject: Re: [PATCH net-next v8 06/16] net: Proxy net_mp_{open,close}_rxq for
 leased queues

On 2026-02-01 09:02, Jakub Kicinski wrote:
> On Thu, 29 Jan 2026 23:28:20 +0100 Daniel Borkmann wrote:
>> @@ -191,24 +214,15 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(netdev_rx_queue_restart, "NETDEV_INTERNAL");
>>   
>> -int net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
>> -		    const struct pp_memory_provider_params *p,
>> -		    struct netlink_ext_ack *extack)
>> +static int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
>> +			     const struct pp_memory_provider_params *p,
>> +			     struct netlink_ext_ack *extack)
>>   {
>>   	const struct netdev_queue_mgmt_ops *qops = dev->queue_mgmt_ops;
>>   	struct netdev_queue_config qcfg[2];
>>   	struct netdev_rx_queue *rxq;
>>   	int ret;
>>   
>> -	if (!netdev_need_ops_lock(dev))
>> -		return -EOPNOTSUPP;
>> -
>> -	if (rxq_idx >= dev->real_num_rx_queues) {
>> -		NL_SET_ERR_MSG(extack, "rx queue index out of range");
>> -		return -ERANGE;
>> -	}
>> -	rxq_idx = array_index_nospec(rxq_idx, dev->real_num_rx_queues);
>> -
> 
>> +int net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
>> +		    const struct pp_memory_provider_params *p,
>> +		    struct netlink_ext_ack *extack)
>> +{
>> +	struct net_device *orig_dev = dev;
>> +	int ret;
>> +
>> +	if (!netdev_need_ops_lock(dev))
>> +		return -EOPNOTSUPP;
>> +
>> +	if (rxq_idx >= dev->real_num_rx_queues) {
>> +		NL_SET_ERR_MSG(extack, "rx queue index out of range");
>> +		return -ERANGE;
>> +	}
>> +
> 
> nit: delete this empty line please, like it was in the original code.

Yes.

> 
>> +	rxq_idx = array_index_nospec(rxq_idx, dev->real_num_rx_queues);
>> +
>> +	if (!netif_get_rx_queue_lease_locked(&dev, &rxq_idx)) {
>> +		NL_SET_ERR_MSG(extack, "rx queue leased to a virtual netdev");
>> +		return -EBUSY;
>> +	}
>> +	if (!dev->dev.parent) {
>> +		NL_SET_ERR_MSG(extack, "rx queue belongs to a virtual netdev");
>> +		ret = -EOPNOTSUPP;
>> +		goto out;
>> +	}
>> +
>> +	ret = __net_mp_open_rxq(dev, rxq_idx, p, extack);
>> +out:
>> +	netif_put_rx_queue_lease_locked(orig_dev, dev);
>> +	return ret;
> 
> Please:
> 
> 	if (!netif_queue_is_a_lease())
> 		return __net_mp_open_rxq(dev, rxq_idx, p, extack);
> 
> 	..explicit code that deals with the lease, no conditional locking
> 	...
> 	ret = __net_mp_open_rxq($dev, $rxq_idx, p, extack);
> 	...

Understood.

> 
> 
>> -	rxq = __netif_get_rx_queue(dev, ifq_idx);
>> +	rxq = __netif_get_rx_queue(dev, rxq_idx);
> 
> Indeed good to cleanup the ifq_idx naming that sneaked in, but IDK if
> this belongs in this commit :S

It really annoyed me haha. I'll remove this from this series and send it
as a follow up.