[PATCH] um: Migrate vector drivers to NAPI

Anton Ivanov anton.ivanov at cambridgegreys.com
Fri Jan 21 02:08:14 PST 2022



On 21/01/2022 09:56, Anton Ivanov wrote:
> 
> 
> On 21/01/2022 09:29, Johannes Berg wrote:
>> On Fri, 2022-01-21 at 08:55 +0000, anton.ivanov at cambridgegreys.com
>> wrote:
>>> From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
>>>
>>> Migrate UML vector drivers from a bespoke scheduling mechanism
>>> to NAPI.
>>
>> Nice :)
>>
>>> -static void vector_rx(struct vector_private *vp)
>>> -{
>>> -    int err;
>>> -    int iter = 0;
>>> -
>>> -    if ((vp->options & VECTOR_RX) > 0)
>>> -        while (((err = vector_mmsg_rx(vp)) > 0) && (iter < MAX_ITERATIONS))
>>
>> You can remove the MAX_ITERATIONS define now.
> 
> Thanks, missed that.
> 
>>
>>
>>> +static int vector_poll(struct napi_struct *napi, int budget)
>>>   {
>>> -    struct vector_private *vp = from_tasklet(vp, t, tx_poll);
>>> +    struct vector_private *vp = container_of(napi, struct vector_private, napi);
>>> +    int work_done = 0;
>>> +    int err;
>>> +    bool tx_enqueued = false;
>>> -    vp->estats.tx_kicks++;
>>> -    vector_send(vp->tx_queue);
>>> +    if ((vp->options & VECTOR_TX) != 0)
>>> +        tx_enqueued = (vector_send(vp->tx_queue) > 0);
>>> +    if ((vp->options & VECTOR_RX) > 0)
>>> +        err = vector_mmsg_rx(vp);
>>> +    else {
>>> +        err = vector_legacy_rx(vp);
>>
>> It feels like you should honour the NAPI and pass the budget down, using
>> it to limit the number of packets processed for TX/RX, something like
>>
>> tx_enqueued = vector_send(..., budget);
>> budget -= tx_enqueued;
> 
> Good idea :)
> 
>>
>> if (budget > 0)
>>     err = vector_mmsg_rx(vp, budget);
>>
>> etc.
>>
>>> @@ -1264,6 +1251,10 @@ static int vector_net_open(struct net_device *dev)
>>>           if (vp->header_txbuffer == NULL)
>>>               goto out_close;
>>>       }
>>> +    /* NAPI */
>>> +
>>
>> That comment feels a bit pointless, but whatever :)
> 
> Thanks, I used that to make my life easier editing it. Forgot to remove it.
> 
>>
>>> +    netif_napi_add(vp->dev, &vp->napi, vector_poll, 64);
>>> +    napi_enable(&vp->napi);
>>
>> I think this should only be done once when you create the netdev?
> 
> I need to have a look around the kernel and think on this one. Historically, UML does a few things in open() which are closer to device instantiation for real hardware.

There are quite a few  drivers which do it in open and disable it in close.

Example: Atheros AR71xx

So this looks OK in terms of where it is and how vector_kern is using it.

> 
>>
>>
>>> @@ -1312,9 +1303,11 @@ static int vector_net_open(struct net_device *dev)
>>>        * SIGIOs never arrive, and the net never works.
>>>        */
>>> -    vector_rx(vp);
>>>       vector_reset_stats(vp);
>>> +
>>> +    napi_schedule(&vp->napi);
>>>
>>
>> The comment should probably move with this?
> 
> Yes. Well spotted.
> 
>>
>> johannes
>>
> 

-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/



More information about the linux-um mailing list