From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753126AbcI1KIO (ORCPT ); Wed, 28 Sep 2016 06:08:14 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:34016 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752186AbcI1KII (ORCPT ); Wed, 28 Sep 2016 06:08:08 -0400 Subject: Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets To: Cyrill Gorcunov , Eric Dumazet , David Ahern References: <20160928090357.GT1876@uranus.lan> Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, David Miller , kuznet@ms2.inr.ac.ru, jmorris@namei.org, yoshfuji@linux-ipv6.org, kaber@trash.net, avagin@openvz.org, stephen@networkplumber.org From: Jamal Hadi Salim Message-ID: Date: Wed, 28 Sep 2016 06:08:00 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20160928090357.GT1876@uranus.lan> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 16-09-28 05:03 AM, Cyrill Gorcunov wrote: > In criu we are actively using diag interface to collect sockets > present in the system when dumping applications. And while for > unix, tcp, udp[lite], packet, netlink it works as expected, > the raw sockets do not have. Thus add it. > > v2: > - add missing sock_put calls in raw_diag_dump_one (by eric.dumazet@) > - implement @destroy for diag requests (by dsa@) > > v3: > - add export of raw_abort for IPv6 (by dsa@) > - pass net-admin flag into inet_sk_diag_fill due to > changes in net-next branch (by dsa@) > > v4: > - use @pad in struct inet_diag_req_v2 for raw socket > protocol specification: raw module carries sockets > which may have custom protocol passed from socket() > syscall and sole @sdiag_protocol is not enough to > match underlied ones > - start reporting protocol specifed in socket() call > when sockets are raw ones for the same reason: user > space tools like ss may parse this attribute and use > it for socket matching > > v5 (by eric.dumazet@): > - use sock_hold in raw_sock_get instead of atomic_inc, > we're holding (raw_v4_hashinfo|raw_v6_hashinfo)->lock > when looking up so counter won't be zero here. > > CC: David S. Miller > CC: Eric Dumazet > CC: David Ahern > CC: Alexey Kuznetsov > CC: James Morris > CC: Hideaki YOSHIFUJI > CC: Patrick McHardy > CC: Andrey Vagin > CC: Stephen Hemminger > Signed-off-by: Cyrill Gorcunov > --- > > Thanks all for feedback! Take a look please once time permit. > > include/net/raw.h | 6 + > include/net/rawv6.h | 7 + > include/uapi/linux/inet_diag.h | 5 > net/ipv4/Kconfig | 8 + > net/ipv4/Makefile | 1 > net/ipv4/inet_diag.c | 9 + > net/ipv4/raw.c | 21 +++ > net/ipv4/raw_diag.c | 233 +++++++++++++++++++++++++++++++++++++++++ > net/ipv6/raw.c | 7 - > 9 files changed, 292 insertions(+), 5 deletions(-) > > Index: linux-ml.git/include/net/raw.h > =================================================================== > --- linux-ml.git.orig/include/net/raw.h > +++ linux-ml.git/include/net/raw.h > @@ -23,6 +23,12 @@ > > extern struct proto raw_prot; > > +extern struct raw_hashinfo raw_v4_hashinfo; > +struct sock *__raw_v4_lookup(struct net *net, struct sock *sk, > + unsigned short num, __be32 raddr, > + __be32 laddr, int dif); > + > +int raw_abort(struct sock *sk, int err); > void raw_icmp_error(struct sk_buff *, int, u32); > int raw_local_deliver(struct sk_buff *, int); > > Index: linux-ml.git/include/net/rawv6.h > =================================================================== > --- linux-ml.git.orig/include/net/rawv6.h > +++ linux-ml.git/include/net/rawv6.h > @@ -3,6 +3,13 @@ > > #include > > +extern struct raw_hashinfo raw_v6_hashinfo; > +struct sock *__raw_v6_lookup(struct net *net, struct sock *sk, > + unsigned short num, const struct in6_addr *loc_addr, > + const struct in6_addr *rmt_addr, int dif); > + > +int raw_abort(struct sock *sk, int err); > + > void raw6_icmp_error(struct sk_buff *, int nexthdr, > u8 type, u8 code, int inner_offset, __be32); > bool raw6_local_deliver(struct sk_buff *, int); > Index: linux-ml.git/include/uapi/linux/inet_diag.h > =================================================================== > --- linux-ml.git.orig/include/uapi/linux/inet_diag.h > +++ linux-ml.git/include/uapi/linux/inet_diag.h > @@ -38,7 +38,10 @@ struct inet_diag_req_v2 { > __u8 sdiag_family; > __u8 sdiag_protocol; > __u8 idiag_ext; > - __u8 pad; > + union { > + __u8 pad; > + __u8 sdiag_raw_protocol; /* SOCK_RAW only, @pad for others */ > + }; Above looks funny. Why is it a union? pad is for exposing a byte-hole for padding/alignment reasons and i doubt anybody is using it. Should you not just rename it? Also I notice when things like __raw_v4_lookup() are claiming it is unsigned short instead of a u8? cheers, jamal