From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sowmini Varadhan Date: Mon, 17 Oct 2016 15:18:04 -0400 Subject: [Intel-wired-lan] [PATCH V2 RFC 2/2] ixgbe: ixgbe_atr() compute l4_proto only if non-paged data has network/transport headers In-Reply-To: References: Message-ID: <20161017191804.GE15976@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: On (10/17/16 11:15), Alexander Duyck wrote: > I would say you probably only need the first check here for skb->data > and could probably skip the second part. You will be testing for > skb_tail_pointer in all the other tests you added so this check is > redundant anyway. > > Also you might want to go through and wrap these with unlikely() since > most of these are exception cases. Ok.. v3 will have this. > > /* Currently only IPv4/IPv6 with TCP is supported */ > > switch (hdr.ipv4->version) { > > case IPVERSION: > > /* access ihl as u8 to avoid unaligned access on ia64 */ > > hlen = (hdr.network[0] & 0x0F) << 2; > > + if (skb_tail_pointer(skb) < hdr.network + hlen + > > + sizeof(struct tcphdr)) > > + return; > > l4_proto = hdr.ipv4->protocol; > > break; > > case 6: > > hlen = hdr.network - skb->data; > > + if (skb_tail_pointer(skb) < hdr.network + hlen + > > + sizeof(struct tcphdr)) > > + return; > > l4_proto = ipv6_find_hdr(skb, &hlen, IPPROTO_TCP, NULL, NULL); > > hlen -= hdr.network - skb->data; > > break; > > I believe one more check is needed after this block to verify the TCP > header fields are present. > > So you probably need to add a check for "skb_tail_pointer(skb) < > (hdr.network + hlen + 20)". But isnt that the same thing as the checks before l4_proto computation above? --Sowmini