* [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for skmsg redirect
2023-08-11 9:32 [PATCH bpf-next v2 0/7] add BPF_F_PERMANENTLY flag for sockmap skmsg redirect Liu Jian
@ 2023-08-11 9:32 ` Liu Jian
2023-08-17 6:13 ` John Fastabend
` (2 more replies)
2023-08-11 9:32 ` [PATCH bpf-next v2 2/7] selftests/bpf: Add txmsg ingress permanently test for sockmap Liu Jian
` (5 subsequent siblings)
6 siblings, 3 replies; 15+ messages in thread
From: Liu Jian @ 2023-08-11 9:32 UTC (permalink / raw)
To: john.fastabend, jakub, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern
Cc: netdev, bpf, liujian56
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.
Then we can enable this function in the bpf program as follows:
bpf_msg_redirect_hash(xx, xx, xx, BPF_F_INGRESS | BPF_F_PERMANENTLY);
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 | 7 +++++--
net/core/skmsg.c | 1 +
net/core/sock_map.c | 4 ++--
net/ipv4/tcp_bpf.c | 21 +++++++++++++++------
tools/include/uapi/linux/bpf.h | 7 +++++--
6 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 054d7911bfc9..b2da9c432f52 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -82,6 +82,7 @@ struct sk_psock {
u32 cork_bytes;
u32 eval;
bool redir_ingress; /* undefined if sk_redir is null */
+ bool eval_permanently;
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 70da85200695..cf622ea4f018 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3004,7 +3004,8 @@ union bpf_attr {
* 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.
+ * egress path otherwise). The **BPF_F_PERMANENTLY** value in
+ * *flags* is used to indicates whether the eBPF result is permanent.
* Return
* **SK_PASS** on success, or **SK_DROP** on error.
*
@@ -3276,7 +3277,8 @@ union bpf_attr {
* 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.
+ * egress path otherwise). The **BPF_F_PERMANENTLY** value in
+ * *flags* is used to indicates whether the eBPF result is permanent.
* Return
* **SK_PASS** on success, or **SK_DROP** on error.
*
@@ -5872,6 +5874,7 @@ enum {
/* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
enum {
BPF_F_INGRESS = (1ULL << 0),
+ BPF_F_PERMANENTLY = (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 a29508e1ff35..b2bf9b5c4252 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -875,6 +875,7 @@ int sk_psock_msg_verdict(struct sock *sk, struct sk_psock *psock,
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->eval_permanently = msg->flags & BPF_F_PERMANENTLY;
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 08ab108206bf..6a0c90be7f4f 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_PERMANENTLY)))
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_PERMANENTLY)))
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..36cf2b0fa6f8 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->eval_permanently) {
+ psock->eval = __SK_NONE;
+ psock->sk_redir = NULL;
+ }
}
if (psock->cork) {
cork = true;
@@ -433,9 +435,15 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
ret = tcp_bpf_sendmsg_redir(sk_redir, redir_ingress,
msg, tosend, flags);
sent = origsize - msg->sg.size;
+ /* disable the ability when something wrong */
+ if (unlikely(ret < 0))
+ psock->eval_permanently = 0;
- if (eval == __SK_REDIRECT)
+ if (!psock->eval_permanently && eval == __SK_REDIRECT) {
sock_put(sk_redir);
+ psock->sk_redir = NULL;
+ psock->eval = __SK_NONE;
+ }
lock_sock(sk);
if (unlikely(ret < 0)) {
@@ -460,8 +468,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->eval_permanently) {
+ psock->eval = __SK_NONE;
if (psock->sk_redir) {
sock_put(psock->sk_redir);
psock->sk_redir = NULL;
@@ -540,7 +548,8 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
if (psock->cork_bytes && !enospc)
goto out_err;
/* All cork bytes are accounted, rerun the prog. */
- psock->eval = __SK_NONE;
+ if (!psock->eval_permanently)
+ psock->eval = __SK_NONE;
psock->cork_bytes = 0;
}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 70da85200695..cf622ea4f018 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3004,7 +3004,8 @@ union bpf_attr {
* 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.
+ * egress path otherwise). The **BPF_F_PERMANENTLY** value in
+ * *flags* is used to indicates whether the eBPF result is permanent.
* Return
* **SK_PASS** on success, or **SK_DROP** on error.
*
@@ -3276,7 +3277,8 @@ union bpf_attr {
* 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.
+ * egress path otherwise). The **BPF_F_PERMANENTLY** value in
+ * *flags* is used to indicates whether the eBPF result is permanent.
* Return
* **SK_PASS** on success, or **SK_DROP** on error.
*
@@ -5872,6 +5874,7 @@ enum {
/* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
enum {
BPF_F_INGRESS = (1ULL << 0),
+ BPF_F_PERMANENTLY = (1ULL << 1),
};
/* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* RE: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for skmsg redirect
2023-08-11 9:32 ` [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for " Liu Jian
@ 2023-08-17 6:13 ` John Fastabend
2023-08-19 9:25 ` liujian (CE)
2023-08-20 18:03 ` Jakub Sitnicki
2023-08-17 12:05 ` Ferenc Fejes
2023-08-21 7:40 ` Jakub Sitnicki
2 siblings, 2 replies; 15+ messages in thread
From: John Fastabend @ 2023-08-17 6:13 UTC (permalink / raw)
To: Liu Jian, john.fastabend, jakub, ast, daniel, andrii, martin.lau,
song, yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet,
kuba, pabeni, dsahern
Cc: netdev, bpf, liujian56
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.
>
I like the use case. Did you consider using
long bpf_msg_apply_bytes(struct sk_msg_buff *msg, u32 bytes)
This could be set to UINT32_MAX and then the BPF prog would only be run
every 0xfffffff bytes.
> Then we can enable this function in the bpf program as follows:
> bpf_msg_redirect_hash(xx, xx, xx, BPF_F_INGRESS | BPF_F_PERMANENTLY);
>
> 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
I suspect comparing against
bpf_msg_redirect_hash(...)
bpf_msg_apply_bytes(msg, UINT32_MAX)
the diff will be rather small. I agree the API is nicer though to simply
set the flag. Its too bad we didn't think to add a forever to apply_bytes.
I would prefer this API for example,
bpf_msg_redirect_hash(...)
bpf_msg_apply_bytes(msg, 0, PERMANENT);
Given we have apply_bytes is it still useful to have a PERMANENT flag
in your use case? Here we would just reset to UNINT32_MAX if we reached
max bytes.
>
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---
> include/linux/skmsg.h | 1 +
> include/uapi/linux/bpf.h | 7 +++++--
> net/core/skmsg.c | 1 +
> net/core/sock_map.c | 4 ++--
> net/ipv4/tcp_bpf.c | 21 +++++++++++++++------
> tools/include/uapi/linux/bpf.h | 7 +++++--
> 6 files changed, 29 insertions(+), 12 deletions(-)
[...]
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 70da85200695..cf622ea4f018 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -3004,7 +3004,8 @@ union bpf_attr {
> * 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.
> + * egress path otherwise). The **BPF_F_PERMANENTLY** value in
> + * *flags* is used to indicates whether the eBPF result is permanent.
We at least need to document what happens if PERMANENTLY and apply_bytes are
used together.
> * Return
> * **SK_PASS** on success, or **SK_DROP** on error.
> *
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for skmsg redirect
2023-08-17 6:13 ` John Fastabend
@ 2023-08-19 9:25 ` liujian (CE)
2023-08-20 18:03 ` Jakub Sitnicki
1 sibling, 0 replies; 15+ messages in thread
From: liujian (CE) @ 2023-08-19 9:25 UTC (permalink / raw)
To: John Fastabend, jakub, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern
Cc: netdev, bpf
On 2023/8/17 14:13, 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_PERMANENTLY flag to bpf_msg_redirect_map() and
>> bpf_msg_redirect_hash() to implement this ability.
>>
>
> I like the use case. Did you consider using
>
> long bpf_msg_apply_bytes(struct sk_msg_buff *msg, u32 bytes)
>
> This could be set to UINT32_MAX and then the BPF prog would only be run
> every 0xfffffff bytes.
>
I didn't realize that this feature could be used for this, and I thought
it should have the same effect. Thanks John.
>> Then we can enable this function in the bpf program as follows:
>> bpf_msg_redirect_hash(xx, xx, xx, BPF_F_INGRESS | BPF_F_PERMANENTLY);
>>
>> 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
>
> I suspect comparing against
>
> bpf_msg_redirect_hash(...)
> bpf_msg_apply_bytes(msg, UINT32_MAX)
>
> the diff will be rather small. I agree the API is nicer though to simply
Yes, it should have the same effect and looks good to me.
> set the flag. Its too bad we didn't think to add a forever to apply_bytes.
> I would prefer this API for example,
>
> bpf_msg_redirect_hash(...)
> bpf_msg_apply_bytes(msg, 0, PERMANENT);
>
What do you mean by this? Should I post another version for this?
> Given we have apply_bytes is it still useful to have a PERMANENT flag
> in your use case? Here we would just reset to UNINT32_MAX if we reached
> max bytes.
>
If apply_bytes is set to UNINT32_MAX, the number of times that the bpf
program runs should be small enough to meet my needs.
>>
>> Signed-off-by: Liu Jian <liujian56@huawei.com>
>> ---
>> include/linux/skmsg.h | 1 +
>> include/uapi/linux/bpf.h | 7 +++++--
>> net/core/skmsg.c | 1 +
>> net/core/sock_map.c | 4 ++--
>> net/ipv4/tcp_bpf.c | 21 +++++++++++++++------
>> tools/include/uapi/linux/bpf.h | 7 +++++--
>> 6 files changed, 29 insertions(+), 12 deletions(-)
>
> [...]
>
>>
>> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
>> index 70da85200695..cf622ea4f018 100644
>> --- a/tools/include/uapi/linux/bpf.h
>> +++ b/tools/include/uapi/linux/bpf.h
>> @@ -3004,7 +3004,8 @@ union bpf_attr {
>> * 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.
>> + * egress path otherwise). The **BPF_F_PERMANENTLY** value in
>> + * *flags* is used to indicates whether the eBPF result is permanent.
>
> We at least need to document what happens if PERMANENTLY and apply_bytes are
> used together.
>
>> * Return
>> * **SK_PASS** on success, or **SK_DROP** on error.
>> *
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for skmsg redirect
2023-08-17 6:13 ` John Fastabend
2023-08-19 9:25 ` liujian (CE)
@ 2023-08-20 18:03 ` Jakub Sitnicki
1 sibling, 0 replies; 15+ messages in thread
From: Jakub Sitnicki @ 2023-08-20 18:03 UTC (permalink / raw)
To: John Fastabend
Cc: ast, daniel, andrii, martin.lau, song, yonghong.song, kpsingh,
sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni, dsahern,
netdev, bpf, liujian56
On Wed, Aug 16, 2023 at 11:13 PM -07, 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_PERMANENTLY flag to bpf_msg_redirect_map() and
>> bpf_msg_redirect_hash() to implement this ability.
>>
>
> I like the use case. Did you consider using
>
> long bpf_msg_apply_bytes(struct sk_msg_buff *msg, u32 bytes)
>
> This could be set to UINT32_MAX and then the BPF prog would only be run
> every 0xfffffff bytes.
It would be great to have the permanent redirect feature implemented
also for BPF_SK_SKB_STREAM_VERDICT and BPF_SK_SKB_VERDICT. I don't think
there are any obstacles to support it for both input configurations.
But in SK_SKB verdict prog we don't have apply_bytes. So we couldn't
keep the API the same without introducing a helper.
That's why I'd go with the flag.
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for skmsg redirect
2023-08-11 9:32 ` [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for " Liu Jian
2023-08-17 6:13 ` John Fastabend
@ 2023-08-17 12:05 ` Ferenc Fejes
2023-08-19 9:32 ` liujian (CE)
2023-08-20 18:19 ` Jakub Sitnicki
2023-08-21 7:40 ` Jakub Sitnicki
2 siblings, 2 replies; 15+ messages in thread
From: Ferenc Fejes @ 2023-08-17 12:05 UTC (permalink / raw)
To: Liu Jian, john.fastabend, jakub, ast, daniel, andrii, martin.lau,
song, yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet,
kuba, pabeni, dsahern
Cc: netdev, bpf
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?
BTW good addition, makes sense for the skb case too.
>
> Then we can enable this function in the bpf program as follows:
> bpf_msg_redirect_hash(xx, xx, xx, BPF_F_INGRESS | BPF_F_PERMANENTLY);
>
> 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 | 7 +++++--
> net/core/skmsg.c | 1 +
> net/core/sock_map.c | 4 ++--
> net/ipv4/tcp_bpf.c | 21 +++++++++++++++------
> tools/include/uapi/linux/bpf.h | 7 +++++--
> 6 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
> index 054d7911bfc9..b2da9c432f52 100644
> --- a/include/linux/skmsg.h
> +++ b/include/linux/skmsg.h
> @@ -82,6 +82,7 @@ struct sk_psock {
> u32 cork_bytes;
> u32 eval;
> bool redir_ingress; /* undefined
> if sk_redir is null */
> + bool eval_permanently;
> 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 70da85200695..cf622ea4f018 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3004,7 +3004,8 @@ union bpf_attr {
> * 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.
> + * egress path otherwise). The **BPF_F_PERMANENTLY**
> value in
> + * *flags* is used to indicates whether the eBPF result
> is permanent.
> * Return
> * **SK_PASS** on success, or **SK_DROP** on error.
> *
> @@ -3276,7 +3277,8 @@ union bpf_attr {
> * 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.
> + * egress path otherwise). The **BPF_F_PERMANENTLY**
> value in
> + * *flags* is used to indicates whether the eBPF result
> is permanent.
> * Return
> * **SK_PASS** on success, or **SK_DROP** on error.
> *
> @@ -5872,6 +5874,7 @@ enum {
> /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
> enum {
> BPF_F_INGRESS = (1ULL << 0),
> + BPF_F_PERMANENTLY = (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 a29508e1ff35..b2bf9b5c4252 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -875,6 +875,7 @@ int sk_psock_msg_verdict(struct sock *sk, struct
> sk_psock *psock,
> 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->eval_permanently = msg->flags & BPF_F_PERMANENTLY;
> 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 08ab108206bf..6a0c90be7f4f 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_PERMANENTLY)))
> 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_PERMANENTLY)))
> 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..36cf2b0fa6f8 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->eval_permanently) {
> + psock->eval = __SK_NONE;
> + psock->sk_redir = NULL;
> + }
> }
> if (psock->cork) {
> cork = true;
> @@ -433,9 +435,15 @@ static int tcp_bpf_send_verdict(struct sock *sk,
> struct sk_psock *psock,
> ret = tcp_bpf_sendmsg_redir(sk_redir, redir_ingress,
> msg, tosend, flags);
> sent = origsize - msg->sg.size;
> + /* disable the ability when something wrong */
> + if (unlikely(ret < 0))
> + psock->eval_permanently = 0;
>
> - if (eval == __SK_REDIRECT)
> + if (!psock->eval_permanently && eval ==
> __SK_REDIRECT) {
> sock_put(sk_redir);
> + psock->sk_redir = NULL;
> + psock->eval = __SK_NONE;
> + }
>
> lock_sock(sk);
> if (unlikely(ret < 0)) {
> @@ -460,8 +468,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->eval_permanently)
> {
> + psock->eval = __SK_NONE;
> if (psock->sk_redir) {
> sock_put(psock->sk_redir);
> psock->sk_redir = NULL;
> @@ -540,7 +548,8 @@ static int tcp_bpf_sendmsg(struct sock *sk,
> struct msghdr *msg, size_t size)
> if (psock->cork_bytes && !enospc)
> goto out_err;
> /* All cork bytes are accounted, rerun the
> prog. */
> - psock->eval = __SK_NONE;
> + if (!psock->eval_permanently)
> + psock->eval = __SK_NONE;
> psock->cork_bytes = 0;
> }
>
> diff --git a/tools/include/uapi/linux/bpf.h
> b/tools/include/uapi/linux/bpf.h
> index 70da85200695..cf622ea4f018 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -3004,7 +3004,8 @@ union bpf_attr {
> * 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.
> + * egress path otherwise). The **BPF_F_PERMANENTLY**
> value in
> + * *flags* is used to indicates whether the eBPF result
> is permanent.
> * Return
> * **SK_PASS** on success, or **SK_DROP** on error.
> *
> @@ -3276,7 +3277,8 @@ union bpf_attr {
> * 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.
> + * egress path otherwise). The **BPF_F_PERMANENTLY**
> value in
> + * *flags* is used to indicates whether the eBPF result
> is permanent.
> * Return
> * **SK_PASS** on success, or **SK_DROP** on error.
> *
> @@ -5872,6 +5874,7 @@ enum {
> /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
> enum {
> BPF_F_INGRESS = (1ULL << 0),
> + BPF_F_PERMANENTLY = (1ULL << 1),
> };
>
> /* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key
> flags. */
Ferenc
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for skmsg redirect
2023-08-17 12:05 ` Ferenc Fejes
@ 2023-08-19 9:32 ` liujian (CE)
2023-08-20 18:19 ` Jakub Sitnicki
1 sibling, 0 replies; 15+ messages in thread
From: liujian (CE) @ 2023-08-19 9:32 UTC (permalink / raw)
To: Ferenc Fejes, john.fastabend, jakub, ast, daniel, andrii,
martin.lau, song, yonghong.song, kpsingh, sdf, haoluo, jolsa,
davem, edumazet, kuba, pabeni, dsahern
Cc: netdev, bpf
On 2023/8/17 20:05, 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?
>
Yes, it's all ok for me.
> BTW good addition, makes sense for the skb case too.
>
Yes, I had planned to modify bpf_sk_redirect_map/hash() if this patch
can be incorporated into the mainline. However, John proposed an
existing solution for this patch, and this patch should not be needed.
I'll post the changes to bpf_sk_redirect_map/hash() separately later?
Hi, John, do you have any suggestions?
>>
>> Then we can enable this function in the bpf program as follows:
>> bpf_msg_redirect_hash(xx, xx, xx, BPF_F_INGRESS | BPF_F_PERMANENTLY);
>>
>> 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 | 7 +++++--
>> net/core/skmsg.c | 1 +
>> net/core/sock_map.c | 4 ++--
>> net/ipv4/tcp_bpf.c | 21 +++++++++++++++------
>> tools/include/uapi/linux/bpf.h | 7 +++++--
>> 6 files changed, 29 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
>> index 054d7911bfc9..b2da9c432f52 100644
>> --- a/include/linux/skmsg.h
>> +++ b/include/linux/skmsg.h
>> @@ -82,6 +82,7 @@ struct sk_psock {
>> u32 cork_bytes;
>> u32 eval;
>> bool redir_ingress; /* undefined
>> if sk_redir is null */
>> + bool eval_permanently;
>> 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 70da85200695..cf622ea4f018 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -3004,7 +3004,8 @@ union bpf_attr {
>> * 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.
>> + * egress path otherwise). The **BPF_F_PERMANENTLY**
>> value in
>> + * *flags* is used to indicates whether the eBPF result
>> is permanent.
>> * Return
>> * **SK_PASS** on success, or **SK_DROP** on error.
>> *
>> @@ -3276,7 +3277,8 @@ union bpf_attr {
>> * 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.
>> + * egress path otherwise). The **BPF_F_PERMANENTLY**
>> value in
>> + * *flags* is used to indicates whether the eBPF result
>> is permanent.
>> * Return
>> * **SK_PASS** on success, or **SK_DROP** on error.
>> *
>> @@ -5872,6 +5874,7 @@ enum {
>> /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
>> enum {
>> BPF_F_INGRESS = (1ULL << 0),
>> + BPF_F_PERMANENTLY = (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 a29508e1ff35..b2bf9b5c4252 100644
>> --- a/net/core/skmsg.c
>> +++ b/net/core/skmsg.c
>> @@ -875,6 +875,7 @@ int sk_psock_msg_verdict(struct sock *sk, struct
>> sk_psock *psock,
>> 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->eval_permanently = msg->flags & BPF_F_PERMANENTLY;
>> 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 08ab108206bf..6a0c90be7f4f 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_PERMANENTLY)))
>> 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_PERMANENTLY)))
>> 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..36cf2b0fa6f8 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->eval_permanently) {
>> + psock->eval = __SK_NONE;
>> + psock->sk_redir = NULL;
>> + }
>> }
>> if (psock->cork) {
>> cork = true;
>> @@ -433,9 +435,15 @@ static int tcp_bpf_send_verdict(struct sock *sk,
>> struct sk_psock *psock,
>> ret = tcp_bpf_sendmsg_redir(sk_redir, redir_ingress,
>> msg, tosend, flags);
>> sent = origsize - msg->sg.size;
>> + /* disable the ability when something wrong */
>> + if (unlikely(ret < 0))
>> + psock->eval_permanently = 0;
>>
>> - if (eval == __SK_REDIRECT)
>> + if (!psock->eval_permanently && eval ==
>> __SK_REDIRECT) {
>> sock_put(sk_redir);
>> + psock->sk_redir = NULL;
>> + psock->eval = __SK_NONE;
>> + }
>>
>> lock_sock(sk);
>> if (unlikely(ret < 0)) {
>> @@ -460,8 +468,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->eval_permanently)
>> {
>> + psock->eval = __SK_NONE;
>> if (psock->sk_redir) {
>> sock_put(psock->sk_redir);
>> psock->sk_redir = NULL;
>> @@ -540,7 +548,8 @@ static int tcp_bpf_sendmsg(struct sock *sk,
>> struct msghdr *msg, size_t size)
>> if (psock->cork_bytes && !enospc)
>> goto out_err;
>> /* All cork bytes are accounted, rerun the
>> prog. */
>> - psock->eval = __SK_NONE;
>> + if (!psock->eval_permanently)
>> + psock->eval = __SK_NONE;
>> psock->cork_bytes = 0;
>> }
>>
>> diff --git a/tools/include/uapi/linux/bpf.h
>> b/tools/include/uapi/linux/bpf.h
>> index 70da85200695..cf622ea4f018 100644
>> --- a/tools/include/uapi/linux/bpf.h
>> +++ b/tools/include/uapi/linux/bpf.h
>> @@ -3004,7 +3004,8 @@ union bpf_attr {
>> * 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.
>> + * egress path otherwise). The **BPF_F_PERMANENTLY**
>> value in
>> + * *flags* is used to indicates whether the eBPF result
>> is permanent.
>> * Return
>> * **SK_PASS** on success, or **SK_DROP** on error.
>> *
>> @@ -3276,7 +3277,8 @@ union bpf_attr {
>> * 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.
>> + * egress path otherwise). The **BPF_F_PERMANENTLY**
>> value in
>> + * *flags* is used to indicates whether the eBPF result
>> is permanent.
>> * Return
>> * **SK_PASS** on success, or **SK_DROP** on error.
>> *
>> @@ -5872,6 +5874,7 @@ enum {
>> /* BPF_FUNC_clone_redirect and BPF_FUNC_redirect flags. */
>> enum {
>> BPF_F_INGRESS = (1ULL << 0),
>> + BPF_F_PERMANENTLY = (1ULL << 1),
>> };
>>
>> /* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key
>> flags. */
>
> Ferenc
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for skmsg redirect
2023-08-17 12:05 ` Ferenc Fejes
2023-08-19 9:32 ` liujian (CE)
@ 2023-08-20 18:19 ` Jakub Sitnicki
1 sibling, 0 replies; 15+ messages in thread
From: Jakub Sitnicki @ 2023-08-20 18:19 UTC (permalink / raw)
To: Ferenc Fejes, Liu Jian
Cc: john.fastabend, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern, netdev, bpf
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.
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for skmsg redirect
2023-08-11 9:32 ` [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for " Liu Jian
2023-08-17 6:13 ` John Fastabend
2023-08-17 12:05 ` Ferenc Fejes
@ 2023-08-21 7:40 ` Jakub Sitnicki
2 siblings, 0 replies; 15+ messages in thread
From: Jakub Sitnicki @ 2023-08-21 7:40 UTC (permalink / raw)
To: Liu Jian
Cc: john.fastabend, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern, netdev, bpf
On Fri, Aug 11, 2023 at 05:32 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_PERMANENTLY 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_PERMANENTLY);
>
> 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 | 7 +++++--
> net/core/skmsg.c | 1 +
> net/core/sock_map.c | 4 ++--
> net/ipv4/tcp_bpf.c | 21 +++++++++++++++------
> tools/include/uapi/linux/bpf.h | 7 +++++--
> 6 files changed, 29 insertions(+), 12 deletions(-)
>
[...]
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index 81f0dff69e0b..36cf2b0fa6f8 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->eval_permanently) {
> + psock->eval = __SK_NONE;
> + psock->sk_redir = NULL;
> + }
> }
> if (psock->cork) {
> cork = true;
> @@ -433,9 +435,15 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
> ret = tcp_bpf_sendmsg_redir(sk_redir, redir_ingress,
> msg, tosend, flags);
> sent = origsize - msg->sg.size;
> + /* disable the ability when something wrong */
> + if (unlikely(ret < 0))
> + psock->eval_permanently = 0;
>
> - if (eval == __SK_REDIRECT)
> + if (!psock->eval_permanently && eval == __SK_REDIRECT) {
> sock_put(sk_redir);
> + psock->sk_redir = NULL;
> + psock->eval = __SK_NONE;
> + }
>
> lock_sock(sk);
> if (unlikely(ret < 0)) {
Looking at the above changes, I'm wondering - have you considered
introducing a dedicated a __sk_action for this? Like
__SK_REDIRECT_PERMANENT?
Just a gut feeling. Maybe it would make the code easier to ready if we
don't have to have another flag remember about.
Also, eval_permenently is not a great name, IMHO, because eval can be
also PASS or NONE, to which this flag does not apply. If the flag needs
to stay, it could be named something like redir_permanent so it's
obvious that it applies just to REDIRECT action.
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH bpf-next v2 2/7] selftests/bpf: Add txmsg ingress permanently test for sockmap
2023-08-11 9:32 [PATCH bpf-next v2 0/7] add BPF_F_PERMANENTLY flag for sockmap skmsg redirect Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for " Liu Jian
@ 2023-08-11 9:32 ` Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 3/7] selftests/bpf: Add txmsg redir " Liu Jian
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Liu Jian @ 2023-08-11 9:32 UTC (permalink / raw)
To: john.fastabend, jakub, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern
Cc: netdev, bpf, liujian56
Add one test for txmsg ingress permanently test for sockmap.
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
tools/testing/selftests/bpf/test_sockmap.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 024a0faafb3b..8fb49586f8bb 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -77,6 +77,7 @@ int txmsg_end_push;
int txmsg_start_pop;
int txmsg_pop;
int txmsg_ingress;
+int txmsg_permanently;
int txmsg_redir_skb;
int txmsg_ktls_skb;
int txmsg_ktls_skb_drop;
@@ -107,6 +108,7 @@ static const struct option long_options[] = {
{"txmsg_start_pop", required_argument, NULL, 'w'},
{"txmsg_pop", required_argument, NULL, 'x'},
{"txmsg_ingress", no_argument, &txmsg_ingress, 1 },
+ {"txmsg_permanently", no_argument, &txmsg_permanently, 1 },
{"txmsg_redir_skb", no_argument, &txmsg_redir_skb, 1 },
{"ktls", no_argument, &ktls, 1 },
{"peek", no_argument, &peek_flag, 1 },
@@ -175,7 +177,7 @@ static void test_reset(void)
txmsg_start_push = txmsg_end_push = 0;
txmsg_pass = txmsg_drop = txmsg_redir = 0;
txmsg_apply = txmsg_cork = 0;
- txmsg_ingress = txmsg_redir_skb = 0;
+ txmsg_ingress = txmsg_permanently = txmsg_redir_skb = 0;
txmsg_ktls_skb = txmsg_ktls_skb_drop = txmsg_ktls_skb_redir = 0;
txmsg_omit_skb_parser = 0;
skb_use_parser = 0;
@@ -1167,6 +1169,9 @@ static int run_options(struct sockmap_options *options, int cg_fd, int test)
if (txmsg_ingress) {
int in = BPF_F_INGRESS;
+ if (txmsg_permanently)
+ in |= BPF_F_PERMANENTLY;
+
i = 0;
err = bpf_map_update_elem(map_fd[6], &i, &in, BPF_ANY);
if (err) {
@@ -1506,6 +1511,14 @@ static void test_txmsg_ingress_redir(int cgrp, struct sockmap_options *opt)
test_send(opt, cgrp);
}
+static void test_txmsg_ingress_redir_permanently(int cgrp, struct sockmap_options *opt)
+{
+ txmsg_pass = txmsg_drop = 0;
+ txmsg_ingress = txmsg_redir = 1;
+ txmsg_permanently = 1;
+ test_send(opt, cgrp);
+}
+
static void test_txmsg_skb(int cgrp, struct sockmap_options *opt)
{
bool data = opt->data_test;
@@ -1862,6 +1875,7 @@ struct _test test[] = {
{"txmsg test redirect wait send mem", test_txmsg_redir_wait_sndmem},
{"txmsg test drop", test_txmsg_drop},
{"txmsg test ingress redirect", test_txmsg_ingress_redir},
+ {"txmsg test ingress redirect permanently", test_txmsg_ingress_redir_permanently},
{"txmsg test skb", test_txmsg_skb},
{"txmsg test apply", test_txmsg_apply},
{"txmsg test cork", test_txmsg_cork},
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v2 3/7] selftests/bpf: Add txmsg redir permanently test for sockmap
2023-08-11 9:32 [PATCH bpf-next v2 0/7] add BPF_F_PERMANENTLY flag for sockmap skmsg redirect Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 1/7] bpf, sockmap: add BPF_F_PERMANENTLY flag for " Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 2/7] selftests/bpf: Add txmsg ingress permanently test for sockmap Liu Jian
@ 2023-08-11 9:32 ` Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 4/7] selftests/bpf: add skmsg verdict tests Liu Jian
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Liu Jian @ 2023-08-11 9:32 UTC (permalink / raw)
To: john.fastabend, jakub, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern
Cc: netdev, bpf, liujian56
Add one test for txmsg redir permanently test for sockmap.
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
.../selftests/bpf/progs/test_sockmap_kern.h | 4 ++-
tools/testing/selftests/bpf/test_sockmap.c | 27 ++++++++++++++++---
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_kern.h b/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
index 99d2ea9fb658..a9b2cb5e831b 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
@@ -298,9 +298,11 @@ int bpf_prog6(struct sk_msg_md *msg)
f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
if (f && *f) {
- key = 2;
flags = *f;
+ if (flags & BPF_F_INGRESS)
+ key = 2;
}
+ bpf_printk("flags is 0x%x, key is :%d\n", flags, key);
#ifdef SOCKMAP
return bpf_msg_redirect_map(msg, &sock_map_redir, key, flags);
#else
diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index 8fb49586f8bb..91347c9c4f93 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -1166,14 +1166,27 @@ static int run_options(struct sockmap_options *options, int cg_fd, int test)
}
+ if (txmsg_permanently) {
+ int txmsg_flag = BPF_F_PERMANENTLY;
+
+ i = 0;
+ err = bpf_map_update_elem(map_fd[6], &i, &txmsg_flag, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (txmsg_permanently): %d (%s)\n",
+ err, strerror(errno));
+ goto out;
+ }
+ }
+
if (txmsg_ingress) {
- int in = BPF_F_INGRESS;
+ int txmsg_flag = BPF_F_INGRESS;
if (txmsg_permanently)
- in |= BPF_F_PERMANENTLY;
+ txmsg_flag |= BPF_F_PERMANENTLY;
i = 0;
- err = bpf_map_update_elem(map_fd[6], &i, &in, BPF_ANY);
+ err = bpf_map_update_elem(map_fd[6], &i, &txmsg_flag, BPF_ANY);
if (err) {
fprintf(stderr,
"ERROR: bpf_map_update_elem (txmsg_ingress): %d (%s)\n",
@@ -1490,6 +1503,13 @@ static void test_txmsg_redir(int cgrp, struct sockmap_options *opt)
test_send(opt, cgrp);
}
+static void test_txmsg_redir_permanently(int cgrp, struct sockmap_options *opt)
+{
+ txmsg_redir = 1;
+ txmsg_permanently = 1;
+ test_send(opt, cgrp);
+}
+
static void test_txmsg_redir_wait_sndmem(int cgrp, struct sockmap_options *opt)
{
txmsg_redir = 1;
@@ -1872,6 +1892,7 @@ static int populate_progs(char *bpf_file)
struct _test test[] = {
{"txmsg test passthrough", test_txmsg_pass},
{"txmsg test redirect", test_txmsg_redir},
+ {"txmsg test redirect permanently", test_txmsg_redir_permanently},
{"txmsg test redirect wait send mem", test_txmsg_redir_wait_sndmem},
{"txmsg test drop", test_txmsg_drop},
{"txmsg test ingress redirect", test_txmsg_ingress_redir},
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v2 4/7] selftests/bpf: add skmsg verdict tests
2023-08-11 9:32 [PATCH bpf-next v2 0/7] add BPF_F_PERMANENTLY flag for sockmap skmsg redirect Liu Jian
` (2 preceding siblings ...)
2023-08-11 9:32 ` [PATCH bpf-next v2 3/7] selftests/bpf: Add txmsg redir " Liu Jian
@ 2023-08-11 9:32 ` Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 5/7] selftests/bpf: add two skmsg verdict tests for BPF_F_PERMANENTLY flag Liu Jian
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Liu Jian @ 2023-08-11 9:32 UTC (permalink / raw)
To: john.fastabend, jakub, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern
Cc: netdev, bpf, liujian56
Add two normal skmsg verdict tests in sockmap_basic.c
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 71 +++++++++++++++++++
.../bpf/progs/test_sockmap_msg_verdict.c | 25 +++++++
2 files changed, 96 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 064cc5e8d9ad..93bf1907abd9 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -12,6 +12,7 @@
#include "test_sockmap_progs_query.skel.h"
#include "test_sockmap_pass_prog.skel.h"
#include "test_sockmap_drop_prog.skel.h"
+#include "test_sockmap_msg_verdict.skel.h"
#include "bpf_iter_sockmap.skel.h"
#include "sockmap_helpers.h"
@@ -475,6 +476,72 @@ static void test_sockmap_skb_verdict_fionread(bool pass_prog)
test_sockmap_drop_prog__destroy(drop);
}
+static void test_sockmap_msg_verdict(bool is_ingress)
+{
+ int key, sent, recvd, recv_fd;
+ int err, map, verdict, s, c0, c1, p0, p1;
+ struct test_sockmap_msg_verdict *skel;
+ char buf[256] = "0123456789";
+
+ skel = test_sockmap_msg_verdict__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+ verdict = bpf_program__fd(skel->progs.prog_skmsg_verdict);
+ map = bpf_map__fd(skel->maps.sock_map);
+
+
+ err = bpf_prog_attach(verdict, map, BPF_SK_MSG_VERDICT, 0);
+ if (!ASSERT_OK(err, "bpf_prog_attach"))
+ goto out;
+
+ s = socket_loopback(AF_INET, SOCK_STREAM);
+ if (!ASSERT_GT(s, -1, "socket_loopback(s)"))
+ goto out;
+ err = create_socket_pairs(s, AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1);
+ if (!ASSERT_OK(err, "create_socket_pairs(s)"))
+ goto out;
+
+ key = 0;
+ err = bpf_map_update_elem(map, &key, &p1, BPF_NOEXIST);
+ if (!ASSERT_OK(err, "bpf_map_update_elem(key0)"))
+ goto out_close;
+ key = 1;
+ err = bpf_map_update_elem(map, &key, &c1, BPF_NOEXIST);
+ if (!ASSERT_OK(err, "bpf_map_update_elem(key1)"))
+ goto out_close;
+ key = 2;
+ err = bpf_map_update_elem(map, &key, &p0, BPF_NOEXIST);
+ if (!ASSERT_OK(err, "bpf_map_update_elem(key2)"))
+ goto out_close;
+ key = 3;
+ err = bpf_map_update_elem(map, &key, &c0, BPF_NOEXIST);
+ if (!ASSERT_OK(err, "bpf_map_update_elem(key3)"))
+ goto out_close;
+
+ if (is_ingress) {
+ recv_fd = c1;
+ skel->bss->skmsg_redir_flags = BPF_F_INGRESS;
+ skel->bss->skmsg_redir_key = 1;
+ } else {
+ recv_fd = c0;
+ skel->bss->skmsg_redir_flags = 0;
+ skel->bss->skmsg_redir_key = 2;
+ }
+
+ sent = xsend(p1, &buf, sizeof(buf), 0);
+ ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+ recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
+ ASSERT_EQ(recvd, sizeof(buf), "recv_timeout(recv_fd)");
+
+out_close:
+ close(c0);
+ close(p0);
+ close(c1);
+ close(p1);
+out:
+ test_sockmap_msg_verdict__destroy(skel);
+}
+
void test_sockmap_basic(void)
{
if (test__start_subtest("sockmap create_update_free"))
@@ -515,4 +582,8 @@ void test_sockmap_basic(void)
test_sockmap_skb_verdict_fionread(true);
if (test__start_subtest("sockmap skb_verdict fionread on drop"))
test_sockmap_skb_verdict_fionread(false);
+ if (test__start_subtest("sockmap msg_verdict"))
+ test_sockmap_msg_verdict(false);
+ if (test__start_subtest("sockmap msg_verdict ingress"))
+ test_sockmap_msg_verdict(true);
}
diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c b/tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c
new file mode 100644
index 000000000000..002b76a1ae35
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_msg_verdict.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+struct {
+ __uint(type, BPF_MAP_TYPE_SOCKMAP);
+ __uint(max_entries, 4);
+ __type(key, int);
+ __type(value, int);
+} sock_map SEC(".maps");
+
+u64 skmsg_redir_flags = 0;
+u32 skmsg_redir_key = 0;
+
+SEC("sk_msg")
+int prog_skmsg_verdict(struct sk_msg_md *msg)
+{
+ u64 flags = skmsg_redir_flags;
+ int key = skmsg_redir_key;
+
+ bpf_msg_redirect_map(msg, &sock_map, key, flags);
+ return SK_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v2 5/7] selftests/bpf: add two skmsg verdict tests for BPF_F_PERMANENTLY flag
2023-08-11 9:32 [PATCH bpf-next v2 0/7] add BPF_F_PERMANENTLY flag for sockmap skmsg redirect Liu Jian
` (3 preceding siblings ...)
2023-08-11 9:32 ` [PATCH bpf-next v2 4/7] selftests/bpf: add skmsg verdict tests Liu Jian
@ 2023-08-11 9:32 ` Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 6/7] selftests/bpf: add tests for verdict skmsg to itself Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 7/7] selftests/bpf: add tests for verdict skmsg to closed socket Liu Jian
6 siblings, 0 replies; 15+ messages in thread
From: Liu Jian @ 2023-08-11 9:32 UTC (permalink / raw)
To: john.fastabend, jakub, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern
Cc: netdev, bpf, liujian56
Add two tests for BPF_F_PERMANENTLY flag in sockmap_basic.c.
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 93bf1907abd9..52bbf2825fe7 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -476,7 +476,7 @@ static void test_sockmap_skb_verdict_fionread(bool pass_prog)
test_sockmap_drop_prog__destroy(drop);
}
-static void test_sockmap_msg_verdict(bool is_ingress)
+static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanently)
{
int key, sent, recvd, recv_fd;
int err, map, verdict, s, c0, c1, p0, p1;
@@ -528,11 +528,18 @@ static void test_sockmap_msg_verdict(bool is_ingress)
skel->bss->skmsg_redir_key = 2;
}
+ if (is_permanently)
+ skel->bss->skmsg_redir_flags |= BPF_F_PERMANENTLY;
+
sent = xsend(p1, &buf, sizeof(buf), 0);
ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
ASSERT_EQ(recvd, sizeof(buf), "recv_timeout(recv_fd)");
+ sent = xsend(p1, &buf, sizeof(buf), 0);
+ ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+ recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
+ ASSERT_EQ(recvd, sizeof(buf), "recv_timeout(recv_fd)");
out_close:
close(c0);
close(p0);
@@ -583,7 +590,11 @@ void test_sockmap_basic(void)
if (test__start_subtest("sockmap skb_verdict fionread on drop"))
test_sockmap_skb_verdict_fionread(false);
if (test__start_subtest("sockmap msg_verdict"))
- test_sockmap_msg_verdict(false);
+ test_sockmap_msg_verdict(false, false);
if (test__start_subtest("sockmap msg_verdict ingress"))
- test_sockmap_msg_verdict(true);
+ test_sockmap_msg_verdict(true, false);
+ if (test__start_subtest("sockmap msg_verdict permanently"))
+ test_sockmap_msg_verdict(false, true);
+ if (test__start_subtest("sockmap msg_verdict ingress permanently"))
+ test_sockmap_msg_verdict(true, true);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v2 6/7] selftests/bpf: add tests for verdict skmsg to itself
2023-08-11 9:32 [PATCH bpf-next v2 0/7] add BPF_F_PERMANENTLY flag for sockmap skmsg redirect Liu Jian
` (4 preceding siblings ...)
2023-08-11 9:32 ` [PATCH bpf-next v2 5/7] selftests/bpf: add two skmsg verdict tests for BPF_F_PERMANENTLY flag Liu Jian
@ 2023-08-11 9:32 ` Liu Jian
2023-08-11 9:32 ` [PATCH bpf-next v2 7/7] selftests/bpf: add tests for verdict skmsg to closed socket Liu Jian
6 siblings, 0 replies; 15+ messages in thread
From: Liu Jian @ 2023-08-11 9:32 UTC (permalink / raw)
To: john.fastabend, jakub, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern
Cc: netdev, bpf, liujian56
Add tests for verdict skmsg to itself in sockmap_basic.c
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 32 +++++++++++++------
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 52bbf2825fe7..37b0c05b77ff 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -476,7 +476,7 @@ static void test_sockmap_skb_verdict_fionread(bool pass_prog)
test_sockmap_drop_prog__destroy(drop);
}
-static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanently)
+static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanently, bool is_self)
{
int key, sent, recvd, recv_fd;
int err, map, verdict, s, c0, c1, p0, p1;
@@ -519,13 +519,23 @@ static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanently)
goto out_close;
if (is_ingress) {
- recv_fd = c1;
skel->bss->skmsg_redir_flags = BPF_F_INGRESS;
- skel->bss->skmsg_redir_key = 1;
+ if (is_self) {
+ skel->bss->skmsg_redir_key = 0;
+ recv_fd = p1;
+ } else {
+ skel->bss->skmsg_redir_key = 1;
+ recv_fd = c1;
+ }
} else {
- recv_fd = c0;
skel->bss->skmsg_redir_flags = 0;
- skel->bss->skmsg_redir_key = 2;
+ if (is_self) {
+ skel->bss->skmsg_redir_key = 0;
+ recv_fd = c1;
+ } else {
+ skel->bss->skmsg_redir_key = 2;
+ recv_fd = c0;
+ }
}
if (is_permanently)
@@ -590,11 +600,15 @@ void test_sockmap_basic(void)
if (test__start_subtest("sockmap skb_verdict fionread on drop"))
test_sockmap_skb_verdict_fionread(false);
if (test__start_subtest("sockmap msg_verdict"))
- test_sockmap_msg_verdict(false, false);
+ test_sockmap_msg_verdict(false, false, false);
if (test__start_subtest("sockmap msg_verdict ingress"))
- test_sockmap_msg_verdict(true, false);
+ test_sockmap_msg_verdict(true, false, false);
if (test__start_subtest("sockmap msg_verdict permanently"))
- test_sockmap_msg_verdict(false, true);
+ test_sockmap_msg_verdict(false, true, false);
if (test__start_subtest("sockmap msg_verdict ingress permanently"))
- test_sockmap_msg_verdict(true, true);
+ test_sockmap_msg_verdict(true, true, false);
+ if (test__start_subtest("sockmap msg_verdict permanently self"))
+ test_sockmap_msg_verdict(false, true, true);
+ if (test__start_subtest("sockmap msg_verdict ingress permanently self"))
+ test_sockmap_msg_verdict(true, true, true);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH bpf-next v2 7/7] selftests/bpf: add tests for verdict skmsg to closed socket
2023-08-11 9:32 [PATCH bpf-next v2 0/7] add BPF_F_PERMANENTLY flag for sockmap skmsg redirect Liu Jian
` (5 preceding siblings ...)
2023-08-11 9:32 ` [PATCH bpf-next v2 6/7] selftests/bpf: add tests for verdict skmsg to itself Liu Jian
@ 2023-08-11 9:32 ` Liu Jian
6 siblings, 0 replies; 15+ messages in thread
From: Liu Jian @ 2023-08-11 9:32 UTC (permalink / raw)
To: john.fastabend, jakub, ast, daniel, andrii, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba,
pabeni, dsahern
Cc: netdev, bpf, liujian56
Add four tests for verdict skmsg to closed socket in sockmap_basic.c.
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 42 +++++++++++++++----
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 37b0c05b77ff..689e904cc11e 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -476,9 +476,10 @@ static void test_sockmap_skb_verdict_fionread(bool pass_prog)
test_sockmap_drop_prog__destroy(drop);
}
-static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanently, bool is_self)
+static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanently, bool is_self,
+ bool target_shutdown)
{
- int key, sent, recvd, recv_fd;
+ int key, sent, recvd, recv_fd, target_fd;
int err, map, verdict, s, c0, c1, p0, p1;
struct test_sockmap_msg_verdict *skel;
char buf[256] = "0123456789";
@@ -522,18 +523,22 @@ static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanently, bool
skel->bss->skmsg_redir_flags = BPF_F_INGRESS;
if (is_self) {
skel->bss->skmsg_redir_key = 0;
+ target_fd = p1;
recv_fd = p1;
} else {
skel->bss->skmsg_redir_key = 1;
+ target_fd = c1;
recv_fd = c1;
}
} else {
skel->bss->skmsg_redir_flags = 0;
if (is_self) {
skel->bss->skmsg_redir_key = 0;
+ target_fd = p1;
recv_fd = c1;
} else {
skel->bss->skmsg_redir_key = 2;
+ target_fd = p0;
recv_fd = c0;
}
}
@@ -546,6 +551,19 @@ static void test_sockmap_msg_verdict(bool is_ingress, bool is_permanently, bool
recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
ASSERT_EQ(recvd, sizeof(buf), "recv_timeout(recv_fd)");
+ if (target_shutdown) {
+ signal(SIGPIPE, SIG_IGN);
+ close(target_fd);
+ sent = send(p1, &buf, sizeof(buf), 0);
+ if (is_permanently) {
+ ASSERT_EQ(sent, -1, "xsend(p1)");
+ ASSERT_EQ(errno, EPIPE, "xsend(p1)");
+ } else {
+ ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+ }
+ goto out_close;
+ }
+
sent = xsend(p1, &buf, sizeof(buf), 0);
ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
recvd = recv_timeout(recv_fd, &buf, sizeof(buf), SOCK_NONBLOCK, IO_TIMEOUT_SEC);
@@ -600,15 +618,23 @@ void test_sockmap_basic(void)
if (test__start_subtest("sockmap skb_verdict fionread on drop"))
test_sockmap_skb_verdict_fionread(false);
if (test__start_subtest("sockmap msg_verdict"))
- test_sockmap_msg_verdict(false, false, false);
+ test_sockmap_msg_verdict(false, false, false, false);
if (test__start_subtest("sockmap msg_verdict ingress"))
- test_sockmap_msg_verdict(true, false, false);
+ test_sockmap_msg_verdict(true, false, false, false);
if (test__start_subtest("sockmap msg_verdict permanently"))
- test_sockmap_msg_verdict(false, true, false);
+ test_sockmap_msg_verdict(false, true, false, false);
if (test__start_subtest("sockmap msg_verdict ingress permanently"))
- test_sockmap_msg_verdict(true, true, false);
+ test_sockmap_msg_verdict(true, true, false, false);
if (test__start_subtest("sockmap msg_verdict permanently self"))
- test_sockmap_msg_verdict(false, true, true);
+ test_sockmap_msg_verdict(false, true, true, false);
if (test__start_subtest("sockmap msg_verdict ingress permanently self"))
- test_sockmap_msg_verdict(true, true, true);
+ test_sockmap_msg_verdict(true, true, true, false);
+ if (test__start_subtest("sockmap msg_verdict permanently shutdown"))
+ test_sockmap_msg_verdict(false, true, false, true);
+ if (test__start_subtest("sockmap msg_verdict ingress permanently shutdown"))
+ test_sockmap_msg_verdict(true, true, false, true);
+ if (test__start_subtest("sockmap msg_verdict shutdown"))
+ test_sockmap_msg_verdict(false, false, false, true);
+ if (test__start_subtest("sockmap msg_verdict ingress shutdown"))
+ test_sockmap_msg_verdict(true, false, false, true);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread