netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [iproute PATCH v2 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs
@ 2015-12-17 13:22 Lorenzo Colitti
  2015-12-17 13:22 ` [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
  2015-12-17 16:07 ` [iproute PATCH v2 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Eric Dumazet
  0 siblings, 2 replies; 16+ messages in thread
From: Lorenzo Colitti @ 2015-12-17 13:22 UTC (permalink / raw)
  To: netdev; +Cc: stephen, eric.dumazet, zenczykowski, Lorenzo Colitti

The new variant is identical to rtnl_send_check, except it also
consumes the kernel response instead of using MSG_PEEK. This is
useful for callers that send simple commands that never cause a
response but only ACKs, and that expect to receive and deal
with errors without printing them to stderr like rtnl_talk does.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 include/libnetlink.h |  2 ++
 lib/libnetlink.c     | 14 +++++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index 431189e..a88cb4d 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -75,6 +75,8 @@ int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
 	__attribute__((warn_unused_result));
 int rtnl_send(struct rtnl_handle *rth, const void *buf, int)
 	__attribute__((warn_unused_result));
+int rtnl_send_check_ack(struct rtnl_handle *rth, const void *buf, int, int)
+	__attribute__((warn_unused_result));
 int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int)
 	__attribute__((warn_unused_result));
 
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 1658214..a3ad83a 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -134,18 +134,21 @@ int rtnl_send(struct rtnl_handle *rth, const void *buf, int len)
 	return send(rth->fd, buf, len, 0);
 }
 
-int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int len)
+int rtnl_send_check_ack(struct rtnl_handle *rth, const void *buf, int len,
+		int ack)
 {
 	struct nlmsghdr *h;
-	int status;
+	int status, flags;
 	char resp[1024];
 
 	status = send(rth->fd, buf, len, 0);
 	if (status < 0)
 		return status;
 
+	flags = MSG_DONTWAIT | (ack ? 0 : MSG_PEEK);
+
 	/* Check for immediate errors */
-	status = recv(rth->fd, resp, sizeof(resp), MSG_DONTWAIT|MSG_PEEK);
+	status = recv(rth->fd, resp, sizeof(resp), flags);
 	if (status < 0) {
 		if (errno == EAGAIN)
 			return 0;
@@ -167,6 +170,11 @@ int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int len)
 	return 0;
 }
 
