netdev - Re: [PATCH] net: ip/tnl: Set iph->id only when don't fragment is not set

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]

Date:   Sat, 23 Nov 2019 10:29:58 -0800
From:   Joe Perches <joe@...ches.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        Oliver Herms <oliver.peter.herms@...il.com>,
        davem@...emloft.net, kuznet@....inr.ac.ru, yoshfuji@...ux-ipv6.org
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] net: ip/tnl: Set iph->id only when don't fragment is
 not set

On Sat, 2019-11-23 at 09:53 -0800, Eric Dumazet wrote:
> 
> On 11/23/19 6:58 AM, Oliver Herms wrote:
> > In IPv4 the identification field ensures that fragments of different datagrams
> > are not mixed by the receiver. Packets with Don't Fragment (DF) flag set are not
> > to be fragmented in transit and thus don't need an identification.
> 
> Official sources for this assertion please, so that we can double check if you
> implemented the proper avoidance ?
> 
> > Calculating the identification takes significant CPU time.
> > This patch will increase IP tunneling performance by ~10% unless DF is not set.
> > However, DF is set by default which is best practice.
> > 
> > Signed-off-by: Oliver Herms <oliver.peter.herms@...il.com>
> > ---
> >  net/ipv4/ip_tunnel_core.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
> > index 1452a97914a0..8636c1e0e7b7 100644
> > --- a/net/ipv4/ip_tunnel_core.c
> > +++ b/net/ipv4/ip_tunnel_core.c
> > @@ -73,7 +73,9 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,
> >  	iph->daddr	=	dst;
> >  	iph->saddr	=	src;
> >  	iph->ttl	=	ttl;
> > -	__ip_select_ident(net, iph, skb_shinfo(skb)->gso_segs ?: 1);
> > +
> > +	if (unlikely((iph->frag_off & htons(IP_DF)) == false))
> 
> This unlikely() seems wrong to me.

as does the comparison to false.