mbox series

[0/3] ss: pretty-printing BPF socket-local storage

Message ID 20231128023058.53546-1-qde@naccy.de (mailing list archive)
Headers show
Series ss: pretty-printing BPF socket-local storage | expand

Message

Quentin Deslandes Nov. 28, 2023, 2:30 a.m. UTC
BPF allows programs to store socket-specific data using
BPF_MAP_TYPE_SK_STORAGE maps. The data is attached to the socket itself,
and Martin added INET_DIAG_REQ_SK_BPF_STORAGES, so it can be fetched
using the INET_DIAG mechanism.

Currently, ss doesn't request the socket-local data, this patch aims to
fix this.

The first patch fixes a bug where the "Process" column would always be
printed on ss' output, even if --processes/-p is not used.

Patch #2 requests the socket-local data for the requested map ID
(--bpf-map-id=) or all the maps (--bpf-maps). It then prints the map_id
in a dedicated column.

Patch #3 uses libbpf and BTF to pretty print the map's content, like
`bpftool map dump` would do.

While I think it makes sense for ss to provide the socket-local storage
content for the sockets, it's difficult to conciliate the column-based
output of ss and having readable socket-local data. Hence, the
socket-local data is printed in a readable fashion over multiple lines
under its socket statistics, independently of the column-based approach.

Here is an example of ss' output with --bpf-maps:
[...]
ESTAB                  2960280             0 [...]
    map_id: 259 [
        (struct my_sk_storage) {
            .field_hh = (char)127,
            .<anon> = (union <anon>) {
                .a = (int)0,
                .b = (int)0,
            },
        },
    ]

Quentin Deslandes (3):
  ss: prevent "Process" column from being printed unless requested
  ss: add support for BPF socket-local storage
  ss: pretty-print BPF socket-local storage

 misc/ss.c | 822 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 818 insertions(+), 4 deletions(-)

Comments

Stephen Hemminger Nov. 28, 2023, 10:43 p.m. UTC | #1
On Mon, 27 Nov 2023 18:30:55 -0800
Quentin Deslandes <qde@naccy.de> wrote:

> BPF allows programs to store socket-specific data using
> BPF_MAP_TYPE_SK_STORAGE maps. The data is attached to the socket itself,
> and Martin added INET_DIAG_REQ_SK_BPF_STORAGES, so it can be fetched
> using the INET_DIAG mechanism.
> 
> Currently, ss doesn't request the socket-local data, this patch aims to
> fix this.
> 
> The first patch fixes a bug where the "Process" column would always be
> printed on ss' output, even if --processes/-p is not used.
> 
> Patch #2 requests the socket-local data for the requested map ID
> (--bpf-map-id=) or all the maps (--bpf-maps). It then prints the map_id
> in a dedicated column.
> 
> Patch #3 uses libbpf and BTF to pretty print the map's content, like
> `bpftool map dump` would do.
> 
> While I think it makes sense for ss to provide the socket-local storage
> content for the sockets, it's difficult to conciliate the column-based
> output of ss and having readable socket-local data. Hence, the
> socket-local data is printed in a readable fashion over multiple lines
> under its socket statistics, independently of the column-based approach.
> 
> Here is an example of ss' output with --bpf-maps:
> [...]
> ESTAB                  2960280             0 [...]
>     map_id: 259 [
>         (struct my_sk_storage) {
>             .field_hh = (char)127,
>             .<anon> = (union <anon>) {
>                 .a = (int)0,
>                 .b = (int)0,
>             },
>         },
>     ]
> 
> Quentin Deslandes (3):
>   ss: prevent "Process" column from being printed unless requested
>   ss: add support for BPF socket-local storage
>   ss: pretty-print BPF socket-local storage
> 
>  misc/ss.c | 822 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 818 insertions(+), 4 deletions(-)


Useful, but ss is growing into a huge monolithic program and may need some
refactoring. Also, this cries out for a json output format. Which ss doesn't
have yet.
Quentin Deslandes Dec. 8, 2023, 3:01 p.m. UTC | #2
On 2023-11-28 23:43, Stephen Hemminger wrote:
> On Mon, 27 Nov 2023 18:30:55 -0800
> Quentin Deslandes <qde@naccy.de> wrote:
> 
>> BPF allows programs to store socket-specific data using
>> BPF_MAP_TYPE_SK_STORAGE maps. The data is attached to the socket itself,
>> and Martin added INET_DIAG_REQ_SK_BPF_STORAGES, so it can be fetched
>> using the INET_DIAG mechanism.
>>
>> Currently, ss doesn't request the socket-local data, this patch aims to
>> fix this.
>>
>> The first patch fixes a bug where the "Process" column would always be
>> printed on ss' output, even if --processes/-p is not used.
>>
>> Patch #2 requests the socket-local data for the requested map ID
>> (--bpf-map-id=) or all the maps (--bpf-maps). It then prints the map_id
>> in a dedicated column.
>>
>> Patch #3 uses libbpf and BTF to pretty print the map's content, like
>> `bpftool map dump` would do.
>>
>> While I think it makes sense for ss to provide the socket-local storage
>> content for the sockets, it's difficult to conciliate the column-based
>> output of ss and having readable socket-local data. Hence, the
>> socket-local data is printed in a readable fashion over multiple lines
>> under its socket statistics, independently of the column-based approach.
>>
>> Here is an example of ss' output with --bpf-maps:
>> [...]
>> ESTAB                  2960280             0 [...]
>>     map_id: 259 [
>>         (struct my_sk_storage) {
>>             .field_hh = (char)127,
>>             .<anon> = (union <anon>) {
>>                 .a = (int)0,
>>                 .b = (int)0,
>>             },
>>         },
>>     ]
>>
>> Quentin Deslandes (3):
>>   ss: prevent "Process" column from being printed unless requested
>>   ss: add support for BPF socket-local storage
>>   ss: pretty-print BPF socket-local storage
>>
>>  misc/ss.c | 822 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 818 insertions(+), 4 deletions(-)
> 
> 
> Useful, but ss is growing into a huge monolithic program and may need some
> refactoring. Also, this cries out for a json output format. Which ss doesn't
> have yet.

I've submitted a v2 to fix Martin's comments and also improve the printing
behavior. The updated revision reduces the number of lines added by 50%.

Regarding the JSON output, is it specifically for socket-local storage, or
more generally, for the whole tool? I agree with you anyway, but I would argue
that it doesn't fit this series, although I can work on this as a next step.
Stephen Hemminger Dec. 8, 2023, 5:27 p.m. UTC | #3
On Fri, 8 Dec 2023 16:01:56 +0100
Quentin Deslandes <qde@naccy.de> wrote:

> I've submitted a v2 to fix Martin's comments and also improve the printing
> behavior. The updated revision reduces the number of lines added by 50%.

Not sure about what best format for this is.

> 
> Regarding the JSON output, is it specifically for socket-local storage, or
> more generally, for the whole tool? I agree with you anyway, but I would argue
> that it doesn't fit this series, although I can work on this as a next step.

It is more for the whole tool in future.