From mboxrd@z Thu Jan 1 00:00:00 1970 From: Atzm Watanabe Subject: Re: [PATCH] packet: Deliver VLAN TPID to userspace Date: Tue, 22 Oct 2013 11:56:31 +0900 Message-ID: <87txgaj9nk.wl%atzm@stratosphere.co.jp> References: <87a9i6jymc.wl%atzm@stratosphere.co.jp> <20131018105655.2cdc628e@nehalam.linuxnetplumber.net> <87k3h9hjen.wl%atzm@stratosphere.co.jp> <1382347494.25689.8.camel@deadeye.wl.decadent.org.uk> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Cc: Stephen Hemminger , To: Ben Hutchings Return-path: Received: from mail-pa0-f51.google.com ([209.85.220.51]:46906 "EHLO mail-pa0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752854Ab3JVC4p (ORCPT ); Mon, 21 Oct 2013 22:56:45 -0400 Received: by mail-pa0-f51.google.com with SMTP id ld10so5497442pab.24 for ; Mon, 21 Oct 2013 19:56:45 -0700 (PDT) In-Reply-To: <1382347494.25689.8.camel@deadeye.wl.decadent.org.uk> Sender: netdev-owner@vger.kernel.org List-ID: At Mon, 21 Oct 2013 10:24:54 +0100, Ben Hutchings wrote: > > On Sat, 2013-10-19 at 15:19 +0900, Atzm Watanabe wrote: > > At Fri, 18 Oct 2013 10:56:55 -0700, > > Stephen Hemminger wrote: > > > > > > On Sat, 19 Oct 2013 02:08:11 +0900 > > > Atzm Watanabe wrote: > > > > > > > diff --git a/include/uapi/linux/if_packet.h b/include/uapi/linux/if_packet.h > > > > index dbf0666..6e36e0a 100644 > > > > --- a/include/uapi/linux/if_packet.h > > > > +++ b/include/uapi/linux/if_packet.h > > > > @@ -83,7 +83,7 @@ struct tpacket_auxdata { > > > > __u16 tp_mac; > > > > __u16 tp_net; > > > > __u16 tp_vlan_tci; > > > > - __u16 tp_padding; > > > > + __u16 tp_vlan_tpid; > > > > }; > > > > > > > > /* Rx ring - header status */ > > > > @@ -132,12 +132,13 @@ struct tpacket2_hdr { > > > > __u32 tp_sec; > > > > __u32 tp_nsec; > > > > __u16 tp_vlan_tci; > > > > - __u16 tp_padding; > > > > + __u16 tp_vlan_tpid; > > > > }; > > > > > > > > struct tpacket_hdr_variant1 { > > > > __u32 tp_rxhash; > > > > __u32 tp_vlan_tci; > > > > + __u32 tp_vlan_tpid; > > > > }; > > > > > > The last change will break ABI to userspace applications. > > > You can reuse padding elements; but you can't increase (or shrink) > > > an existing structure. > > > > Thank you for pointing. > > But I have two questions: > > > > - The patch that increases existing structures was posted and > > accepted in the past (e.g 393e52e33c6c26ec7db290dab803bac1bed962d4 > > "packet: deliver VLAN TCI to userspace"). > > What is the difference between them and my patch? > > struct tpacket_auxdata is allowed to grow as it will be written/read > using the cmsg API where the length of each structure is explicit at > run-time. > > > - I tested using tools/testing/selftests/net/psock_tpacket.c built > > before applying my patch, and all test cases were passed. > > Also I tested by the code that was listed in > > Documentation/networking/packet_mmap.txt "AF_PACKET TPACKET_V3 > > example". It seems that problem was not caused. > > What situation causes the problem that you assumed? > > Yes, it looks like it would be safe to grow struct tpacket3_hdr too, as > the length is also explicit at run-time. Thank you for the reply. That's same as my thought! I'll repost the patch with such additional comments. > (And TPACKET{1,2,3}_HDRLEN should be removed from > include/uapi/linux/if_packet.h, as they are not relevant to userland.) Hmm... I think TPACKET{,2,3}_HDRLEN should not be removed without careful considerations. Because some userspace programs (e.g libpcap) are using them in order to check mmap ability of the kernel... Thanks again!