Re: [PATCH] net/packet: fix network header offset-VLAN raw packets on VLAN subinterfaces

[Junnan Zhang]

Hi Willem,
Thanks for the follow-up.
> Why is the real length 14 + 14 == 28?
> Where does the second 14 come from?
You're right to flag this - my previous mail used "base" inconsistently,
which is what made the "14 + 14" look unexplained. Let me redo it with a
single reference point.
The cleanest reference is skb->data after packet_snd() has set things up
(i.e. the start of the user-supplied raw frame). With hard_header_len=18
and min_header_len=14 on a software-offload VLAN subif:
  - skb_reset_network_header() runs while data is at head + hlen
    (hlen = LL_RESERVED_SPACE_EX(dev, 18) = 32, the HH_DATA_MOD-rounded
     headroom), so network_header lands at head + hlen = head + 32.
  - The SOCK_RAW branch then does skb_reserve(skb, -reserve) with
    reserve = hard_header_len = 18, moving data back to head + 14
    (= head + hlen - hard_header_len).
  - The small-frame skb_reset_network_header() at packet_snd:3078 does
    not fire for a GSO frame, so network_header stays at head + 32.
Relative to data (= head + 14), this means:
    network_header = data + (32 - 14) = data + 18 = data + hard_header_len
    real IP header  = data + ETH_HLEN  = data + 14 = data + min_header_len
So network_header points VLAN_HLEN (4) bytes past the real IP header.
The "14 + 14 = 28" in my earlier reply was the absolute offset of IP
from head (data-offset 14 from rounding + ETH_HLEN 14); the second 14 is
ETH_HLEN, i.e. the user-supplied Ethernet header. I should not have mixed
the head-relative IP position with a data-relative network_header - sorry
for the confusion. v2 will use data-relative offsets throughout, which
also matches skb_network_offset() and is independent of the
LL_RESERVED_SPACE rounding.
With the fix, skb_set_network_header(skb, dev->min_header_len) sets
network_header = data + min_header_len = data + ETH_HLEN, i.e. exactly on
the real IP header, so both skb_probe_transport_header() (nhoff) and GSO
see the correct L3.

> This is the hint that this is a vlan device with software VLAN tag
> insertion? Technically, it might apply to other variable length
> header devices too.
>
You're right, it is not specific to VLAN. min_header_len <
hard_header_len also matches Ethernet drivers that reserve extra space
in hard_header_len beyond ETH_HLEN for their own wrapping. For all of these, 
the user-supplied non-VLAN SOCK_RAW frame still carries a standard 14-byte 
Ethernet header, so its L3 header sits at ETH_HLEN = min_header_len, and 
pointing network_header there is correct.
The condition is intentionally generic, not VLAN-specific.
For any ARPHRD_ETHER device whose hard_header_len exceeds min_header_len,
a non-VLAN SOCK_RAW frame's L3 sits at min_header_len (the standard
Ethernet header length), regardless of what extra bytes hard_header_len
reserves for driver-internal wrapping. The fix points network_header at
exactly that L2/L3 boundary. This matches the existing
dev->min_header_len != hard_header_len check already used in packet_snd()
(the small-frame skb_reset_network_header path).
I'll reword the v2 commit message to describe the scope as "Ethernet
devices whose hard_header_len exceeds min_header_len" rather than "VLAN
subinterfaces", and drop the misleading VLAN-only framing.
I'll send v2 as a new thread.

Thanks,
Junnan