At Mon, 28 Oct 2013 20:11:14 +0000,
Ben Hutchings wrote:

On Fri, 2013-10-25 at 18:59 -0400, David Miller wrote:

quoted

From: Atzm Watanabe <redacted>
Date: Tue, 22 Oct 2013 17:39:40 +0900

quoted

 struct tpacket_hdr_variant1 {
 	__u32	tp_rxhash;
 	__u32	tp_vlan_tci;
+	__u32	tp_vlan_tpid;
 };
 

You cannot do this, the header length is not variable.

Well it is variable to an extent.  This is used as a member of the final
anonymous union member of struct tpacket3_hdr, and the latter is
specified to be tail-padded to a multiple of 16 bytes
(TPACKET_ALIGNMENT).  Since its current size is 36 bytes, I think it can
safely grow by 12 bytes, so long as userland doesn't depend on
getsockopt(..., PACKET_HDRLEN, ...) returning only specific values.

Possibly there should be a separate struct tpacket_hdr_variant2 which
includes the extra member.  Possibly there should also be a status flag
to indicate that this member is present.

Should it be structures as below?

struct tpacket_hdr_variant1 {
        __u32   tp_rxhash;
        __u32   tp_vlan_tci;
};

struct tpacket_hdr_variant2 {
        __u32   tp_rxhash;
        __u32   tp_vlan_tci;
        __u32   tp_vlan_tpid;
};

struct tpacket3_hdr {
        __u32           tp_next_offset;
        __u32           tp_sec;
        __u32           tp_nsec;
        __u32           tp_snaplen;
        __u32           tp_len;
        __u32           tp_status;
        __u16           tp_mac;
        __u16           tp_net;
        /* pkt_hdr variants */
        union {
                struct tpacket_hdr_variant1 hv1;
                struct tpacket_hdr_variant2 hv2;
        };
};

If it's ok, I'd like to send the patch v2.

quoted

This patch has been submitted several times, each of which you
have been shown ways in which your changes break userspace in
one way or another.

I think we established that struct tpacket3_hdr can't grow arbitrarily,
but not that it can't grow at all.

I think so, too.