On (10/14/16 02:06), Duyck, Alexander H wrote: > > + case ETH_P_IP: > > + skb_header_pointer(skb, ETH_HLEN, sizeof (struct iphdr), > > + &ip_hdr); > > /* access ihl as u8 to avoid unaligned access on ia64 */ > > - hlen = (hdr.network[0] & 0x0F) << 2; > > - l4_proto = hdr.ipv4->protocol; > > + hlen = ip_hdr.ipv4.ihl << 2; > > + l4_proto = ip_hdr.ipv4.protocol; > > break; : > The problem is this will break other stuff, for example I have seen > the ihl access actually cause problems with unaligned accesses as some > architectures decide to pull it as a u32 and then mask it.
Yes, I noticed that u8 comment for ia64.. if that's the only issue here, we could just reset hdr.network to &ip_hdr.. However, I suspect the above patch is probably not going to work for the vlan case (it was just a first-pass hack) > My advice would be to keep this simple. Add a check to make sure we > have room for at least skb_headlen(skb) - 40 >= hrd.raw - skb->data. I don't parse that- the hdr union in ixgbe_atr doesnt have a ->raw field. Can you explain? > Messing with the protocol bits will break stuff since there is support > for tunneling also floating around in here now. > > I believe we are planning on dropping this code in favor of > ndo_rx_flow_steer in the future. If we do that then the whole problem > becomes moot. Dropping it is fine with me I guess - maybe just return, if the skb_headlen() doesnt have enough bytes for a network header, i.e., skb_headlen is at least ETH_HLEN + sizeof (struct iphdr) for ETH_P_IP, or ETH_HLEN + sizeof (struct ipv6hdr) for ETH_P_IPV6? --Sowmini
