netdev - Re: [PATCH v4 06/10] e1000e: Support for byte queue limits

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: <1322734334.2335.20.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Date:	Thu, 01 Dec 2011 11:12:14 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Tom Herbert <therbert@...gle.com>
Cc:	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v4 06/10] e1000e: Support for byte queue limits

Le mercredi 30 novembre 2011 à 17:04 -0800, Tom Herbert a écrit :
> > whats wrong with using total_tx_bytes or buffer_info->bytecount?  it
> > contains the "bytes on the wire" value which will be slightly larger
> > than skb->len, but avoids warming the skb->len cacheline unnecessarily.
> >
> 
> This makes sense.  I made the changes, but the limits computed are
> much higher than with using sbk->len.  I suspect there may be a bug in
> GSO path.
> 
> For instance, in the driver at:
> 
>   bytecount = ((segs - 1) * skb_headlen(skb)) + skb->len;
> 
> I see cases like:
> 
>   segs=34, skb_header_len(skb)=70, skb->len=49298 so bytecount=51608
> 
> Which seems reasonable... but, I also see things like:
> 
>   segs=45, skb_header_len(skb)=1522, skb->len=65226, so bytecount= 132194
>                                                       ^^^^
> Which doesn't seem right at all.  I am thinking there may be a bug
> setting skb->data_len improperly.
> 

OK, as stated on your other thread, its obvious this driver (and
probably other intel drivers) made assumptions that are now obsolete,
since skb head can contain some data payload, not only (MAC+IP+TCP)
headers.

If Intel guys cannot afford approximate the bytecount by skb->len, I
suggest to use same trick found in 
drivers/net/ethernet/intel/igb/igb_main.c

static int igb_tso(struct igb_ring *tx_ring, 
...
        /* compute header lengths */
        l4len = tcp_hdrlen(skb);
        *hdr_len = skb_transport_offset(skb) + l4len;

        /* update gso size and bytecount with header size */
        first->gso_segs = skb_shinfo(skb)->gso_segs;
        first->bytecount += (first->gso_segs - 1) * *hdr_len;




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html