[patch] reducing arp locking

[Fabien Thomas]

Le 9 nov. 2012 à 10:05, Alexander V. Chernikov a écrit :

> On 09.11.2012 12:51, Fabien Thomas wrote:
>> 
>> Le 8 nov. 2012 à 11:25, Alexander V. Chernikov a écrit :
>> 
>>> On 08.11.2012 14:24, Andre Oppermann wrote:
>>>> On 08.11.2012 00:24, Alexander V. Chernikov wrote:
>>>>> Hello list!
>>>>> 
>>>>> Currently we need to acquire 2 read locks to perform simple 6-byte
>>>>> copying from arp record to packet
>>>>> ethernet header.
>>>>> 
>>>>> It seems that acquiring lle lock for fast path (main traffic flow) is
>>>>> not necessary even with
>>>>> current code.
>>>>> 
>>>>> My tests shows ~10% improvement with this patch applied.
>>>>> 
>>>>> If nobody objects I plan to commit this change at the end of next week.
>>>> 
>>>> This is risky and prone to race conditions.  The copy of the MAC address
>>>> should be done while the table read lock is held to protect against the
>>> It is done exactly as you say: table read lock is held.
>> 
>> How do you protect from entry update if i've a ref to the entry ?
>> You can end up doing bcopy of a partial mac address.
> I see no problems in copying incorrect mac address in that case:
> if host mac address id updated, this is, most likely, another host, and several packets being lost changes nothing.

Sending packet to a bogus mac address is not really nothing :)

> 
> However, there can be some realistic scenario where this can be the case (L2 load balancing/failover). I'll update in_arpinput() to do lle removal/insertion in that case.
> 
>> la_preempt modification is also write access to an unlocked structure.
> This one changes nothing:
> current code does this under _read_ lock.

Under the table lock not the entry lock ?
Table lock is here to protect the table if I've understood the code correctly.
If i get an exclusive reference to the entry you will end up reading and writing to the entry without any lock.

> 
>> 
>> 
>>> 
>>>> entry going away.  You can either return with table lock held and drop
>>>> it after the copy, or you could a modified lookup function that takes a
>>>> pointer for the copy destination, do the copy with the read lock, and then
>>>> return.  If no entry is found an error is returned and obviously no copy
>>>> is done.
>>>> 
>>> 
>>> 
>>> --
>>> WBR, Alexander
>>> 
>>> 
>>> _______________________________________________
>>> freebsd-hackers at freebsd.org mailing list
>>> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>>> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"
>> 
>> 
>