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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 28 Jul 2014 16:08:46 -0700 From: Alexander Duyck <alexander.h.duyck@...el.com> To: David Miller <davem@...emloft.net>, cwang@...pensource.com CC: amirv@...lanox.com, edumazet@...gle.com, netdev@...r.kernel.org, ogerlitz@...lanox.com, yevgenyp@...lanox.com, idos@...lanox.com, eric.dumazet@...il.com Subject: Re: [PATCH net-next V1 1/2] net: Header length compution function On 07/28/2014 03:42 PM, David Miller wrote: > From: Cong Wang <cwang@...pensource.com> > Date: Mon, 28 Jul 2014 14:26:08 -0700 > >> On Mon, Jul 28, 2014 at 7:50 AM, Alexander Duyck >> <alexander.h.duyck@...el.com> wrote: >>> On 07/28/2014 04:28 AM, Amir Vadai wrote: >>>> +u32 eth_frame_headlen(void *data, unsigned int len) >>>> +{ >>>> + const struct ethhdr *eth = data; >>>> + struct sk_buff skb; >>>> + >>>> + if (unlikely(len < ETH_HLEN)) >>>> + return len; >>>> + >>>> + skb.protocol = eth->h_proto; >>>> + skb.head = data + ETH_HLEN; >>>> + skb.data = skb.head; >>>> + skb_reset_network_header(&skb); >>>> + skb.len = len - ETH_HLEN; >>>> + skb.data_len = 0; >>>> + return __skb_get_poff(&skb) + ETH_HLEN; >>>> +} >>> >>> I'm still not a big fan of allocating an sk_buff on the stack. Seems >>> like it isn't maintainable and really opens things up to possible issues >>> if someone ever extends the __skb_get_poff call. But I'm not going to >>> force the issue since for now this isn't impacting igb or ixgbe. >>> >> >> +1 >> >> I think you can refactor the code to pass all these input as >> arguments instead of a whole skbuff. > > I was going to say the same thing, but if you take a look it's not so > simple. > > The code currently handles fragmented SKBs just fine, and you'd > therefore have to make a seperate code path for purely linear buffers, > and thus code duplication. > > I'm still not sure what's better, to be honest. Currently I'm leaning > towards allowing the version in this patch set, even though it's a bit > risky this is in the fast path so perhaps warrants such tricks for > performance's sake. > I think if anything it might be nice to add some warnings to __skb_get_poff, and skb_flow_dissect pointing back to this code in order to warn someone thinking of optimizing it so that they are aware that there is a caller that provides partially initialized and incomplete SKBs. Thanks, Alex -- 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
Powered by blists - more mailing lists