Re: [PATCH iproute2 v7 1/3] ss: add support for BPF socket-local storage

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 12 Feb 2024 16:43:29 +0100
Quentin Deslandes <qde@naccy.de> wrote:

> While sock_diag is able to return BPF socket-local storage in response
> to INET_DIAG_REQ_SK_BPF_STORAGES requests, ss doesn't request it.
> 
> This change introduces the --bpf-maps and --bpf-map-id= options to request
> BPF socket-local storage for all SK_STORAGE maps, or only specific ones.
> 
> The bigger part of this change will check the requested map IDs and
> ensure they are valid. The column COL_EXT is used to print the
> socket-local data into.
> 
> When --bpf-maps is used, ss will send an empty INET_DIAG_REQ_SK_BPF_STORAGES
> request, in return the kernel will send all the BPF socket-local storage
> entries for a given socket. The BTF data for each map is loaded on
> demand, as ss can't predict which map ID are used.
> 
> When --bpf-map-id=ID is used, a file descriptor to the requested maps is
> open to 1) ensure the map doesn't disappear before the data is printed,
> and 2) ensure the map type is BPF_MAP_TYPE_SK_STORAGE. The BTF data for
> each requested map is loaded before the request is sent to the kernel.
> 
> Signed-off-by: Quentin Deslandes <qde@naccy.de>
> Co-authored-by: Martin KaFai Lau <martin.lau@kernel.org>
> Acked-by: Martin KaFai Lau <martin.lau@kernel.org>
> ---
>  misc/ss.c | 262 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 259 insertions(+), 3 deletions(-)


Some checkpatch complaints here.

WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#123: 
When --bpf-maps is used, ss will send an empty INET_DIAG_REQ_SK_BPF_STORAGES

WARNING: Non-standard signature: Co-authored-by:
#134: 
Co-authored-by: Martin KaFai Lau <martin.lau@kernel.org>

WARNING: line length of 112 exceeds 100 columns
#189: FILE: misc/ss.c:3417:
+		fprintf(stderr, "ss: too many (> %u) BPF socket-local storage maps found, skipping map ID %u\n",

WARNING: Block comments use a trailing */ on a separate line
#286: FILE: misc/ss.c:3514:
+	 * a socket, no matter their map ID. */

WARNING: Block comments use a trailing */ on a separate line
#304: FILE: misc/ss.c:3532:
+	 * to avoid inserting specific map IDs into the request. */

total: 0 errors, 5 warnings, 344 lines checked