ixgbe & if_igb RX ring locking

[Jack Vogel]

On Thu, Oct 18, 2012 at 6:20 AM, Andre Oppermann <oppermann at networx.ch>wrote:

> On 13.10.2012 20:22, Luigi Rizzo wrote:
>
>> On Sat, Oct 13, 2012 at 09:49:21PM +0400, Alexander V. Chernikov wrote:
>>
>>> Hello list!
>>>
>>>
>>> Packets receiving code for both ixgbe and if_igb looks like the
>>> following:
>>>
>>>
>>> ixgbe_msix_que
>>>
>>> -- ixgbe_rxeof()
>>>     {
>>>        IXGBE_RX_LOCK(rxr);
>>>          while
>>>          {
>>>             get_packet;
>>>
>>>             -- ixgbe_rx_input()
>>>                {
>>>                   ++ IXGBE_RX_UNLOCK(rxr);
>>>                   if_input(packet);
>>>                   ++ IXGBE_RX_LOCK(rxr);
>>>                }
>>>
>>>          }
>>>        IXGBE_RX_UNLOCK(rxr);
>>>      }
>>>
>>> Lines marked with ++ appeared in r209068(igb) and r217593(ixgbe).
>>>
>>> These lines probably do LORs masking (if any) well.
>>> However, such change introduce quite significant performance drop:
>>>
>>> On my routing setup (nearly the same from previous -Intel 10G thread in
>>> -net) adding lock/unlock causes 2.8MPPS decrease to 2.3MPPS which is
>>> nearly 20%.
>>>
>>
>> one option could be (same as it is done in the timer
>> routine in dummynet) to build a list of all the packets
>> that need to be sent to if_input(), and then call
>> if_input with the entire list outside the lock.
>>
>> It would be even easier if we modify the various *_input()
>> routines to handle a list of mbufs instead of just one.
>>
>
> Not really. You'd just run into tons of layering complexity.
> Somewhere the decomposition and serialization has to be done.
>
> Perhaps the right place is to dequeue a batch of packets from
> the HW ring and then have a task/thread send it up the stack
> one by one.
>

I was thinking about how to code this, something like what I did with
the refresh routine, in any case I will experiment with it.

Jack