netdev - Re: [PATCH bpf-next v5 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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ef4882c0-b570-37a4-de37-c2ad251458d1@huawei.com>
Date: Sat, 14 Oct 2023 20:05:11 +0800
From: "liujian (CE)" <liujian56@...wei.com>
To: John Fastabend <john.fastabend@...il.com>, <jakub@...udflare.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>
CC: <netdev@...r.kernel.org>, <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next v5 1/7] bpf, sockmap: add BPF_F_PERMANENT flag
 for skmsg redirect



On 2023/10/3 12:27, John Fastabend wrote:
> 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_PERMANENT flag to bpf_msg_redirect_map() and
>> bpf_msg_redirect_hash() to implement this ability.
>>
>> Then we can enable this function in the bpf program as follows:
>> bpf_msg_redirect_hash(xx, xx, xx, BPF_F_INGRESS | BPF_F_PERMANENT);
>>
>> Test results using netperf  TCP_STREAM mode:
>> for i in 1 64 128 512 1k 2k 32k 64k 100k 500k 1m;then
>> netperf -T 1,2 -t TCP_STREAM -H 127.0.0.1 -l 20 -- -m $i -s 100m,100m -S 100m,100m
>> done
>>
>> before:
>> 3.84 246.52 496.89 1885.03 3415.29 6375.03 40749.09 48764.40 51611.34 55678.26 55992.78
>> after:
>> 4.43 279.20 555.82 2080.79 3870.70 7105.44 41836.41 49709.75 51861.56 55211.00 54566.85
>>
>> Signed-off-by: Liu Jian <liujian56@...wei.com>
>> ---
> 
> First sorry for the delay I was thinking about this a bit. I decided it likely makes
> a lot of sense if you want to build an l7 load balancer where you just read some
> keys off an initial msg and then pin the rest of the connection to a specific
> backend or proxy socket, etc.
> 
>>   include/linux/skmsg.h          |  1 +
>>   include/uapi/linux/bpf.h       | 45 ++++++++++++++++++++++++++--------
>>   net/core/skmsg.c               |  6 ++++-
>>   net/core/sock_map.c            |  4 +--
>>   net/ipv4/tcp_bpf.c             | 12 +++++----
>>   tools/include/uapi/linux/bpf.h | 45 ++++++++++++++++++++++++++--------
>>   6 files changed, 85 insertions(+), 28 deletions(-)
>>
>> diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
>> index c1637515a8a4..acd7de85608b 100644
>> --- a/include/linux/skmsg.h
>> +++ b/include/linux/skmsg.h
>> @@ -83,6 +83,7 @@ struct sk_psock {
>>   	u32				cork_bytes;
>>   	u32				eval;
>>   	bool				redir_ingress; /* undefined if sk_redir is null */
>> +	bool				redir_permanent;
>>   	struct sk_msg			*cork;
>>   	struct sk_psock_progs		progs;
>>   #if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 70bfa997e896..cec6c34f4486 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -3029,11 +3029,23 @@ union bpf_attr {
>>    * 		socket level. If the message *msg* is allowed to pass (i.e. if
>>    * 		the verdict eBPF program returns **SK_PASS**), redirect it to
>>    * 		the socket referenced by *map* (of type
>> - * 		**BPF_MAP_TYPE_SOCKMAP**) at index *key*. Both ingress and
>> - * 		egress interfaces can be used for redirection. The
>> - * 		**BPF_F_INGRESS** value in *flags* is used to make the
>> - * 		distinction (ingress path is selected if the flag is present,
>> - * 		egress path otherwise). This is the only flag supported for now.
>> + * 		**BPF_MAP_TYPE_SOCKMAP**) at index *key*.
>> + *
>> + *		The following *flags* are supported:
>> + *
>> + *		**BPF_F_INGRESS**
>> + *		        Both ingress and egress interfaces can be used for redirection.
>> + *		        The **BPF_F_INGRESS** value in *flags* is used to make the
>> + *		        distinction. Ingress path is selected if the flag is present,
>> + *		        egress path otherwise.
>> + *		**BPF_F_PERMANENT**
>> + *		        Indicates that redirect verdict and the target socket should be
>> + *		        remembered. The verdict program will not be run for subsequent
>> + *		        packets, unless an error occurs when forwarding packets.
> 
> Why clear it on error? The error is almost always because either the socket is
> being torn down or there is a memory ENOMEM error that is going to be kicked
> back to user.
Sorry, yes, you're right, I've changed the code, but forgot to change 
the help document. Have sent next version to fix this. Thank you~.
> 
> Is the idea that you can try anopther backend possibly by picking another socket
> out of the table? But, I'm not sure how that might work because you probably
> don't know that this is even the beginning of a msg block.
> 
> I'm wondering if I missed some other reason or if its just simpler to pass the
> error up the stack and keep the same sk_redir.
> 
> Thanks,
> John
>