netdev - Re: [net-next 02/11] ixgbe: Mask off check of frag

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: <1365785511.4459.46.camel@edumazet-glaptop>
Date:	Fri, 12 Apr 2013 09:51:51 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Alexander Duyck <alexander.h.duyck@...el.com>
Cc:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>, davem@...emloft.net,
	netdev@...r.kernel.org, gospo@...hat.com, sassmann@...hat.com
Subject: Re: [net-next 02/11] ixgbe: Mask off check of frag_off as we only
 want fragment offset

On Fri, 2013-04-12 at 09:38 -0700, Alexander Duyck wrote:
> On 04/12/2013 06:45 AM, Eric Dumazet wrote:
> > On Fri, 2013-04-12 at 06:28 -0700, Eric Dumazet wrote:
> >
> >> I wonder if you could use core functions instead of all this...
> >>
> >> A simple wrapper would be :
> > Or more something like :
> >
> > static noinline unsigned int ixgbe_get_headlen(unsigned char *data,
> >                                                 u32 maxlen)
> > {
> >     struct skb fake;
> >     unsigned int res;
> >
> >     if (maxlen < ETH_HLEN)
> > 	return maxlen;
> >
> >     fake->data = data + ETH_HLEN;
> >     fake->head = data;
> >     fake->data_len = 0;
> >     fake->len = maxlen - ETH_HLEN;
> >     skb_reset_network_header(&fake);
> >     res = __skb_get_poff(&fake);
> >     return res ? res + ETH_HLEN : maxlen;
> > }
> 
> The problem is this is way more then I need, and I would prefer not to
> allocate a 192+ byte structure on the stack in order to just parse a
> header that is likely less than 128 bytes.
> 

Thats why I used 'noinline' keyword.

Your code adds significative icache pressure and latencies.


> I could probably do something like create a copy of the
> ixgbe_get_headlen function, maybe named something like
> etherdev_get_headlen and stored in eth.c that could be used by both igb
> and ixgbe.  That way it would be available for anyone else who might
> want to do something similar.  If that would work for you I could
> probably submit that patch sometime in the next few hours.

No please don't do that.

I suggested reusing stuff, not duplicating it.

The main problem is not the cpu cycles spent to parse the header, but
bringing two cache lines for the memcpy() to pull headers.  (TCP uses 66
bytes of headers)

If you use a prefetch(data + 64), chances are good the current generic
code will run before hitting the memory stall


--
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