netdev - Re: [PATCH RFC net-next 2/7] net: add dst_pending

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f9dadb5e-172a-4c39-c564-38be672ef0eb@stressinduktion.org>
Date:   Mon, 19 Dec 2016 17:36:46 +0100
From:   Hannes Frederic Sowa <hannes@...essinduktion.org>
To:     Eric Dumazet <eric.dumazet@...il.com>, Julian Anastasov <ja@....bg>
Cc:     netdev@...r.kernel.org, linux-sctp@...r.kernel.org,
        YueHaibing <yuehaibing@...wei.com>
Subject: Re: [PATCH RFC net-next 2/7] net: add dst_pending_confirm flag to
 skbuff

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