On Wed, Apr 22, 2015 at 2:42 PM, Alexander Duyck <alexander.du...@gmail.com> wrote: > On 04/22/2015 01:33 PM, Cong Wang wrote: >> First, make sure you don't miss the TSIP case right above: >> >> The frag starting pointer and its size are advanced by: >> >> skb_frag_size_sub(frag, IGB_TS_HDR_LEN); >> ... >> va += IGB_TS_HDR_LEN; >> >> even though we unlikely pull header longer than >> IGB_RX_HDR_LEN - IGB_TS_HDR_LEN either. > > So I believe this is a possible bug, one heck of a corner case to get > into though. It requires timestamp in packet, size 240 - 256, and a > malformed header. > > The proper fix would probably be to pull the timestamp out of the packet > before we add it to the frame. I'll submit a patch to address this. >
Huh? Doesn't my patch already fix this? skb_frag_size() is always up to date. Or you mean another different problem? >> >> Second, the check you mentioned above is: >> >> if ((size <= IGB_RX_HDR_LEN) && !skb_is_nonlinear(skb)) >> >> skb is nonlinear _after_ the first igb_add_rx_frag(), a second >> igb_add_rx_frag() is possible since igb_is_non_eop() could >> return true. > > I'm not sure this part makes any sense. We pull the data out of the > first fragment always. If skb_is_nonlinear is set then we should have > at least 2K - 16B in the case of igb. We will never have a second > fragment without at least 2K of data being given in the first. Apparently my igb knowledge isn't enough to verify this, I just did logical analysis. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
