Thread (27 messages) 27 messages, 4 authors, 2016-09-28

Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets

flat view

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2016-09-28 10:08:14
Also in: lkml

On 16-09-28 05:03 AM, Cyrill Gorcunov wrote:
quoted hunk ↗ jump to hunk
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 <davem@davemloft.net>
CC: Eric Dumazet <redacted>
CC: David Ahern <redacted>
CC: Alexey Kuznetsov <redacted>
CC: James Morris <jmorris@namei.org>
CC: Hideaki YOSHIFUJI <redacted>
CC: Patrick McHardy <redacted>
CC: Andrey Vagin <redacted>
CC: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Cyrill Gorcunov <redacted>
---

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 <net/protocol.h>

+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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help