At Mon, 04 Nov 2013 19:33:52 +0100,
Daniel Borkmann wrote:

On 11/04/2013 06:16 PM, Ben Hutchings wrote:

quoted

On Wed, 2013-10-30 at 16:03 +0900, Atzm Watanabe wrote:
[...]

quoted

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.

[...]

I think this makes sense.

Also add a BUILD_BUG_ON(TPACKET3_HDRLEN != 48) somewhere.

Sorry for coming late into this discussion.

I think the packet_mmap API with its 3 different API versions
already is a "bit" terrible, and should only be further extended
for user space if there is a _*huge*_ (!) improvement somewhere
(e.g. in terms of packet capturing/transmission performance) that
would justify it. I'm thinking that this could e.g. be in terms
of packet capturing and transmission performance.

Otherwise, each time we want to add a new member, we add yet
another subheader for userspace into tpacket, making it even
more complicated to use, and adding more "legacy" API fragments?

Thank you for the comments.

Indeed, your worry will become reality if we often want to add new
members as other aims, but I also think that VLAN related members
perhaps won't be added anymore because the VLAN only contains TCI
and ethertype.

To solve your problem, why don't you add a socket option that,
if _enabled_, would reconstruct the vlan header as it was seen
on the "wire" and push that up to userspace, just like libpcap
does internally. It's not perfect either, but perhaps a better
way to go. What do you think?

Sounds good to me, but I also think that a socket option should be
added when we want to add new members to enable userspace to
reassemble the pakcet for reasons other than the VLAN.  Because TCI
is already put into the tpacket headers or auxdata, userspace may
want to get TPID by same way.

However, if we cannot add new members into the header anyway,
we need to avoid that by other ways like you proposed.


Thanks!