+inline int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int len)
+{
+	return rtnl_send_check_ack(rth, buf, len, 0);
+}
+
 int rtnl_dump_request(struct rtnl_handle *rth, int type, void *req, int len)
 {
 	struct nlmsghdr nlh;
-- 
2.6.0.rc2.230.g3dd15c0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2015-12-17 13:22 [iproute PATCH v2 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Lorenzo Colitti
@ 2015-12-17 13:22 ` Lorenzo Colitti
  2015-12-17 15:29   ` Eric Dumazet
  2015-12-22  5:42   ` Stephen Hemminger
  2015-12-17 16:07 ` [iproute PATCH v2 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Eric Dumazet
  1 sibling, 2 replies; 16+ messages in thread
From: Lorenzo Colitti @ 2015-12-17 13:22 UTC (permalink / raw)
  To: netdev; +Cc: stephen, eric.dumazet, zenczykowski, Lorenzo Colitti

This patch adds a -K / --kill option to ss that attempts to
forcibly close matching sockets using SOCK_DESTROY.

This is implemented by adding a new "struct action" to struct
filter. If necessary, this can be extended later on to support
further actions on sockets.

Because ss typically prints sockets instead of acting on them,
and because the kernel only suppors forcibly closing some types
of sockets, the output of -K is as follows:

- If closing the socket succeeds, the socket is printed.
- If the kernel does not support forcibly closing this type of
  socket (e.g., if it's a UDP socket, or a TIME_WAIT socket),
  the socket is silently skipped.
- If an error occurs (e.g., permission denied), the error is
  reported and ss exits.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 include/linux/sock_diag.h |  1 +
 misc/ss.c                 | 52 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/include/linux/sock_diag.h b/include/linux/sock_diag.h
index 024e1f4..dafcb89 100644
--- a/include/linux/sock_diag.h
+++ b/include/linux/sock_diag.h
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 
 #define SOCK_DIAG_BY_FAMILY 20
+#define SOCK_DESTROY 21
 
 struct sock_diag_req {
 	__u8	sdiag_family;
diff --git a/misc/ss.c b/misc/ss.c
index 0dab32c..be70c41 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -160,6 +160,9 @@ struct filter
 	int states;
 	int families;
 	struct ssfilter *f;
+	struct {
+		__u8 kill:1;
+	} action;
 };
 
 static const struct filter default_dbs[MAX_DB] = {
@@ -2194,8 +2197,27 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
 struct inet_diag_arg {
 	struct filter *f;
 	int protocol;
+	struct rtnl_handle *rth;
 };
 
+static int kill_inet_sock(const struct sockaddr_nl *addr,
+		struct nlmsghdr *h, void *arg)
+{
+	struct inet_diag_msg *d = NLMSG_DATA(h);
+	struct inet_diag_arg *diag_arg = arg;
+	struct rtnl_handle *rth = diag_arg->rth;
+	DIAG_REQUEST(req, struct inet_diag_req_v2 r);
+
+	req.nlh.nlmsg_type = SOCK_DESTROY;
+	req.nlh.nlmsg_flags = NLM_F_REQUEST;
+	req.nlh.nlmsg_seq = ++rth->seq;
+	req.r.sdiag_family = d->idiag_family;
+	req.r.sdiag_protocol = diag_arg->protocol;
+	req.r.id = d->id;
+
+	return rtnl_send_check_ack(rth, &req.nlh, req.nlh.nlmsg_len, 1);
+}
+
 static int show_one_inet_sock(const struct sockaddr_nl *addr,
 		struct nlmsghdr *h, void *arg)
 {
@@ -2205,6 +2227,15 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
 
 	if (!(diag_arg->f->families & (1 << r->idiag_family)))
 		return 0;
+	if (diag_arg->f->action.kill && kill_inet_sock(addr, h, arg) != 0) {
+		if (errno == EOPNOTSUPP) {
+			/* This socket can't be closed. Silently skip it. */
+			return 0;
+		} else {
+			perror("SOCK_DESTROY answers");
+			return -1;
+		}
+	}
 	if ((err = inet_show_sock(h, diag_arg->f, diag_arg->protocol)) < 0)
 		return err;
 
@@ -2214,12 +2245,21 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
 static int inet_show_netlink(struct filter *f, FILE *dump_fp, int protocol)
 {
 	int err = 0;
-	struct rtnl_handle rth;
+	struct rtnl_handle rth, rth2;
 	int family = PF_INET;
 	struct inet_diag_arg arg = { .f = f, .protocol = protocol };
 
 	if (rtnl_open_byproto(&rth, 0, NETLINK_SOCK_DIAG))
 		return -1;
+
+	if (f->action.kill) {
+		if (rtnl_open_byproto(&rth2, 0, NETLINK_SOCK_DIAG)) {
+			rtnl_close(&rth);
+			return -1;
+		}
+		arg.rth = &rth2;
+	}
+
 	rth.dump = MAGIC_SEQ;
 	rth.dump_fp = dump_fp;
 	if (preferred_family == PF_INET6)
@@ -2243,6 +2283,8 @@ again:
 
 Exit:
 	rtnl_close(&rth);
+	if (arg.rth)
+		rtnl_close(arg.rth);
 	return err;
 }
 
@@ -3489,6 +3531,8 @@ static void _usage(FILE *dest)
 "   -x, --unix          display only Unix domain sockets\n"
 "   -f, --family=FAMILY display sockets of type FAMILY\n"
 "\n"
+"   -K, --kill          forcibly close sockets, display what was closed\n"
+"\n"
 "   -A, --query=QUERY, --socket=QUERY\n"
 "       QUERY := {all|inet|tcp|udp|raw|unix|unix_dgram|unix_stream|unix_seqpacket|packet|netlink}[,QUERY]\n"
 "\n"
@@ -3579,6 +3623,7 @@ static const struct option long_opts[] = {
 	{ "context", 0, 0, 'Z' },
 	{ "contexts", 0, 0, 'z' },
 	{ "net", 1, 0, 'N' },
+	{ "kill", 0, 0, 'K' },
 	{ 0 }
 
 };
@@ -3593,7 +3638,7 @@ int main(int argc, char *argv[])
 	int ch;
 	int state_filter = 0;
 
-	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbEf:miA:D:F:vVzZN:",
+	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbEf:miA:D:F:vVzZN:K",
 				 long_opts, NULL)) != EOF) {
 		switch(ch) {
 		case 'n':
@@ -3774,6 +3819,9 @@ int main(int argc, char *argv[])
 			if (netns_switch(optarg))
 				exit(1);
 			break;
+		case 'K':
+			current_filter.action.kill = 1;
+			break;
 		case 'h':
 			help();
 		case '?':
-- 
2.6.0.rc2.230.g3dd15c0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2015-12-17 13:22 ` [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
@ 2015-12-17 15:29   ` Eric Dumazet
  2015-12-22  5:42   ` Stephen Hemminger
  1 sibling, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2015-12-17 15:29 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: netdev, stephen, zenczykowski

On Thu, 2015-12-17 at 22:22 +0900, Lorenzo Colitti wrote:
> This patch adds a -K / --kill option to ss that attempts to
> forcibly close matching sockets using SOCK_DESTROY.
> 
> This is implemented by adding a new "struct action" to struct
> filter. If necessary, this can be extended later on to support
> further actions on sockets.

Does not work for me :

lpaa23:~# ./ss dst lpaa24|wc -l
401
lpaa23:~# ./ss -K dst lpaa24|wc -l
1
lpaa23:~# ./ss dst lpaa24|wc -l
401
lpaa23:~# id
uid=0(root) gid=0(root)
groups=0(root),1(bin),2(daemon),3(sys),4(adm),6(disk),26(tape)

Kernel is latest David Miller net-next

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v2 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs
  2015-12-17 13:22 [iproute PATCH v2 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Lorenzo Colitti
  2015-12-17 13:22 ` [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
@ 2015-12-17 16:07 ` Eric Dumazet
  1 sibling, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2015-12-17 16:07 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: netdev, stephen, zenczykowski

On Thu, 2015-12-17 at 22:22 +0900, Lorenzo Colitti wrote:
> The new variant is identical to rtnl_send_check, except it also
> consumes the kernel response instead of using MSG_PEEK. This is
> useful for callers that send simple commands that never cause a
> response but only ACKs, and that expect to receive and deal
> with errors without printing them to stderr like rtnl_talk does.

> +inline int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int len)
> +{
> +	return rtnl_send_check_ack(rth, buf, len, 0);
> +}

Please remove this inline attribute.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2015-12-17 13:22 ` [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
  2015-12-17 15:29   ` Eric Dumazet
@ 2015-12-22  5:42   ` Stephen Hemminger
  2015-12-22  8:31     ` Lorenzo Colitti
  1 sibling, 1 reply; 16+ messages in thread
From: Stephen Hemminger @ 2015-12-22  5:42 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: netdev, eric.dumazet, zenczykowski

On Thu, 17 Dec 2015 22:22:18 +0900
Lorenzo Colitti <lorenzo@google.com> wrote:

> diff --git a/misc/ss.c b/misc/ss.c
> index 0dab32c..be70c41 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -160,6 +160,9 @@ struct filter
>  	int states;
>  	int families;
>  	struct ssfilter *f;
> +	struct {
> +		__u8 kill:1;
> +	} action;
>  };
>  
>  stati

Please just make it a boolean or integer, not a structure wrapped around a bit field.
If you need to extend in the future then change it then. Please don't write code
based on speculative future additions.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2015-12-22  5:42   ` Stephen Hemminger
@ 2015-12-22  8:31     ` Lorenzo Colitti
  2015-12-22  8:31       ` [iproute PATCH v3 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Lorenzo Colitti
                         ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Lorenzo Colitti @ 2015-12-22  8:31 UTC (permalink / raw)
  To: netdev; +Cc: stephen, eric.dumazet, zenczykowski

I've just uploaded a new version. The changes from v2 are as
follows:

- Removed the superfluous inline keyword.
- The code now ignores ENOENT from kill_inet_sock. This can
  happen if something else closed the socket during the scan, or
  if the user requests killing a socket that is not in the hash
  tables and thus cannot be found by inet_diag_find_one_icsk.
- The semantics of rtnl_send_check_ack are clearer. If the caller
  passes in ack=1, the function blocks until a response is
  received (unlike v2 which passed in MSG_PEEK). Also, an
  NLMSG_ERROR with an err of 0 is not treated as a failure.
- kill_inet_sock always requests an ACK when closing a socket.

This version is also tested on real hardware. The following work:

- Passing in -K as non-root immediately stops with EPERM.
- Running "ss -a -K dport = :22" closes SSH.
- Running ss -a -K dport = :5222 closes my XMPP connections,
  interrupts my chat client, and sends RSTs to the server.
- The above command silently skips TIME_WAIT sockets, which
  cannot be destroyed, without interrupting the dump.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [iproute PATCH v3 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs
  2015-12-22  8:31     ` Lorenzo Colitti
@ 2015-12-22  8:31       ` Lorenzo Colitti
  2015-12-23 21:17         ` Stephen Hemminger
  2015-12-22  8:31       ` [iproute PATCH v3 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
  2015-12-22  8:35       ` [iproute PATCH v2 " Lorenzo Colitti
  2 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Colitti @ 2015-12-22  8:31 UTC (permalink / raw)
  To: netdev; +Cc: stephen, eric.dumazet, zenczykowski, Lorenzo Colitti

The new variant is identical to rtnl_send_check, except it also
consumes the kernel response instead of using MSG_PEEK. This is
useful for callers that send simple commands that never cause a
response but only ACKs, and that expect to receive and deal
with errors without printing them to stderr like rtnl_talk does.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 include/libnetlink.h |  2 ++
 lib/libnetlink.c     | 16 +++++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index 431189e..a88cb4d 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -75,6 +75,8 @@ int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
 	__attribute__((warn_unused_result));
 int rtnl_send(struct rtnl_handle *rth, const void *buf, int)
 	__attribute__((warn_unused_result));
+int rtnl_send_check_ack(struct rtnl_handle *rth, const void *buf, int, int)
+	__attribute__((warn_unused_result));
 int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int)
 	__attribute__((warn_unused_result));
 
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 1658214..43cc73b 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -134,18 +134,21 @@ int rtnl_send(struct rtnl_handle *rth, const void *buf, int len)
 	return send(rth->fd, buf, len, 0);
 }
 
-int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int len)
+int rtnl_send_check_ack(struct rtnl_handle *rth, const void *buf, int len,
+		int ack)
 {
 	struct nlmsghdr *h;
-	int status;
+	int status, flags;
 	char resp[1024];
 
 	status = send(rth->fd, buf, len, 0);
 	if (status < 0)
 		return status;
 
+	flags = ack ? 0 : MSG_DONTWAIT|MSG_PEEK;
+
 	/* Check for immediate errors */
-	status = recv(rth->fd, resp, sizeof(resp), MSG_DONTWAIT|MSG_PEEK);
+	status = recv(rth->fd, resp, sizeof(resp), flags);
 	if (status < 0) {
 		if (errno == EAGAIN)
 			return 0;
@@ -160,6 +163,8 @@ int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int len)
 				fprintf(stderr, "ERROR truncated\n");
 			else 
 				errno = -err->error;
+			if (ack && errno == 0)
+				return 0;
 			return -1;
 		}
 	}
@@ -167,6 +172,11 @@ int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int len)
 	return 0;
 }
 
+int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int len)
+{
+	return rtnl_send_check_ack(rth, buf, len, 0);
+}
+
 int rtnl_dump_request(struct rtnl_handle *rth, int type, void *req, int len)
 {
 	struct nlmsghdr nlh;
-- 
2.6.0.rc2.230.g3dd15c0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [iproute PATCH v3 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2015-12-22  8:31     ` Lorenzo Colitti
  2015-12-22  8:31       ` [iproute PATCH v3 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Lorenzo Colitti
@ 2015-12-22  8:31       ` Lorenzo Colitti
  2015-12-30 20:34         ` Stephen Hemminger
  2015-12-22  8:35       ` [iproute PATCH v2 " Lorenzo Colitti
  2 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Colitti @ 2015-12-22  8:31 UTC (permalink / raw)
  To: netdev; +Cc: stephen, eric.dumazet, zenczykowski, Lorenzo Colitti

This patch adds a -K / --kill option to ss that attempts to
forcibly close matching sockets using SOCK_DESTROY.

Because ss typically prints sockets instead of acting on them,
and because the kernel only supports forcibly closing some types
of sockets, the output of -K is as follows:

- If closing the socket succeeds, the socket is printed.
- If the kernel does not support forcibly closing this type of
  socket (e.g., if it's a UDP socket, or a TIME_WAIT socket),
  the socket is silently skipped.
- If an error occurs (e.g., permission denied), the error is
  reported and ss exits.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 include/linux/sock_diag.h |  1 +
 misc/ss.c                 | 50 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/include/linux/sock_diag.h b/include/linux/sock_diag.h
index 024e1f4..dafcb89 100644
--- a/include/linux/sock_diag.h
+++ b/include/linux/sock_diag.h
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 
 #define SOCK_DIAG_BY_FAMILY 20
+#define SOCK_DESTROY 21
 
 struct sock_diag_req {
 	__u8	sdiag_family;
diff --git a/misc/ss.c b/misc/ss.c
index 0dab32c..4d60f14 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -160,6 +160,7 @@ struct filter
 	int states;
 	int families;
 	struct ssfilter *f;
+	bool kill;
 };
 
 static const struct filter default_dbs[MAX_DB] = {
@@ -2194,8 +2195,27 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
 struct inet_diag_arg {
 	struct filter *f;
 	int protocol;
+	struct rtnl_handle *rth;
 };
 
+static int kill_inet_sock(const struct sockaddr_nl *addr,
+		struct nlmsghdr *h, void *arg)
+{
+	struct inet_diag_msg *d = NLMSG_DATA(h);
+	struct inet_diag_arg *diag_arg = arg;
+	struct rtnl_handle *rth = diag_arg->rth;
+	DIAG_REQUEST(req, struct inet_diag_req_v2 r);
+
+	req.nlh.nlmsg_type = SOCK_DESTROY;
+	req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+	req.nlh.nlmsg_seq = ++rth->seq;
+	req.r.sdiag_family = d->idiag_family;
+	req.r.sdiag_protocol = diag_arg->protocol;
+	req.r.id = d->id;
+
+	return rtnl_send_check_ack(rth, &req.nlh, req.nlh.nlmsg_len, 1);
+}
+
 static int show_one_inet_sock(const struct sockaddr_nl *addr,
 		struct nlmsghdr *h, void *arg)
 {
@@ -2205,6 +2225,15 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
 
 	if (!(diag_arg->f->families & (1 << r->idiag_family)))
 		return 0;
+	if (diag_arg->f->kill && kill_inet_sock(addr, h, arg) != 0) {
+		if (errno == EOPNOTSUPP || errno == ENOENT) {
+			/* Socket can't be closed, or is already closed. */
+			return 0;
+		} else {
+			perror("SOCK_DESTROY answers");
+			return -1;
+		}
+	}
 	if ((err = inet_show_sock(h, diag_arg->f, diag_arg->protocol)) < 0)
 		return err;
 
@@ -2214,12 +2243,21 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
 static int inet_show_netlink(struct filter *f, FILE *dump_fp, int protocol)
 {
 	int err = 0;
-	struct rtnl_handle rth;
+	struct rtnl_handle rth, rth2;
 	int family = PF_INET;
 	struct inet_diag_arg arg = { .f = f, .protocol = protocol };
 
 	if (rtnl_open_byproto(&rth, 0, NETLINK_SOCK_DIAG))
 		return -1;
+
+	if (f->kill) {
+		if (rtnl_open_byproto(&rth2, 0, NETLINK_SOCK_DIAG)) {
+			rtnl_close(&rth);
+			return -1;
+		}
+		arg.rth = &rth2;
+	}
+
 	rth.dump = MAGIC_SEQ;
 	rth.dump_fp = dump_fp;
 	if (preferred_family == PF_INET6)
@@ -2243,6 +2281,8 @@ again:
 
 Exit:
 	rtnl_close(&rth);
+	if (arg.rth)
+		rtnl_close(arg.rth);
 	return err;
 }
 
@@ -3489,6 +3529,8 @@ static void _usage(FILE *dest)
 "   -x, --unix          display only Unix domain sockets\n"
 "   -f, --family=FAMILY display sockets of type FAMILY\n"
 "\n"
+"   -K, --kill          forcibly close sockets, display what was closed\n"
+"\n"
 "   -A, --query=QUERY, --socket=QUERY\n"
 "       QUERY := {all|inet|tcp|udp|raw|unix|unix_dgram|unix_stream|unix_seqpacket|packet|netlink}[,QUERY]\n"
 "\n"
@@ -3579,6 +3621,7 @@ static const struct option long_opts[] = {
 	{ "context", 0, 0, 'Z' },
 	{ "contexts", 0, 0, 'z' },
 	{ "net", 1, 0, 'N' },
+	{ "kill", 0, 0, 'K' },
 	{ 0 }
 
 };
@@ -3593,7 +3636,7 @@ int main(int argc, char *argv[])
 	int ch;
 	int state_filter = 0;
 
-	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbEf:miA:D:F:vVzZN:",
+	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbEf:miA:D:F:vVzZN:K",
 				 long_opts, NULL)) != EOF) {
 		switch(ch) {
 		case 'n':
@@ -3774,6 +3817,9 @@ int main(int argc, char *argv[])
 			if (netns_switch(optarg))
 				exit(1);
 			break;
+		case 'K':
+			current_filter.kill = 1;
+			break;
 		case 'h':
 			help();
 		case '?':
-- 
2.6.0.rc2.230.g3dd15c0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2015-12-22  8:31     ` Lorenzo Colitti
  2015-12-22  8:31       ` [iproute PATCH v3 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Lorenzo Colitti
  2015-12-22  8:31       ` [iproute PATCH v3 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
@ 2015-12-22  8:35       ` Lorenzo Colitti
  2 siblings, 0 replies; 16+ messages in thread
From: Lorenzo Colitti @ 2015-12-22  8:35 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger, Eric Dumazet, Maciej Żenczykowski

On Tue, Dec 22, 2015 at 5:31 PM, Lorenzo Colitti <lorenzo@google.com> wrote:
> I've just uploaded a new version. The changes from v2 are as
> follows:
> [...]

Also: as requested, kill is now a boolean instead of a bitfield inside a struct.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v3 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs
  2015-12-22  8:31       ` [iproute PATCH v3 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Lorenzo Colitti
@ 2015-12-23 21:17         ` Stephen Hemminger
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2015-12-23 21:17 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: netdev, eric.dumazet, zenczykowski

On Tue, 22 Dec 2015 17:31:33 +0900
Lorenzo Colitti <lorenzo@google.com> wrote:

> The new variant is identical to rtnl_send_check, except it also
> consumes the kernel response instead of using MSG_PEEK. This is
> useful for callers that send simple commands that never cause a
> response but only ACKs, and that expect to receive and deal
> with errors without printing them to stderr like rtnl_talk does.
> 
> Signed-off-by: Lorenzo Colitti <lorenzo@google.com>

Originally, iproute2 used netlink so that every request had an ACK and
this is the API in rtnl_talk. Then as an optimization it was observed
that ACK from kernel is not necessary (all error reports are handled in
send), but that for some asynchronous errors a check was necessary.

Therefore, I wonder why you need this, either:
 * don't ask kernel for ACK's (like most other ip commands),
 * or use rtnl_talk() and expect ACK.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v3 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2015-12-22  8:31       ` [iproute PATCH v3 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
@ 2015-12-30 20:34         ` Stephen Hemminger
  2016-01-04  1:54           ` Lorenzo Colitti
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Hemminger @ 2015-12-30 20:34 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: netdev, eric.dumazet, zenczykowski

On Tue, 22 Dec 2015 17:31:34 +0900
Lorenzo Colitti <lorenzo@google.com> wrote:

>  
> +static int kill_inet_sock(const struct sockaddr_nl *addr,
> +		struct nlmsghdr *h, void *arg)
> +{
> +	struct inet_diag_msg *d = NLMSG_DATA(h);
> +	struct inet_diag_arg *diag_arg = arg;
> +	struct rtnl_handle *rth = diag_arg->rth;
> +	DIAG_REQUEST(req, struct inet_diag_req_v2 r);
> +
> +	req.nlh.nlmsg_type = SOCK_DESTROY;
> +	req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
> +	req.nlh.nlmsg_seq = ++rth->seq;
> +	req.r.sdiag_family = d->idiag_family;
> +	req.r.sdiag_protocol = diag_arg->protocol;
> +	req.r.id = d->id;
> +
> +	return rtnl_send_check_ack(rth, &req.nlh, req.nlh.nlmsg_len, 1);

Just use rtnl_talk() instead, it does request/reply.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v3 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2015-12-30 20:34         ` Stephen Hemminger
@ 2016-01-04  1:54           ` Lorenzo Colitti
  2016-01-08  8:32             ` Lorenzo Colitti
  0 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Colitti @ 2016-01-04  1:54 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Eric Dumazet, Maciej Żenczykowski

On Thu, Dec 31, 2015 at 5:34 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>> +     req.nlh.nlmsg_type = SOCK_DESTROY;
>> +     req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
>> +     req.nlh.nlmsg_seq = ++rth->seq;
>> +     req.r.sdiag_family = d->idiag_family;
>> +     req.r.sdiag_protocol = diag_arg->protocol;
>> +     req.r.id = d->id;
>> +
>> +     return rtnl_send_check_ack(rth, &req.nlh, req.nlh.nlmsg_len, 1);
>
> Just use rtnl_talk() instead, it does request/reply.

The reason I did not use rtnl_talk is that it prints all errors to
stderr. This does not
fit well with SOCK_DESTROY, for which it is expected that some
operations will fail.

For example, if you type "ss -a -K dport = :443", you probably don't
want to see one "RTNETLINK answers: Operation not supported" error for
every TIME-WAIT socket to port 443, and you don't want to see
"RTNETLINK answers: No such file or directory" if one of those sockets
happens to be closed during the scan. Silently ignoring these errors
seemed best.

I could also add a parameter to rtnl_talk to suppress printing errors,
though the patch to do so would be roughly equivalent to the patch
where I added rtnl_send_check_ack. I can also just use rtnl_talk as
you suggest and not care about the errors.

Let me know what you prefer.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v3 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2016-01-04  1:54           ` Lorenzo Colitti
@ 2016-01-08  8:32             ` Lorenzo Colitti
  2016-01-08  8:32               ` [iproute PATCH v4 1/2] libnetlink: don't print NETLINK_SOCK_DIAG errors in rtnl_talk Lorenzo Colitti
  2016-01-08  8:32               ` [iproute PATCH v4 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
  0 siblings, 2 replies; 16+ messages in thread
From: Lorenzo Colitti @ 2016-01-08  8:32 UTC (permalink / raw)
  To: netdev; +Cc: stephen, eric.dumazet, zenczykowski

This new version uses rtnl_talk as requested. It solves the
problem of spurious error messages by making rtnl_talk not print
any netlink-returned errors when operating on a NETLINK_SOCK_DIAG
socket.

Because there is currently no code that calls rtnl_talk on a
NETLINK_SOCK_DIAG socket, this does not affect existing
behaviour. The code in this patchset prints netlink errors by
itself, and any future code that does this will need to be
responsible for doing the same.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [iproute PATCH v4 1/2] libnetlink: don't print NETLINK_SOCK_DIAG errors in rtnl_talk
  2016-01-08  8:32             ` Lorenzo Colitti
@ 2016-01-08  8:32               ` Lorenzo Colitti
  2016-01-08  8:32               ` [iproute PATCH v4 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
  1 sibling, 0 replies; 16+ messages in thread
From: Lorenzo Colitti @ 2016-01-08  8:32 UTC (permalink / raw)
  To: netdev; +Cc: stephen, eric.dumazet, zenczykowski, Lorenzo Colitti

This change is a no-op, as currently no code uses rtnl_talk on
NETLINK_SOCK_DIAG_BY_FAMILY sockets. It is needed to suppress
spurious errors when using SOCK_DESTROY via rtnl_talk.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 lib/libnetlink.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 1658214..d6b5fd3 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -419,8 +419,10 @@ int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
 					return 0;
 				}
 
-				fprintf(stderr, "RTNETLINK answers: %s\n",
-					strerror(-err->error));
+				if (rtnl->proto != NETLINK_SOCK_DIAG)
+					fprintf(stderr,
+						"RTNETLINK answers: %s\n",
+						strerror(-err->error));
 				errno = -err->error;
 				return -1;
 			}
-- 
2.6.0.rc2.230.g3dd15c0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [iproute PATCH v4 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2016-01-08  8:32             ` Lorenzo Colitti
  2016-01-08  8:32               ` [iproute PATCH v4 1/2] libnetlink: don't print NETLINK_SOCK_DIAG errors in rtnl_talk Lorenzo Colitti
@ 2016-01-08  8:32               ` Lorenzo Colitti
  2016-01-18 19:48                 ` Stephen Hemminger
  1 sibling, 1 reply; 16+ messages in thread
From: Lorenzo Colitti @ 2016-01-08  8:32 UTC (permalink / raw)
  To: netdev; +Cc: stephen, eric.dumazet, zenczykowski, Lorenzo Colitti

This patch adds a -K / --kill option to ss that attempts to
forcibly close matching sockets using SOCK_DESTROY.

Because ss typically prints sockets instead of acting on them,
and because the kernel only supports forcibly closing some types
of sockets, the output of -K is as follows:

- If closing the socket succeeds, the socket is printed.
- If the kernel does not support forcibly closing this type of
  socket (e.g., if it's a UDP socket, or a TIME_WAIT socket),
  the socket is silently skipped.
- If an error occurs (e.g., permission denied), the error is
  reported and ss exits.

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
 include/linux/sock_diag.h |  1 +
 man/man8/ss.8             |  5 +++++
 misc/ss.c                 | 50 +++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/include/linux/sock_diag.h b/include/linux/sock_diag.h
index 024e1f4..dafcb89 100644
--- a/include/linux/sock_diag.h
+++ b/include/linux/sock_diag.h
@@ -4,6 +4,7 @@
 #include <linux/types.h>
 
 #define SOCK_DIAG_BY_FAMILY 20
+#define SOCK_DESTROY 21
 
 struct sock_diag_req {
 	__u8	sdiag_family;
diff --git a/man/man8/ss.8 b/man/man8/ss.8
index f4d5264..758460c 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -48,6 +48,11 @@ Show process using socket.
 .B \-i, \-\-info
 Show internal TCP information.
 .TP
+.B \-K, \-\-kill
+Attempts to forcibly close sockets. This option displays sockets that are
+successfully closed and silently skips sockets that the kernel does not support
+closing. It supports IPv4 and IPv6 sockets only.
+.TP
 .B \-s, \-\-summary
 Print summary statistics. This option does not parse socket lists obtaining
 summary from various sources. It is useful when amount of sockets is so huge
diff --git a/misc/ss.c b/misc/ss.c
index 0dab32c..13fcc8f 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -160,6 +160,7 @@ struct filter
 	int states;
 	int families;
 	struct ssfilter *f;
+	bool kill;
 };
 
 static const struct filter default_dbs[MAX_DB] = {
@@ -2194,8 +2195,27 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
 struct inet_diag_arg {
 	struct filter *f;
 	int protocol;
+	struct rtnl_handle *rth;
 };
 
+static int kill_inet_sock(const struct sockaddr_nl *addr,
+		struct nlmsghdr *h, void *arg)
+{
+	struct inet_diag_msg *d = NLMSG_DATA(h);
+	struct inet_diag_arg *diag_arg = arg;
+	struct rtnl_handle *rth = diag_arg->rth;
+	DIAG_REQUEST(req, struct inet_diag_req_v2 r);
+
+	req.nlh.nlmsg_type = SOCK_DESTROY;
+	req.nlh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+	req.nlh.nlmsg_seq = ++rth->seq;
+	req.r.sdiag_family = d->idiag_family;
+	req.r.sdiag_protocol = diag_arg->protocol;
+	req.r.id = d->id;
+
+	return rtnl_talk(rth, &req.nlh, NULL, 0);
+}
+
 static int show_one_inet_sock(const struct sockaddr_nl *addr,
 		struct nlmsghdr *h, void *arg)
 {
@@ -2205,6 +2225,15 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
 
 	if (!(diag_arg->f->families & (1 << r->idiag_family)))
 		return 0;
+	if (diag_arg->f->kill && kill_inet_sock(addr, h, arg) != 0) {
+		if (errno == EOPNOTSUPP || errno == ENOENT) {
+			/* Socket can't be closed, or is already closed. */
+			return 0;
+		} else {
+			perror("SOCK_DESTROY answers");
+			return -1;
+		}
+	}
 	if ((err = inet_show_sock(h, diag_arg->f, diag_arg->protocol)) < 0)
 		return err;
 
@@ -2214,12 +2243,21 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
 static int inet_show_netlink(struct filter *f, FILE *dump_fp, int protocol)
 {
 	int err = 0;
-	struct rtnl_handle rth;
+	struct rtnl_handle rth, rth2;
 	int family = PF_INET;
 	struct inet_diag_arg arg = { .f = f, .protocol = protocol };
 
 	if (rtnl_open_byproto(&rth, 0, NETLINK_SOCK_DIAG))
 		return -1;
+
+	if (f->kill) {
+		if (rtnl_open_byproto(&rth2, 0, NETLINK_SOCK_DIAG)) {
+			rtnl_close(&rth);
+			return -1;
+		}
+		arg.rth = &rth2;
+	}
+
 	rth.dump = MAGIC_SEQ;
 	rth.dump_fp = dump_fp;
 	if (preferred_family == PF_INET6)
@@ -2243,6 +2281,8 @@ again:
 
 Exit:
 	rtnl_close(&rth);
+	if (arg.rth)
+		rtnl_close(arg.rth);
 	return err;
 }
 
@@ -3489,6 +3529,8 @@ static void _usage(FILE *dest)
 "   -x, --unix          display only Unix domain sockets\n"
 "   -f, --family=FAMILY display sockets of type FAMILY\n"
 "\n"
+"   -K, --kill          forcibly close sockets, display what was closed\n"
+"\n"
 "   -A, --query=QUERY, --socket=QUERY\n"
 "       QUERY := {all|inet|tcp|udp|raw|unix|unix_dgram|unix_stream|unix_seqpacket|packet|netlink}[,QUERY]\n"
 "\n"
@@ -3579,6 +3621,7 @@ static const struct option long_opts[] = {
 	{ "context", 0, 0, 'Z' },
 	{ "contexts", 0, 0, 'z' },
 	{ "net", 1, 0, 'N' },
+	{ "kill", 0, 0, 'K' },
 	{ 0 }
 
 };
@@ -3593,7 +3636,7 @@ int main(int argc, char *argv[])
 	int ch;
 	int state_filter = 0;
 
-	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbEf:miA:D:F:vVzZN:",
+	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbEf:miA:D:F:vVzZN:K",
 				 long_opts, NULL)) != EOF) {
 		switch(ch) {
 		case 'n':
@@ -3774,6 +3817,9 @@ int main(int argc, char *argv[])
 			if (netns_switch(optarg))
 				exit(1);
 			break;
+		case 'K':
+			current_filter.kill = 1;
+			break;
 		case 'h':
 			help();
 		case '?':
-- 
2.6.0.rc2.230.g3dd15c0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [iproute PATCH v4 2/2] ss: support closing inet sockets via SOCK_DESTROY.
  2016-01-08  8:32               ` [iproute PATCH v4 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
@ 2016-01-18 19:48                 ` Stephen Hemminger
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Hemminger @ 2016-01-18 19:48 UTC (permalink / raw)
  To: Lorenzo Colitti; +Cc: netdev, eric.dumazet, zenczykowski

On Fri,  8 Jan 2016 17:32:37 +0900
Lorenzo Colitti <lorenzo@google.com> wrote:

> This patch adds a -K / --kill option to ss that attempts to
> forcibly close matching sockets using SOCK_DESTROY.
> 
> Because ss typically prints sockets instead of acting on them,
> and because the kernel only supports forcibly closing some types
> of sockets, the output of -K is as follows:
> 
> - If closing the socket succeeds, the socket is printed.
> - If the kernel does not support forcibly closing this type of
>   socket (e.g., if it's a UDP socket, or a TIME_WAIT socket),
>   the socket is silently skipped.
> - If an error occurs (e.g., permission denied), the error is
>   reported and ss exits.
> 
> Signed-off-by: Lorenzo Colitti <lorenzo@google.com>

Both applied, thanks

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-01-18 19:48 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-17 13:22 [iproute PATCH v2 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Lorenzo Colitti
2015-12-17 13:22 ` [iproute PATCH v2 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
2015-12-17 15:29   ` Eric Dumazet
2015-12-22  5:42   ` Stephen Hemminger
2015-12-22  8:31     ` Lorenzo Colitti
2015-12-22  8:31       ` [iproute PATCH v3 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Lorenzo Colitti
2015-12-23 21:17         ` Stephen Hemminger
2015-12-22  8:31       ` [iproute PATCH v3 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
2015-12-30 20:34         ` Stephen Hemminger
2016-01-04  1:54           ` Lorenzo Colitti
2016-01-08  8:32             ` Lorenzo Colitti
2016-01-08  8:32               ` [iproute PATCH v4 1/2] libnetlink: don't print NETLINK_SOCK_DIAG errors in rtnl_talk Lorenzo Colitti
2016-01-08  8:32               ` [iproute PATCH v4 2/2] ss: support closing inet sockets via SOCK_DESTROY Lorenzo Colitti
2016-01-18 19:48                 ` Stephen Hemminger
2015-12-22  8:35       ` [iproute PATCH v2 " Lorenzo Colitti
2015-12-17 16:07 ` [iproute PATCH v2 1/2] libnetlink: add a variant of rtnl_send_check that consumes ACKs Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).