netdev - Re: "TCP: eth0: Driver has suspect GRO implementation, TCP performance may be compromised." message with "ethtool -K eth0 gro off"

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: <1486131366.21871.87.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Fri, 03 Feb 2017 06:16:06 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc:     Markus Trippelsdorf <markus@...ppelsdorf.de>,
        netdev@...r.kernel.org
Subject: Re: "TCP: eth0: Driver has suspect GRO implementation, TCP
 performance may be compromised." message with "ethtool -K eth0 gro off"

On Fri, 2017-02-03 at 11:53 -0200, Marcelo Ricardo Leitner wrote:
> On Fri, Feb 03, 2017 at 05:24:06AM -0800, Eric Dumazet wrote:
> > On Fri, 2017-02-03 at 09:54 -0200, Marcelo Ricardo Leitner wrote:
> > > On Thu, Feb 02, 2017 at 05:59:24AM -0800, Eric Dumazet wrote:
> > > > On Thu, 2017-02-02 at 05:31 -0800, Eric Dumazet wrote:
> > > > 
> > > > > Anyway, I suspect the test is simply buggy ;)
> > > > > 
> > > > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > > > > index 41dcbd568cbe2403f2a9e659669afe462a42e228..5394a39fcce964a7fe7075b1531a8a1e05550a54 100644
> > > > > --- a/net/ipv4/tcp_input.c
> > > > > +++ b/net/ipv4/tcp_input.c
> > > > > @@ -164,7 +164,7 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb)
> > > > >  	if (len >= icsk->icsk_ack.rcv_mss) {
> > > > >  		icsk->icsk_ack.rcv_mss = min_t(unsigned int, len,
> > > > >  					       tcp_sk(sk)->advmss);
> > > > > -		if (unlikely(icsk->icsk_ack.rcv_mss != len))
> > > > > +		if (unlikely(icsk->icsk_ack.rcv_mss != len && skb_is_gso(skb)))
> > > > >  			tcp_gro_dev_warn(sk, skb);
> > > > >  	} else {
> > > > >  		/* Otherwise, we make more careful check taking into account,
> > > > 
> > > > This wont really help.
> > > > 
> > > > Our tcp_sk(sk)->advmss can be lower than the MSS used by the remote
> > > > peer.
> > > > 
> > > > ip ro add .... advmss 512
> > > 
> > > I don't follow. With a good driver, how can advmss be smaller than the
> > > MSS used by the remote peer? Even with the route entry above, I get
> > > segments just up to advmss, and no warning.
> > > 
> > 
> > A TCP flow has two ends.
> 
> Indeed, though should be mostly about only one of them.
> 
> > 
> > Common MTU = 1500
> > 
> > One can have advmss 500, the other one no advmss (or the standard 1460
> > one)
> 
> Considering the rx side of peer A. Peer A advertises a given MSS to peer
> B and should not receive any segment from peer B larger than so.
> I'm failing to see how advmss can be smaller than the segment size just
> received.

tcp_sk(sk)->advmss records what the peer announced during its SYN (or
SYNACK) message, in the MSS option.

Nothing prevents the peer to change its mind later.

Eg starting with MSS 512, then switch later to sending packets of 1024
or 1400 bytes.

So the innocent NIC driver is not the problem here.