Re: [PATCH net-next v8 06/16] net: Proxy net_mp_{open,close}_rxq for leased queues

[David Wei <dw@davidwei.uk>]

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.