Hi Willem, > That is not what offset_hint 0 does. It also sets the transport header > to the same as the network header.
No, it's not accurate. With offset_hint == 0 I have the transport header set to the offset of the mac header. It's impossible that both offset_hint == 0 and 14 set skb->transport_header to the same value. > This does leave the question whether the transport header would > be better of not being set if it cannot be probed. I have not checked > whether anything in the stack requires it to be set (i.e., not ~0U). Well, at least we can craft a packet without the transport header at all using AF_PACKET, and if some code expects transport_header to be set, it probably wants it to be some reasonable value, not just pointing to some random bytes. It's difficult to track down every usage of skb_transport_header, but if someone expects it to be set when it can be set to a wrong value, they are already having some trouble. To have it set to ~0U looks totally reasonable if offset == 0 is not really a special value, as I thought. However, it would be good to have two special values for the header offset: one would indicate it wasn't set, and the other would indicate that it has been probed, but no transport header exists. It would allow to avoid probing a second time, just because we don't know whether the transport header has been probed or not. Also, there are some places in kernel where skb_probe_transport_header is called with offset_hint == 0, probably meaning that there is no hint, but the offset is set anyway. Maybe it would be a good idea to allow probing without the offset hint, so that if probing fails, transport_header is not set to some buggy value? > Instrumenting the functions to see the offsets, I observe the > following offsets from skb->head: > > packet_snd, sock_raw: data=2, nh=16, th=16 > packet_snd, sock_dgram: data=2, nh=16, th=2 > tpacket_snd, sock_raw: data=2, nh=16, th=2 > > Recall that data has to point to the link layer header at this > point, so the ETH_HLEN difference between data and nh is correct. > > Of these, only the first looks remotely correct to me. IMO, in all of these cases, the transport offset shouldn't be set to the network or mac header. > Yet this > is the (only?) one that you observed causing problems for the > device. Perhaps because offset 0 is treated as a special valid > case in mlx5e_calc_min_inline. Yes, the second and third cases don't cause the bug for the device, exactly for this reason. Still, either some special value indicating "have probed, but no transport header" should be legalized, or ~0U should be used. Thanks, Max
