[PATCH mptcp-next v2 0/3] mptcp: inet diag listen dump support

[PATCH mptcp-next v2 0/3] mptcp: inet diag listen dump support

[Florian Westphal]

Changes in v2: drop patch 4 and prefer msk->first (Paolo Abeni)

Done in a not-very-elegant way:
Iterate over tcp listen hash, then pick out the tcp sockets
with mptcp-ctx structure attached, then take the conn->sk for dumping.

First patch is preparation/cleanup.
Second patch gets rid of locking to avoid a lockdep splat.
If the socket lock is really needed (I don't see where) I can workaround
this by dropping locks temporarily when iterating the listen hash table,
but its a bit more awkward.

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

Florian Westphal (3):
  mptcp: diag: switch to context structure
  mptcp: remove locking in mptcp_diag_fill_info
  mptcp: listen diag dump support

 net/mptcp/mptcp_diag.c | 102 +++++++++++++++++++++++++++++++++++++++--
 net/mptcp/sockopt.c    |   6 ---
 2 files changed, 99 insertions(+), 9 deletions(-)

-- 
2.35.1

[PATCH mptcp-next v2 1/3] mptcp: diag: switch to context structure

[Florian Westphal]

Raw access to cb->arg[] is deprecated, use a context structure.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/mptcp/mptcp_diag.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index f44125dd6697..c4992eeb67d8 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -66,20 +66,28 @@ static int mptcp_diag_dump_one(struct netlink_callback *cb,
 	return err;
 }
 
+struct mptcp_diag_ctx {
+	long s_slot;
+	long s_num;
+};
+
 static void mptcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 			    const struct inet_diag_req_v2 *r)
 {
 	bool net_admin = netlink_net_capable(cb->skb, CAP_NET_ADMIN);
+	struct mptcp_diag_ctx *diag_ctx = (void *)cb->ctx;
 	struct net *net = sock_net(skb->sk);
 	struct inet_diag_dump_data *cb_data;
 	struct mptcp_sock *msk;
 	struct nlattr *bc;
 
+	BUILD_BUG_ON(sizeof(cb->ctx) < sizeof(*diag_ctx));
+
 	cb_data = cb->data;
 	bc = cb_data->inet_diag_nla_bc;
 
-	while ((msk = mptcp_token_iter_next(net, &cb->args[0], &cb->args[1])) !=
-	       NULL) {
+	while ((msk = mptcp_token_iter_next(net, &diag_ctx->s_slot,
+					    &diag_ctx->s_num)) != NULL) {
 		struct inet_sock *inet = (struct inet_sock *)msk;
 		struct sock *sk = (struct sock *)msk;
 		int ret = 0;
@@ -101,7 +109,7 @@ static void mptcp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
 		sock_put(sk);
 		if (ret < 0) {
 			/* will retry on the same position */
-			cb->args[1]--;
+			diag_ctx->s_num--;
 			break;
 		}
 		cond_resched();
-- 
2.35.1

[PATCH mptcp-next v2 2/3] mptcp: remove locking in mptcp_diag_fill_info

[Florian Westphal]

Problem is that listener iteration would call this from atomic context
so this locking is not allowed.

One way is to drop locks before calling the helper, but afaics the lock
isn't really needed, all values are fetched via READ_ONCE().

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/mptcp/sockopt.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index f949d22f52bd..826b0c1dae98 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -853,15 +853,11 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
 
 void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 {
-	struct sock *sk = &msk->sk.icsk_inet.sk;
 	u32 flags = 0;
-	bool slow;
 	u8 val;
 
 	memset(info, 0, sizeof(*info));
 
-	slow = lock_sock_fast(sk);
-
 	info->mptcpi_subflows = READ_ONCE(msk->pm.subflows);
 	info->mptcpi_add_addr_signal = READ_ONCE(msk->pm.add_addr_signaled);
 	info->mptcpi_add_addr_accepted = READ_ONCE(msk->pm.add_addr_accepted);
@@ -882,8 +878,6 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 	info->mptcpi_snd_una = READ_ONCE(msk->snd_una);
 	info->mptcpi_rcv_nxt = READ_ONCE(msk->ack_seq);
 	info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
-
-	unlock_sock_fast(sk, slow);
 }
 EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
 
-- 
2.35.1

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

[Florian Westphal]

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);
+					   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;
 
-- 
2.35.1

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

[kernel test robot]

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	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6709 bytes --]

CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220328124913.29768-4-fw@strlen.de>
References: <20220328124913.29768-4-fw@strlen.de>
TO: Florian Westphal <fw@strlen.de>
TO: mptcp(a)lists.linux.dev
CC: Florian Westphal <fw@strlen.de>

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
:::::: branch date: 8 hours ago
:::::: commit date: 8 hours ago
config: i386-randconfig-m021-20220328 (https://download.01.org/0day-ci/archive/20220329/202203290401.iBK94uHx-lkp(a)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

fedd1232547507 Florian Westphal 2022-03-28   75  
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);
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  }
6ef1c5326220cf Florian Westphal 2022-03-28  145  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[Mat Martineau]

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

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

[Dan Carpenter]

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  }

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 6036 bytes --]

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(a)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  }

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp