| Message ID | 20230927093013.1951659-2-liujian56@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | add BPF_F_PERMANENT flag for sockmap skmsg redirect | expand |
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@huawei.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. 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
On Wed, Sep 27, 2023 at 05:30 PM +08, 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@huawei.com> > --- I adapted a scripted benchmark for sk_msg redirect I had written recently [1] to double check these numbers. Looks good. The boost is reproducible. Sample test run captured at [2]. *** netns-to-netns TCP latency test *** sockperf: Summary: Message Rate is 87638 [msg/sec] sockperf: Summary: BandWidth is 123.027 MBps (984.216 Mbps) *** netns-to-netns TCP latency test WITH sockmap bypass *** sockperf: Summary: Message Rate is 135718 [msg/sec] sockperf: Summary: BandWidth is 190.522 MBps (1524.177 Mbps) *** netns-to-netns TCP latency test WITH sockmap bypass + F_PERMANENT *** sockperf: Summary: Message Rate is 148700 [msg/sec] sockperf: Summary: BandWidth is 208.746 MBps (1669.971 Mbps) And, as expected, I'm seeing just a different prog run count when using F_PERMANENT after the test: 175: sk_msg name sk_msg_prog tag 7c26e0d6e8e92a36 gpl run_time_ns 245761059 run_cnt 4071588 loaded_at 2023-10-13T14:27:28+0200 uid 0 xlated 80B jited 62B memlock 4096B map_ids 88,90 btf_id 173 177: sk_msg name sk_msg_prog_once tag e460e6fffdc8ff8a gpl run_time_ns 1441 run_cnt 1 loaded_at 2023-10-13T14:27:28+0200 uid 0 xlated 80B jited 62B memlock 4096B map_ids 88,90 btf_id 173 Feel free to add my: Tested-by: Jakub Sitnicki <jakub@cloudflare.com> [1] https://github.com/jsitnicki/srecon-2023-sockmap/blob/test-f_permanent/examples/redir-bypass/test_redir_bypass.sh [2] https://github.com/jsitnicki/srecon-2023-sockmap/blob/test-f_permanent/examples/redir-bypass/example_redir_bypass.txt
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@huawei.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 >
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. + * + * **BPF_F_PERMANENT** cannot be use together with + * **bpf_msg_apply_bytes**\ () and **bpf_msg_cork_bytes**\ (). If + * **BPF_F_PERMANENT** is set apply_bytes and cork_bytes are ignored. * Return * **SK_PASS** on success, or **SK_DROP** on error. * @@ -3301,11 +3313,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_SOCKHASH**) using hash *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_SOCKHASH**) using hash *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. + * + * **BPF_F_PERMANENT** cannot be use together with + * **bpf_msg_apply_bytes**\ () and **bpf_msg_cork_bytes**\ (). If + * **BPF_F_PERMANENT** is set apply_bytes and cork_bytes are ignored. * Return * **SK_PASS** on success, or **SK_DROP** on error. * @@ -5906,6 +5930,7 @@ enum { /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */ enum { BPF_F_INGRESS = (1ULL << 0), + BPF_F_PERMANENT = (1ULL << 1), }; /* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */ diff --git a/net/core/skmsg.c b/net/core/skmsg.c index 6c31eefbd777..22e55eec3089 100644 --- a/net/core/skmsg.c +++ b/net/core/skmsg.c @@ -878,7 +878,11 @@ int sk_psock_msg_verdict(struct sock *sk, struct sk_psock *psock, msg->sk = sk; ret = bpf_prog_run_pin_on_cpu(prog, msg); ret = sk_psock_map_verd(ret, msg->sk_redir); - psock->apply_bytes = msg->apply_bytes; + psock->redir_permanent = msg->flags & BPF_F_PERMANENT; + if (psock->redir_permanent) + msg->cork_bytes = msg->apply_bytes = 0; + else + psock->apply_bytes = msg->apply_bytes; if (ret == __SK_REDIRECT) { if (psock->sk_redir) { sock_put(psock->sk_redir); diff --git a/net/core/sock_map.c b/net/core/sock_map.c index cb11750b1df5..5b2539b04628 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -662,7 +662,7 @@ BPF_CALL_4(bpf_msg_redirect_map, struct sk_msg *, msg, { struct sock *sk; - if (unlikely(flags & ~(BPF_F_INGRESS))) + if (unlikely(flags & ~(BPF_F_INGRESS | BPF_F_PERMANENT))) return SK_DROP; sk = __sock_map_lookup_elem(map, key); @@ -1261,7 +1261,7 @@ BPF_CALL_4(bpf_msg_redirect_hash, struct sk_msg *, msg, { struct sock *sk; - if (unlikely(flags & ~(BPF_F_INGRESS))) + if (unlikely(flags & ~(BPF_F_INGRESS | BPF_F_PERMANENT))) return SK_DROP; sk = __sock_hash_lookup_elem(map, key); diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c index 81f0dff69e0b..bbbbafaa1929 100644 --- a/net/ipv4/tcp_bpf.c +++ b/net/ipv4/tcp_bpf.c @@ -419,8 +419,10 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock, if (!psock->apply_bytes) { /* Clean up before releasing the sock lock. */ eval = psock->eval; - psock->eval = __SK_NONE; - psock->sk_redir = NULL; + if (!psock->redir_permanent) { + psock->eval = __SK_NONE; + psock->sk_redir = NULL; + } } if (psock->cork) { cork = true; @@ -434,7 +436,7 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock, msg, tosend, flags); sent = origsize - msg->sg.size; - if (eval == __SK_REDIRECT) + if (!psock->redir_permanent && eval == __SK_REDIRECT) sock_put(sk_redir); lock_sock(sk); @@ -460,8 +462,8 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock, } if (likely(!ret)) { - if (!psock->apply_bytes) { - psock->eval = __SK_NONE; + if (!psock->apply_bytes && !psock->redir_permanent) { + psock->eval = __SK_NONE; if (psock->sk_redir) { sock_put(psock->sk_redir); psock->sk_redir = NULL; diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 70bfa997e896..cec6c34f4486 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/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. + * + * **BPF_F_PERMANENT** cannot be use together with + * **bpf_msg_apply_bytes**\ () and **bpf_msg_cork_bytes**\ (). If + * **BPF_F_PERMANENT** is set apply_bytes and cork_bytes are ignored. * Return * **SK_PASS** on success, or **SK_DROP** on error. * @@ -3301,11 +3313,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_SOCKHASH**) using hash *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_SOCKHASH**) using hash *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. + * + * **BPF_F_PERMANENT** cannot be use together with + * **bpf_msg_apply_bytes**\ () and **bpf_msg_cork_bytes**\ (). If + * **BPF_F_PERMANENT** is set apply_bytes and cork_bytes are ignored. * Return * **SK_PASS** on success, or **SK_DROP** on error. * @@ -5906,6 +5930,7 @@ enum { /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */ enum { BPF_F_INGRESS = (1ULL << 0), + BPF_F_PERMANENT = (1ULL << 1), }; /* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */
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@huawei.com> --- 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(-)