On 19.12.2016 17:17, Eric Dumazet wrote:
> On Sun, 2016-12-18 at 22:56 +0200, Julian Anastasov wrote:
>
>>
>> +static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour
>> *n)
>> +{
>> + if (unlikely(skb->dst_pending_confirm)) {
>> + struct sock *sk = skb->sk;
>> + unsigned long now = jiffies;
>> +
>> + /* avoid dirtying neighbour */
>> + if (n->confirmed != now)
>> + n->confirmed = now;
>> + if (sk && sk->sk_dst_pending_confirm)
>> + sk->sk_dst_pending_confirm = 0;
>> + }
>> +}
>> +
>
> I am still digesting this awesome patch series ;)
>
> Not sure why you used an unlikely() here. TCP for example would hit this
> path quite often.
>
> So considering sk_dst_pending_confirm might be dirtied quite often,
>
> I am not sure why you placed it in the cache line that contains
> sk_rx_dst (in 1st patch)
Because they have to stay synchronized?
If we modify sk_rx_dst, we automatically also must clear
pending_confirm, otherwise we might end up confirming a wrong neighbor.
Bye,
Hannes
