netdev - Re: [Patch net-next v3] sock_map: dump socket map id via diag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZCHFWr5rqnYNK3qS@google.com>
Date:   Mon, 27 Mar 2023 09:33:30 -0700
From:   Stanislav Fomichev <sdf@...gle.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org,
        Cong Wang <cong.wang@...edance.com>,
        John Fastabend <john.fastabend@...il.com>,
        Jakub Sitnicki <jakub@...udflare.com>
Subject: Re: [Patch net-next v3] sock_map: dump socket map id via diag

On 03/25, Cong Wang wrote:
> On Mon, Mar 20, 2023 at 11:13:03AM -0700, Stanislav Fomichev wrote:
> > On Sun, Mar 19, 2023 at 12:19 PM Cong Wang <xiyou.wangcong@...il.com>  
> wrote:
> > >
> > > From: Cong Wang <cong.wang@...edance.com>
> > >
> > > Currently there is no way to know which sockmap a socket has been  
> added
> > > to from outside, especially for that a socket can be added to multiple
> > > sockmap's. We could dump this via socket diag, as shown below.
> > >
> > > Sample output:
> > >
> > >   # ./iproute2/misc/ss -tnaie --bpf-map
> > >   ESTAB  0      344329     127.0.0.1:1234     127.0.0.1:40912  
> ino:21098 sk:5 cgroup:/user.slice/user-0.slice/session-c1.scope <->  
> sockmap: 1
> > >
> > >   # bpftool map
> > >   1: sockmap  flags 0x0
> > >         key 4B  value 4B  max_entries 2  memlock 4096B
> > >         pids echo-sockmap(549)
> > >   4: array  name pid_iter.rodata  flags 0x480
> > >         key 4B  value 4B  max_entries 1  memlock 4096B
> > >         btf_id 10  frozen
> > >         pids bpftool(624)
> > >
> > > In the future, we could dump other sockmap related stats too, hence I
> > > make it a nested attribute.
> > >
> > > Cc: John Fastabend <john.fastabend@...il.com>
> > > Cc: Jakub Sitnicki <jakub@...udflare.com>
> > > Signed-off-by: Cong Wang <cong.wang@...edance.com>
> >
> > Looks fine from my POW, will let others comment.
> >
> > One thing I still don't understand here: what is missing from the
> > socket iterators to implement this? Is it all the sk_psock_get magic?
> > I remember you dismissed Yonghong's suggestion on v1, but have you
> > actually tried it?

> I am very confused. So in order to figure out which sockmap a socket has
> been added to, I have to dump *all* sockmap's??? It seems you are
> suggesting to solve this with a more complex and unnecessary approach?
> Please tell me why, I am really lost, I don't even see there is a point
> to make here.

With a socket iter, you can iterate over all sockets and run some bpf
program on it do dump some state. So you'd iterate over the sockets,
not sockmaps. For each socket you get a pointer to sock and you do the same
sk_psock_get+list_for_each_entry(psock->link).

(in theory; would be interesting to see whether it works in practice)

> >
> > Also: a test would be nice to have. I know you've tested it with the
> > iproute2, but having something regularly exercised by the ci seems
> > good to have (and not a ton of work).

> Sure, so where are the tests for socket diag? I don't see any within the
> tree:

> $ git grep INET_DIAG_SOCKOPT -- tools/
> $

> Note, this is not suitable for bpf selftests, because it is less relevant
> to bpf, much more relevant to socket diag. I thought this is obvious.

Never too late to start testing those sock diag paths :-)
Put them in the net selftests if you don't think bpf selftests is the right
fit?

> Thanks.