netdev - Re: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F

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: <87zg2l5zaa.fsf@cloudflare.com>
Date: Sun, 20 Aug 2023 20:19:49 +0200
From: Jakub Sitnicki <jakub@...udflare.com>
To: Ferenc Fejes <fejes@....elte.hu>, Liu Jian <liujian56@...wei.com>
Cc: john.fastabend@...il.com, ast@...nel.org, daniel@...earbox.net,
 andrii@...nel.org, martin.lau@...ux.dev, song@...nel.org,
 yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...gle.com,
 haoluo@...gle.com, jolsa@...nel.org, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 dsahern@...nel.org, netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY
 flag for skmsg redirect

On Thu, Aug 17, 2023 at 02:05 PM +02, Ferenc Fejes wrote:
> Hi Liu!
>
> On Fri, 2023-08-11 at 17:32 +0800, Liu Jian wrote:
>> If the sockmap msg redirection function is used only to forward
>> packets
>> and no other operation, the execution result of the
>> BPF_SK_MSG_VERDICT
>> program is the same each time. In this case, the BPF program only
>> needs to
>> be run once. Add BPF_F_PERMANENTLY flag to bpf_msg_redirect_map() and
>> bpf_msg_redirect_hash() to implement this ability.
>
> Did you considered other names for this flag e.g. BPF_F_SPLICED or
> BPF_F_PIPED?

Ferenc,

A reference to splice/pipe syscall certainly paints a picture.

But I'm not sure if it makes it more intutive or more confusing in the
context of bpf_{msg,sk}_redirect_{hash,map}. Consider:

  bpf_msg_redirect_map(..., BPF_F_SPLICE)

vs

  bpf_msg_redirect_map(..., BPF_F_PERMANENTLY)

Liu,

No need to go for the adverb form ("PERMANENTLY"). An adjective
("PERMANENT") will as expressive here. So BPF_F_PERMANENT is what I'm
suggesting.

Also, I'm thinking maybe it's time for a dedicated prefix to avoid name
clashes, like BPF_F_ADJ_ROOM_*.

BPF_F_INGRESS, which is also accepted by other helpers. But that won't
be the case with the new flag. BPF_F_SK_REDIR_*? That would make it
BPF_F_SK_REDIR_PERMANENT.

Alternatively, BPF_F_SK_REDIR_FIXED comes to mind. Naming is hard.

[...]