diff mbox series

[mptcp-next,v2,3/3] mptcp: listen diag dump support

Message ID 20220328124913.29768-4-fw@strlen.de (mailing list archive)
State Superseded, archived
Commit b433728093ddce95c5e45b44a8ba73b410079efe
Headers show
Series mptcp: inet diag listen dump support | expand

Commit Message

Florian Westphal March 28, 2022, 12:49 p.m. UTC 
makes 'ss -Ml' show mptcp listen sockets.

Iterate over the tcp listen sockets and pick those that have mptcp ulp
info attached.

mptcp_diag_get_info() is modified to prefer msk->first for mptcp sockets
in listen state.  This reports accurate number for recv and send queue
(pending / max connection backlog counters).

Sample output:
ss -Mil
State        Recv-Q Send-Q Local Address:Port  Peer Address:Port
LISTEN       0      20     127.0.0.1:12000     0.0.0.0:*
         subflows_max:2

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Changes:
  - place functionality in helper function
  - prefer msk->first for msk listeners
  - more verbose commit message

 net/mptcp/mptcp_diag.c | 88 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

Comments

kernel test robot March 28, 2022, 6:42 p.m. UTC | #1 
Hi Florian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mptcp/export]
[also build test WARNING on linus/master v5.17 next-20220328]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/mptcp-inet-diag-listen-dump-support/20220328-205028
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: x86_64-randconfig-a001 (https://download.01.org/0day-ci/archive/20220329/202203290201.H3kHXAzz-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0f6d9501cf49ce02937099350d08f20c4af86f3d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/6ef1c5326220cfa09c48f9f9c6e02ec914250e3e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Florian-Westphal/mptcp-inet-diag-listen-dump-support/20220328-205028
        git checkout 6ef1c5326220cfa09c48f9f9c6e02ec914250e3e
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash net/mptcp/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/mptcp/mptcp_diag.c:121:39: warning: variable 'bc' is uninitialized when used here [-Wuninitialized]
                           ret = sk_diag_dump(sk, skb, cb, r, bc, net_admin);
                                                              ^~
   net/mptcp/mptcp_diag.c:82:19: note: initialize the variable 'bc' to silence this warning
           struct nlattr *bc;
                            ^
                             = NULL
   1 warning generated.


vim +/bc +121 net/mptcp/mptcp_diag.c

    75	
    76	static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callback *cb,
    77					      const struct inet_diag_req_v2 *r,
    78					      bool net_admin)
    79	{
    80		struct mptcp_diag_ctx *diag_ctx = (void *)cb->ctx;
    81		struct net *net = sock_net(skb->sk);
    82		struct nlattr *bc;
    83		int i;
    84	
    85		for (i = diag_ctx->l_slot; i < INET_LHTABLE_SIZE; i++) {
    86			struct inet_listen_hashbucket *ilb;
    87			struct hlist_nulls_node *node;
    88			struct sock *sk;
    89			int num = 0;
    90	
    91			ilb = &tcp_hashinfo.listening_hash[i];
    92	
    93			rcu_read_lock();
    94			spin_lock(&ilb->lock);
    95			sk_nulls_for_each(sk, node, &ilb->nulls_head) {
    96				const struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(sk);
    97				struct inet_sock *inet = inet_sk(sk);
    98				int ret;
    99	
   100				if (num < diag_ctx->l_num)
   101					goto next_listen;
   102	
   103				if (!ctx || strcmp(inet_csk(sk)->icsk_ulp_ops->name, "mptcp"))
   104					goto next_listen;
   105	
   106				sk = ctx->conn;
   107				if (!sk || !net_eq(sock_net(sk), net))
   108					goto next_listen;
   109	
   110				if (r->sdiag_family != AF_UNSPEC &&
   111				    sk->sk_family != r->sdiag_family)
   112					goto next_listen;
   113	
   114				if (r->id.idiag_sport != inet->inet_sport &&
   115				    r->id.idiag_sport)
   116					goto next_listen;
   117	
   118				if (!refcount_inc_not_zero(&sk->sk_refcnt))
   119					goto next_listen;
   120	
 > 121				ret = sk_diag_dump(sk, skb, cb, r, bc, net_admin);
   122						   sock_put(sk);
   123				if (ret < 0) {
   124					spin_unlock(&ilb->lock);
   125					rcu_read_unlock();
   126					diag_ctx->l_slot = i;
   127					diag_ctx->l_num = num;
   128					return;
   129				}
   130				diag_ctx->l_num = num + 1;
   131				num = 0;
   132	next_listen:
   133				++num;
   134			}
   135			spin_unlock(&ilb->lock);
   136			rcu_read_unlock();
   137	
   138			cond_resched();
   139			diag_ctx->l_num = 0;
   140		}
   141	
   142		diag_ctx->l_num = 0;
   143		diag_ctx->l_slot = i;
   144	}
   145
