* [PATCH] bpf, sockmap: Do not read sk_receive_queue in tcp_bpf_recvmsg if strparser enabled
@ 2021-10-15 8:01 Liu Jian
2021-10-18 8:54 ` Jakub Sitnicki
0 siblings, 1 reply; 4+ messages in thread
From: Liu Jian @ 2021-10-15 8:01 UTC (permalink / raw)
To: john.fastabend, daniel, jakub, lmb, edumazet, davem, kuba,
yoshfuji, dsahern, ast, andrii, kafai, songliubraving, yhs,
kpsingh, netdev, bpf
Cc: liujian56
If the strparser function of sk is turned on, all received data needs to
be processed by strparser first.
Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
include/linux/skmsg.h | 6 ++++++
net/core/skmsg.c | 5 +++++
net/ipv4/tcp_bpf.c | 9 ++++++---
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 94e2a1f6e58d..25e92dff04aa 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -390,6 +390,7 @@ void sk_psock_stop(struct sk_psock *psock, bool wait);
int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock);
void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock);
void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock);
+bool sk_psock_strparser_started(struct sock *sk);
#else
static inline int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock)
{
@@ -403,6 +404,11 @@ static inline void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock)
static inline void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
{
}
+
+static inline bool sk_psock_strparser_started(struct sock *sk)
+{
+ return false;
+}
#endif
void sk_psock_start_verdict(struct sock *sk, struct sk_psock *psock);
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index e85b7f8491b9..dd64ef854f3e 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -1105,6 +1105,11 @@ void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock)
sk->sk_write_space = sk_psock_write_space;
}
+bool sk_psock_strparser_started(struct sock *sk)
+{
+ return sk->sk_data_ready == sk_psock_strp_data_ready;
+}
+
void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
{
if (!psock->saved_data_ready)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 9d068153c316..17129b343966 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -198,6 +198,7 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
if (unlikely(!psock))
return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
if (!skb_queue_empty(&sk->sk_receive_queue) &&
+ !sk_psock_strparser_started(sk) &&
sk_psock_queue_empty(psock)) {
sk_psock_put(sk, psock);
return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
@@ -214,9 +215,11 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
if (data) {
if (!sk_psock_queue_empty(psock))
goto msg_bytes_ready;
- release_sock(sk);
- sk_psock_put(sk, psock);
- return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+ if (!sk_psock_strparser_started(sk)) {
+ release_sock(sk);
+ sk_psock_put(sk, psock);
+ return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+ }
}
copied = -EAGAIN;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] bpf, sockmap: Do not read sk_receive_queue in tcp_bpf_recvmsg if strparser enabled
2021-10-15 8:01 [PATCH] bpf, sockmap: Do not read sk_receive_queue in tcp_bpf_recvmsg if strparser enabled Liu Jian
@ 2021-10-18 8:54 ` Jakub Sitnicki
2021-10-18 13:53 ` John Fastabend
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Sitnicki @ 2021-10-18 8:54 UTC (permalink / raw)
To: Liu Jian
Cc: john.fastabend, daniel, lmb, edumazet, davem, kuba, yoshfuji,
dsahern, ast, andrii, kafai, songliubraving, yhs, kpsingh,
netdev, bpf
On Fri, Oct 15, 2021 at 10:01 AM CEST, Liu Jian wrote:
> If the strparser function of sk is turned on, all received data needs to
> be processed by strparser first.
>
> Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---
[...]
> net/core/skmsg.c | 5 +++++
> net/ipv4/tcp_bpf.c | 9 ++++++---
> 3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
> index 94e2a1f6e58d..25e92dff04aa 100644
> --- a/include/linux/skmsg.h
> +++ b/include/linux/skmsg.h
> @@ -390,6 +390,7 @@ void sk_psock_stop(struct sk_psock *psock, bool wait);
> int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock);
> void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock);
> void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock);
> +bool sk_psock_strparser_started(struct sock *sk);
> #else
> static inline int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock)
> {
> @@ -403,6 +404,11 @@ static inline void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock)
> static inline void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
> {
> }
> +
> +static inline bool sk_psock_strparser_started(struct sock *sk)
> +{
> + return false;
> +}
> #endif
>
> void sk_psock_start_verdict(struct sock *sk, struct sk_psock *psock);
> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index e85b7f8491b9..dd64ef854f3e 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -1105,6 +1105,11 @@ void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock)
> sk->sk_write_space = sk_psock_write_space;
> }
>
> +bool sk_psock_strparser_started(struct sock *sk)
> +{
> + return sk->sk_data_ready == sk_psock_strp_data_ready;
What if kTLS is configured on the socket? I think this check won't work then.
> +}
> +
> void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
> {
> if (!psock->saved_data_ready)
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bpf, sockmap: Do not read sk_receive_queue in tcp_bpf_recvmsg if strparser enabled
2021-10-18 8:54 ` Jakub Sitnicki
@ 2021-10-18 13:53 ` John Fastabend
2021-10-19 3:03 ` liujian (CE)
0 siblings, 1 reply; 4+ messages in thread
From: John Fastabend @ 2021-10-18 13:53 UTC (permalink / raw)
To: Jakub Sitnicki, Liu Jian
Cc: john.fastabend, daniel, lmb, edumazet, davem, kuba, yoshfuji,
dsahern, ast, andrii, kafai, songliubraving, yhs, kpsingh,
netdev, bpf
Jakub Sitnicki wrote:
> On Fri, Oct 15, 2021 at 10:01 AM CEST, Liu Jian wrote:
> > If the strparser function of sk is turned on, all received data needs to
> > be processed by strparser first.
> >
> > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
> > Signed-off-by: Liu Jian <liujian56@huawei.com>
> > ---
>
> [...]
>
> > net/core/skmsg.c | 5 +++++
> > net/ipv4/tcp_bpf.c | 9 ++++++---
> > 3 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
> > index 94e2a1f6e58d..25e92dff04aa 100644
> > --- a/include/linux/skmsg.h
> > +++ b/include/linux/skmsg.h
> > @@ -390,6 +390,7 @@ void sk_psock_stop(struct sk_psock *psock, bool wait);
> > int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock);
> > void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock);
> > void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock);
> > +bool sk_psock_strparser_started(struct sock *sk);
> > #else
> > static inline int sk_psock_init_strp(struct sock *sk, struct sk_psock *psock)
> > {
> > @@ -403,6 +404,11 @@ static inline void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock)
> > static inline void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
> > {
> > }
> > +
> > +static inline bool sk_psock_strparser_started(struct sock *sk)
> > +{
> > + return false;
> > +}
> > #endif
> >
> > void sk_psock_start_verdict(struct sock *sk, struct sk_psock *psock);
> > diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> > index e85b7f8491b9..dd64ef854f3e 100644
> > --- a/net/core/skmsg.c
> > +++ b/net/core/skmsg.c
> > @@ -1105,6 +1105,11 @@ void sk_psock_start_strp(struct sock *sk, struct sk_psock *psock)
> > sk->sk_write_space = sk_psock_write_space;
> > }
> >
> > +bool sk_psock_strparser_started(struct sock *sk)
> > +{
> > + return sk->sk_data_ready == sk_psock_strp_data_ready;
>
> What if kTLS is configured on the socket? I think this check won't work then.
Liu, did you see this. I think its a bit cleaner, avoids the extra parser
check in hotpath, and should solve the issue?
https://patchwork.kernel.org/project/netdevbpf/patch/20211011191647.418704-3-john.fastabend@gmail.com/
I think it should also address Jakub's concern.
Thanks,
John
>
> > +}
> > +
> > void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock)
> > {
> > if (!psock->saved_data_ready)
>
> [...]
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] bpf, sockmap: Do not read sk_receive_queue in tcp_bpf_recvmsg if strparser enabled
2021-10-18 13:53 ` John Fastabend
@ 2021-10-19 3:03 ` liujian (CE)
0 siblings, 0 replies; 4+ messages in thread
From: liujian (CE) @ 2021-10-19 3:03 UTC (permalink / raw)
To: John Fastabend, Jakub Sitnicki
Cc: daniel, lmb, edumazet, davem, kuba, yoshfuji, dsahern, ast,
andrii, kafai, songliubraving, yhs, kpsingh, netdev, bpf
> -----Original Message-----
> From: John Fastabend [mailto:john.fastabend@gmail.com]
> Sent: Monday, October 18, 2021 9:54 PM
> To: Jakub Sitnicki <jakub@cloudflare.com>; liujian (CE)
> <liujian56@huawei.com>
> Cc: john.fastabend@gmail.com; daniel@iogearbox.net; lmb@cloudflare.com;
> edumazet@google.com; davem@davemloft.net; kuba@kernel.org;
> yoshfuji@linux-ipv6.org; dsahern@kernel.org; ast@kernel.org;
> andrii@kernel.org; kafai@fb.com; songliubraving@fb.com; yhs@fb.com;
> kpsingh@kernel.org; netdev@vger.kernel.org; bpf@vger.kernel.org
> Subject: Re: [PATCH] bpf, sockmap: Do not read sk_receive_queue in
> tcp_bpf_recvmsg if strparser enabled
>
> Jakub Sitnicki wrote:
> > On Fri, Oct 15, 2021 at 10:01 AM CEST, Liu Jian wrote:
> > > If the strparser function of sk is turned on, all received data
> > > needs to be processed by strparser first.
> > >
> > > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg
> > > interface")
> > > Signed-off-by: Liu Jian <liujian56@huawei.com>
> > > ---
> >
> > [...]
> >
> > > net/core/skmsg.c | 5 +++++
> > > net/ipv4/tcp_bpf.c | 9 ++++++---
> > > 3 files changed, 17 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h index
> > > 94e2a1f6e58d..25e92dff04aa 100644
> > > --- a/include/linux/skmsg.h
> > > +++ b/include/linux/skmsg.h
> > > @@ -390,6 +390,7 @@ void sk_psock_stop(struct sk_psock *psock, bool
> > > wait); int sk_psock_init_strp(struct sock *sk, struct sk_psock
> > > *psock); void sk_psock_start_strp(struct sock *sk, struct sk_psock
> > > *psock); void sk_psock_stop_strp(struct sock *sk, struct sk_psock
> > > *psock);
> > > +bool sk_psock_strparser_started(struct sock *sk);
> > > #else
> > > static inline int sk_psock_init_strp(struct sock *sk, struct
> > > sk_psock *psock) { @@ -403,6 +404,11 @@ static inline void
> > > sk_psock_start_strp(struct sock *sk, struct sk_psock *psock) static
> > > inline void sk_psock_stop_strp(struct sock *sk, struct sk_psock
> > > *psock) { }
> > > +
> > > +static inline bool sk_psock_strparser_started(struct sock *sk) {
> > > + return false;
> > > +}
> > > #endif
> > >
> > > void sk_psock_start_verdict(struct sock *sk, struct sk_psock
> > > *psock); diff --git a/net/core/skmsg.c b/net/core/skmsg.c index
> > > e85b7f8491b9..dd64ef854f3e 100644
> > > --- a/net/core/skmsg.c
> > > +++ b/net/core/skmsg.c
> > > @@ -1105,6 +1105,11 @@ void sk_psock_start_strp(struct sock *sk,
> struct sk_psock *psock)
> > > sk->sk_write_space = sk_psock_write_space; }
> > >
> > > +bool sk_psock_strparser_started(struct sock *sk) {
> > > + return sk->sk_data_ready == sk_psock_strp_data_ready;
> >
> > What if kTLS is configured on the socket? I think this check won't work then.
>
> Liu, did you see this. I think its a bit cleaner, avoids the extra parser check in
> hotpath, and should solve the issue?
>
> https://patchwork.kernel.org/project/netdevbpf/patch/20211011191647.418
> 704-3-john.fastabend@gmail.com/
>
> I think it should also address Jakub's concern.
>
I am sorry, I did not see the patch before.
I think it can solve my issue, please ignore my patch.
Thanks.
> Thanks,
> John
>
> >
> > > +}
> > > +
> > > void sk_psock_stop_strp(struct sock *sk, struct sk_psock *psock) {
> > > if (!psock->saved_data_ready)
> >
> > [...]
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-19 3:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 8:01 [PATCH] bpf, sockmap: Do not read sk_receive_queue in tcp_bpf_recvmsg if strparser enabled Liu Jian
2021-10-18 8:54 ` Jakub Sitnicki
2021-10-18 13:53 ` John Fastabend
2021-10-19 3:03 ` liujian (CE)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).