netdev - Re: [PATCH RESEND] packet: Deliver VLAN TPID to userspace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5277E890.20308@redhat.com>
Date:	Mon, 04 Nov 2013 19:33:52 +0100
From:	Daniel Borkmann <dborkman@...hat.com>
To:	Ben Hutchings <bhutchings@...arflare.com>
CC:	Atzm Watanabe <atzm@...atosphere.co.jp>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	stephen@...workplumber.org
Subject: Re: [PATCH RESEND] packet: Deliver VLAN TPID to userspace

On 11/04/2013 06:16 PM, Ben Hutchings wrote:
> On Wed, 2013-10-30 at 16:03 +0900, Atzm Watanabe wrote:
> [...]
>> 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?

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?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html