Mat Martineau March 29, 2022, 1:09 a.m. UTC | #2 
On Mon, 28 Mar 2022, Florian Westphal wrote:

> makes 'ss -Ml' show mptcp listen sockets.
>
> Iterate over the tcp listen sockets and pick those that have mptcp ulp
> info attached.
>
> mptcp_diag_get_info() is modified to prefer msk->first for mptcp sockets
> in listen state.  This reports accurate number for recv and send queue
> (pending / max connection backlog counters).
>
> Sample output:
> ss -Mil
> State        Recv-Q Send-Q Local Address:Port  Peer Address:Port
> LISTEN       0      20     127.0.0.1:12000     0.0.0.0:*
>         subflows_max:2
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> Changes:
>  - place functionality in helper function
>  - prefer msk->first for msk listeners
>  - more verbose commit message
>
> net/mptcp/mptcp_diag.c | 88 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 88 insertions(+)
>
> diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
> index c4992eeb67d8..076dd99d35d9 100644
> --- a/net/mptcp/mptcp_diag.c
> +++ b/net/mptcp/mptcp_diag.c
> @@ -69,8 +69,80 @@ static int mptcp_diag_dump_one(struct netlink_callback *cb,
> struct mptcp_diag_ctx {
> 	long s_slot;
> 	long s_num;
> +	unsigned int l_slot;
> +	unsigned int l_num;
> };
>
> +static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callback *cb,
> +				      const struct inet_diag_req_v2 *r,
> +				      bool net_admin)
> +{
> +	struct mptcp_diag_ctx *diag_ctx = (void *)cb->ctx;
> +	struct net *net = sock_net(skb->sk);
> +	struct nlattr *bc;
> +	int i;
> +
> +	for (i = diag_ctx->l_slot; i < INET_LHTABLE_SIZE; i++) {
> +		struct inet_listen_hashbucket *ilb;
> +		struct hlist_nulls_node *node;
> +		struct sock *sk;
> +		int num = 0;
> +
> +		ilb = &tcp_hashinfo.listening_hash[i];
> +
> +		rcu_read_lock();
> +		spin_lock(&ilb->lock);
> +		sk_nulls_for_each(sk, node, &ilb->nulls_head) {
> +			const struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(sk);
> +			struct inet_sock *inet = inet_sk(sk);
> +			int ret;
> +
> +			if (num < diag_ctx->l_num)
> +				goto next_listen;
> +
> +			if (!ctx || strcmp(inet_csk(sk)->icsk_ulp_ops->name, "mptcp"))
> +				goto next_listen;
> +
> +			sk = ctx->conn;
> +			if (!sk || !net_eq(sock_net(sk), net))
> +				goto next_listen;
> +
> +			if (r->sdiag_family != AF_UNSPEC &&
> +			    sk->sk_family != r->sdiag_family)
> +				goto next_listen;
> +
> +			if (r->id.idiag_sport != inet->inet_sport &&
> +			    r->id.idiag_sport)
> +				goto next_listen;
> +
> +			if (!refcount_inc_not_zero(&sk->sk_refcnt))
> +				goto next_listen;
> +
> +			ret = sk_diag_dump(sk, skb, cb, r, bc, net_admin);

As the kbuild bot noticed, bc is uninitialized here.

> +					   sock_put(sk);

Extra tabs here.

It built and ran fine, with some manual tests and diag.sh. Speaking of 
diag.sh, some coverage there for the new MPTCP listener functionality 
would be good.

- Mat

> +			if (ret < 0) {
> +				spin_unlock(&ilb->lock);
> +				rcu_read_unlock();
> +				diag_ctx->l_slot = i;
> +				diag_ctx->l_num = num;
> +				return;
> +			}
> +			diag_ctx->l_num = num + 1;
> +			num = 0;
> +next_listen:
> +			++num;
> +		}
> +		spin_unlock(&ilb->lock);
> +		rcu_read_unlock();
> +
> +		cond_resched();
> +		diag_ctx->l_num = 0;
> +	}
> +
> +	diag_ctx->l_num = 0;
> +	diag_ctx->l_slot = i;
> +}
> +
> static void mptcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> 			    const struct inet_diag_req_v2 *r)
> {
> @@ -114,6 +186,9 @@ static void mptcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
> 		}
> 		cond_resched();
> 	}
> +
> +	if ((r->idiag_states & TCPF_LISTEN) && r->id.idiag_dport == 0)
> +		mptcp_diag_dump_listeners(skb, cb, r, net_admin);
> }
>
> static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
> @@ -124,6 +199,19 @@ static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
>
> 	r->idiag_rqueue = sk_rmem_alloc_get(sk);
> 	r->idiag_wqueue = sk_wmem_alloc_get(sk);
> +
> +	if (inet_sk_state_load(sk) == TCP_LISTEN) {
> +		struct sock *lsk = READ_ONCE(msk->first);
> +
> +		if (lsk) {
> +			/* override with settings from tcp listener,
> +			 * so Send-Q will show accept queue.
> +			 */
> +			r->idiag_rqueue = READ_ONCE(lsk->sk_ack_backlog);
> +			r->idiag_wqueue = READ_ONCE(lsk->sk_max_ack_backlog);
> +		}
> +	}
> +
> 	if (!info)
> 		return;
>
> -- 
> 2.35.1
>
>
>

