[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZWi4nCeJTLsVt6J5@debian>
Date: Thu, 30 Nov 2023 17:30:20 +0100
From: Guillaume Nault <gnault@...hat.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
David Ahern <dsahern@...nel.org>,
Kuniyuki Iwashima <kuniyu@...zon.com>,
Michal Kubecek <mkubecek@...e.cz>
Subject: Re: [PATCH net-next v3] tcp: Dump bound-only sockets in inet_diag.
On Thu, Nov 30, 2023 at 05:17:57PM +0100, Eric Dumazet wrote:
> On Thu, Nov 30, 2023 at 4:40 PM Guillaume Nault <gnault@...hat.com> wrote:
> >
> > Walk the hashinfo->bhash2 table so that inet_diag can dump TCP sockets
> > that are bound but haven't yet called connect() or listen().
> >
> > The code is inspired by the ->lhash2 loop. However there's no manual
> > test of the source port, since this kind of filtering is already
> > handled by inet_diag_bc_sk(). Also, a maximum of 16 sockets are dumped
> > at a time, to avoid running with bh disabled for too long.
> >
> > There's no TCP state for bound but otherwise inactive sockets. Such
> > sockets normally map to TCP_CLOSE. However, "ss -l", which is supposed
> > to only dump listening sockets, actually requests the kernel to dump
> > sockets in either the TCP_LISTEN or TCP_CLOSE states. To avoid dumping
> > bound-only sockets with "ss -l", we therefore need to define a new
> > pseudo-state (TCP_BOUND_INACTIVE) that user space will be able to set
> > explicitly.
> >
> > With an IPv4, an IPv6 and an IPv6-only socket, bound respectively to
> > 40000, 64000, 60000, an updated version of iproute2 could work as
> > follow:
> >
> > $ ss -t state bound-inactive
> > Recv-Q Send-Q Local Address:Port Peer Address:Port Process
> > 0 0 0.0.0.0:40000 0.0.0.0:*
> > 0 0 [::]:60000 [::]:*
> > 0 0 *:64000 *:*
> >
> > Signed-off-by: Guillaume Nault <gnault@...hat.com>
> > ---
> >
> > v3:
> > * Grab sockets with sock_hold(), instead of refcount_inc_not_zero()
> > (Kuniyuki Iwashima).
> > * Use a new TCP pseudo-state (TCP_BOUND_INACTIVE), to dump bound-only
> > sockets, so that "ss -l" won't print them (Eric Dumazet).
> >
>
>
> > +pause_bind_walk:
> > + spin_unlock_bh(&ibb->lock);
> > +
> > + res = 0;
> > + for (idx = 0; idx < accum; idx++) {
> > + if (res >= 0) {
> > + res = inet_sk_diag_fill(sk_arr[idx],
> > + NULL, skb, cb,
> > + r, NLM_F_MULTI,
> > + net_admin);
> > + if (res < 0)
> > + num = num_arr[idx];
> > + }
> > + sock_gen_put(sk_arr[idx]);
>
> nit: this could be a mere sock_put(), because only full sockets are
> hashed in bhash2[]
Yes, makes sense.
I'll send a v4.
> Reviewed-by: Eric Dumazet <edumazet@...gle.com>
>
Powered by blists - more mailing lists