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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <ZB9UdB5pgOAacioS@pop-os.localdomain>
Date:   Sat, 25 Mar 2023 13:07:16 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Stanislav Fomichev <sdf@...gle.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 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.

> 
> 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.

Thanks.