--
Mat Martineau
Intel
Dan Carpenter March 29, 2022, 8:18 a.m. UTC | #3 
Hi Florian,

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/mptcp-inet-diag-listen-dump-support/20220328-205028
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: i386-randconfig-m021-20220328 (https://download.01.org/0day-ci/archive/20220329/202203290401.iBK94uHx-lkp@intel.com/config)
compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
net/mptcp/mptcp_diag.c:121 mptcp_diag_dump_listeners() error: uninitialized symbol 'bc'.
net/mptcp/mptcp_diag.c:122 mptcp_diag_dump_listeners() warn: inconsistent indenting

vim +/bc +121 net/mptcp/mptcp_diag.c

6ef1c5326220cf Florian Westphal 2022-03-28   76  static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callback *cb,
6ef1c5326220cf Florian Westphal 2022-03-28   77  				      const struct inet_diag_req_v2 *r,
6ef1c5326220cf Florian Westphal 2022-03-28   78  				      bool net_admin)
6ef1c5326220cf Florian Westphal 2022-03-28   79  {
6ef1c5326220cf Florian Westphal 2022-03-28   80  	struct mptcp_diag_ctx *diag_ctx = (void *)cb->ctx;
6ef1c5326220cf Florian Westphal 2022-03-28   81  	struct net *net = sock_net(skb->sk);
6ef1c5326220cf Florian Westphal 2022-03-28   82  	struct nlattr *bc;
6ef1c5326220cf Florian Westphal 2022-03-28   83  	int i;
6ef1c5326220cf Florian Westphal 2022-03-28   84  
6ef1c5326220cf Florian Westphal 2022-03-28   85  	for (i = diag_ctx->l_slot; i < INET_LHTABLE_SIZE; i++) {
6ef1c5326220cf Florian Westphal 2022-03-28   86  		struct inet_listen_hashbucket *ilb;
6ef1c5326220cf Florian Westphal 2022-03-28   87  		struct hlist_nulls_node *node;
6ef1c5326220cf Florian Westphal 2022-03-28   88  		struct sock *sk;
6ef1c5326220cf Florian Westphal 2022-03-28   89  		int num = 0;
6ef1c5326220cf Florian Westphal 2022-03-28   90  
6ef1c5326220cf Florian Westphal 2022-03-28   91  		ilb = &tcp_hashinfo.listening_hash[i];
6ef1c5326220cf Florian Westphal 2022-03-28   92  
6ef1c5326220cf Florian Westphal 2022-03-28   93  		rcu_read_lock();
6ef1c5326220cf Florian Westphal 2022-03-28   94  		spin_lock(&ilb->lock);
6ef1c5326220cf Florian Westphal 2022-03-28   95  		sk_nulls_for_each(sk, node, &ilb->nulls_head) {
6ef1c5326220cf Florian Westphal 2022-03-28   96  			const struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(sk);
6ef1c5326220cf Florian Westphal 2022-03-28   97  			struct inet_sock *inet = inet_sk(sk);
6ef1c5326220cf Florian Westphal 2022-03-28   98  			int ret;
6ef1c5326220cf Florian Westphal 2022-03-28   99  
6ef1c5326220cf Florian Westphal 2022-03-28  100  			if (num < diag_ctx->l_num)
6ef1c5326220cf Florian Westphal 2022-03-28  101  				goto next_listen;
6ef1c5326220cf Florian Westphal 2022-03-28  102  
6ef1c5326220cf Florian Westphal 2022-03-28  103  			if (!ctx || strcmp(inet_csk(sk)->icsk_ulp_ops->name, "mptcp"))
6ef1c5326220cf Florian Westphal 2022-03-28  104  				goto next_listen;
6ef1c5326220cf Florian Westphal 2022-03-28  105  
6ef1c5326220cf Florian Westphal 2022-03-28  106  			sk = ctx->conn;
6ef1c5326220cf Florian Westphal 2022-03-28  107  			if (!sk || !net_eq(sock_net(sk), net))
6ef1c5326220cf Florian Westphal 2022-03-28  108  				goto next_listen;
6ef1c5326220cf Florian Westphal 2022-03-28  109  
6ef1c5326220cf Florian Westphal 2022-03-28  110  			if (r->sdiag_family != AF_UNSPEC &&
6ef1c5326220cf Florian Westphal 2022-03-28  111  			    sk->sk_family != r->sdiag_family)
6ef1c5326220cf Florian Westphal 2022-03-28  112  				goto next_listen;
6ef1c5326220cf Florian Westphal 2022-03-28  113  
6ef1c5326220cf Florian Westphal 2022-03-28  114  			if (r->id.idiag_sport != inet->inet_sport &&
6ef1c5326220cf Florian Westphal 2022-03-28  115  			    r->id.idiag_sport)
6ef1c5326220cf Florian Westphal 2022-03-28  116  				goto next_listen;
6ef1c5326220cf Florian Westphal 2022-03-28  117  
6ef1c5326220cf Florian Westphal 2022-03-28  118  			if (!refcount_inc_not_zero(&sk->sk_refcnt))
6ef1c5326220cf Florian Westphal 2022-03-28  119  				goto next_listen;
6ef1c5326220cf Florian Westphal 2022-03-28  120  
6ef1c5326220cf Florian Westphal 2022-03-28 @121  			ret = sk_diag_dump(sk, skb, cb, r, bc, net_admin);
6ef1c5326220cf Florian Westphal 2022-03-28 @122  					   sock_put(sk);

"bc" is never initialized and the sock_put() is indented too far.
Part of the commit is missing?

6ef1c5326220cf Florian Westphal 2022-03-28  123  			if (ret < 0) {
6ef1c5326220cf Florian Westphal 2022-03-28  124  				spin_unlock(&ilb->lock);
6ef1c5326220cf Florian Westphal 2022-03-28  125  				rcu_read_unlock();
6ef1c5326220cf Florian Westphal 2022-03-28  126  				diag_ctx->l_slot = i;
6ef1c5326220cf Florian Westphal 2022-03-28  127  				diag_ctx->l_num = num;
6ef1c5326220cf Florian Westphal 2022-03-28  128  				return;
6ef1c5326220cf Florian Westphal 2022-03-28  129  			}
6ef1c5326220cf Florian Westphal 2022-03-28  130  			diag_ctx->l_num = num + 1;
6ef1c5326220cf Florian Westphal 2022-03-28  131  			num = 0;
6ef1c5326220cf Florian Westphal 2022-03-28  132  next_listen:
6ef1c5326220cf Florian Westphal 2022-03-28  133  			++num;
6ef1c5326220cf Florian Westphal 2022-03-28  134  		}
6ef1c5326220cf Florian Westphal 2022-03-28  135  		spin_unlock(&ilb->lock);
6ef1c5326220cf Florian Westphal 2022-03-28  136  		rcu_read_unlock();
6ef1c5326220cf Florian Westphal 2022-03-28  137  
6ef1c5326220cf Florian Westphal 2022-03-28  138  		cond_resched();
6ef1c5326220cf Florian Westphal 2022-03-28  139  		diag_ctx->l_num = 0;
6ef1c5326220cf Florian Westphal 2022-03-28  140  	}
6ef1c5326220cf Florian Westphal 2022-03-28  141  
6ef1c5326220cf Florian Westphal 2022-03-28  142  	diag_ctx->l_num = 0;
6ef1c5326220cf Florian Westphal 2022-03-28  143  	diag_ctx->l_slot = i;
6ef1c5326220cf Florian Westphal 2022-03-28  144  }
diff mbox series

Patch

diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index c4992eeb67d8..076dd99d35d9 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -69,8 +69,80 @@  static int mptcp_diag_dump_one(struct netlink_callback *cb,
 struct mptcp_diag_ctx {
 	long s_slot;
 	long s_num;
+	unsigned int l_slot;
+	unsigned int l_num;
 };
 
+static void mptcp_diag_dump_listeners(struct sk_buff *skb, struct netlink_callback *cb,
+				      const struct inet_diag_req_v2 *r,
+				      bool net_admin)
+{
+	struct mptcp_diag_ctx *diag_ctx = (void *)cb->ctx;
+	struct net *net = sock_net(skb->sk);
+	struct nlattr *bc;
+	int i;
+
+	for (i = diag_ctx->l_slot; i < INET_LHTABLE_SIZE; i++) {
+		struct inet_listen_hashbucket *ilb;
+		struct hlist_nulls_node *node;
+		struct sock *sk;
+		int num = 0;
+
+		ilb = &tcp_hashinfo.listening_hash[i];
+
+		rcu_read_lock();
+		spin_lock(&ilb->lock);
+		sk_nulls_for_each(sk, node, &ilb->nulls_head) {
+			const struct mptcp_subflow_context *ctx = mptcp_subflow_ctx(sk);
+			struct inet_sock *inet = inet_sk(sk);
+			int ret;
+
+			if (num < diag_ctx->l_num)
+				goto next_listen;
+
+			if (!ctx || strcmp(inet_csk(sk)->icsk_ulp_ops->name, "mptcp"))
+				goto next_listen;
+
+			sk = ctx->conn;
+			if (!sk || !net_eq(sock_net(sk), net))
+				goto next_listen;
+
+			if (r->sdiag_family != AF_UNSPEC &&
+			    sk->sk_family != r->sdiag_family)
+				goto next_listen;
+
+			if (r->id.idiag_sport != inet->inet_sport &&
+			    r->id.idiag_sport)
+				goto next_listen;
+
+			if (!refcount_inc_not_zero(&sk->sk_refcnt))
+				goto next_listen;
+
+			ret = sk_diag_dump(sk, skb, cb, r, bc, net_admin);
+					   sock_put(sk);
+			if (ret < 0) {
+				spin_unlock(&ilb->lock);
+				rcu_read_unlock();
+				diag_ctx->l_slot = i;
+				diag_ctx->l_num = num;
+				return;
+			}
+			diag_ctx->l_num = num + 1;
+			num = 0;
+next_listen:
+			++num;
+		}
+		spin_unlock(&ilb->lock);
+		rcu_read_unlock();
+
+		cond_resched();
+		diag_ctx->l_num = 0;
+	}
+
+	diag_ctx->l_num = 0;
+	diag_ctx->l_slot = i;
+}
+
 static void mptcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 			    const struct inet_diag_req_v2 *r)
 {
@@ -114,6 +186,9 @@  static void mptcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 		}
 		cond_resched();
 	}
+
+	if ((r->idiag_states & TCPF_LISTEN) && r->id.idiag_dport == 0)
+		mptcp_diag_dump_listeners(skb, cb, r, net_admin);
 }
 
 static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
@@ -124,6 +199,19 @@  static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
 
 	r->idiag_rqueue = sk_rmem_alloc_get(sk);
 	r->idiag_wqueue = sk_wmem_alloc_get(sk);
+
+	if (inet_sk_state_load(sk) == TCP_LISTEN) {
+		struct sock *lsk = READ_ONCE(msk->first);
+
+		if (lsk) {
+			/* override with settings from tcp listener,
+			 * so Send-Q will show accept queue.
+			 */
+			r->idiag_rqueue = READ_ONCE(lsk->sk_ack_backlog);
+			r->idiag_wqueue = READ_ONCE(lsk->sk_max_ack_backlog);
+		}
+	}
+
 	if (!info)
 		return;