* Re: [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue [not found] ` <CAH+Qyb+37gaWZzEvvXeX9ghsCYw1JyH_23S+1HW0ML-MZkcYfg@mail.gmail.com> @ 2020-01-08 3:54 ` John Fastabend 2020-01-08 4:57 ` Lingpeng Chen 0 siblings, 1 reply; 9+ messages in thread From: John Fastabend @ 2020-01-08 3:54 UTC (permalink / raw) To: Forrest Chen, John Fastabend; +Cc: Daniel Borkmann, netdev, bpf Forrest Chen wrote: > Thanks John for the review. Usually try not to top post. > > I think you are right for the missing part. I would like to update the > patch and re-send it, is it ok for you? Yep glad to have more folks looking over sockmap code and looks like it already hit the list so that is good. > > Since it is my first kernel patch, I'm not familiar with the process. > What''s the meaning of 'add your Signed-off-by'? > I think I have add Signed-off-by in the first patch, do you mean I should > add 'Signed-off-by: John Fastabend <john.fastabend@gmail.com>' as well? > I was just saying feel free to add _your_ Signed-off-by to the patch I attached and send it. You added my signed-off-by to your patch which is also fine I don't think it matters much as long as we get the fix. Also we should add Fixes tag and a tag to give Arika some credit. Seems we had the same fix in mind. I'll just add those now, next time for fixes please add a Fixes tag so we can track where we need to backport the fix. Thanks for looking into this. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue 2020-01-08 3:54 ` [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue John Fastabend @ 2020-01-08 4:57 ` Lingpeng Chen 2020-01-08 16:50 ` Song Liu 2020-01-08 17:02 ` Daniel Borkmann 0 siblings, 2 replies; 9+ messages in thread From: Lingpeng Chen @ 2020-01-08 4:57 UTC (permalink / raw) To: John Fastabend; +Cc: Daniel Borkmann, netdev, bpf, Lingpeng Chen Right now in tcp_bpf_recvmsg, sock read data first from sk_receive_queue if not empty than psock->ingress_msg otherwise. If a FIN packet arrives and there's also some data in psock->ingress_msg, the data in psock->ingress_msg will be purged. It is always happen when request to a HTTP1.0 server like python SimpleHTTPServer since the server send FIN packet after data is sent out. Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") Reported-by: Arika Chen <eaglesora@gmail.com> Suggested-by: Arika Chen <eaglesora@gmail.com> Signed-off-by: Lingpeng Chen <forrest0579@gmail.com> Signed-off-by: John Fastabend <john.fastabend@gmail.com> --- net/ipv4/tcp_bpf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c index e38705165ac9..f7e902868fce 100644 --- a/net/ipv4/tcp_bpf.c +++ b/net/ipv4/tcp_bpf.c @@ -123,12 +123,13 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, if (unlikely(flags & MSG_ERRQUEUE)) return inet_recv_error(sk, msg, len, addr_len); - if (!skb_queue_empty(&sk->sk_receive_queue)) - return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); psock = sk_psock_get(sk); if (unlikely(!psock)) return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); + if (!skb_queue_empty(&sk->sk_receive_queue) && + sk_psock_queue_empty(psock)) + return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); lock_sock(sk); msg_bytes_ready: copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags); @@ -139,7 +140,7 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, timeo = sock_rcvtimeo(sk, nonblock); data = tcp_bpf_wait_data(sk, psock, flags, timeo, &err); if (data) { - if (skb_queue_empty(&sk->sk_receive_queue)) + if (!sk_psock_queue_empty(psock)) goto msg_bytes_ready; release_sock(sk); sk_psock_put(sk, psock); -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue 2020-01-08 4:57 ` Lingpeng Chen @ 2020-01-08 16:50 ` Song Liu 2020-01-08 17:02 ` Daniel Borkmann 1 sibling, 0 replies; 9+ messages in thread From: Song Liu @ 2020-01-08 16:50 UTC (permalink / raw) To: Lingpeng Chen; +Cc: John Fastabend, Daniel Borkmann, Networking, bpf On Tue, Jan 7, 2020 at 8:58 PM Lingpeng Chen <forrest0579@gmail.com> wrote: > > Right now in tcp_bpf_recvmsg, sock read data first from sk_receive_queue > if not empty than psock->ingress_msg otherwise. If a FIN packet arrives > and there's also some data in psock->ingress_msg, the data in > psock->ingress_msg will be purged. It is always happen when request to a > HTTP1.0 server like python SimpleHTTPServer since the server send FIN > packet after data is sent out. > > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") > Reported-by: Arika Chen <eaglesora@gmail.com> > Suggested-by: Arika Chen <eaglesora@gmail.com> > Signed-off-by: Lingpeng Chen <forrest0579@gmail.com> > Signed-off-by: John Fastabend <john.fastabend@gmail.com> Cc: stable@vger.kernel.org # v4.20+ Acked-by: Song Liu <songliubraving@fb.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue 2020-01-08 4:57 ` Lingpeng Chen 2020-01-08 16:50 ` Song Liu @ 2020-01-08 17:02 ` Daniel Borkmann 2020-01-08 18:01 ` John Fastabend 1 sibling, 1 reply; 9+ messages in thread From: Daniel Borkmann @ 2020-01-08 17:02 UTC (permalink / raw) To: Lingpeng Chen; +Cc: John Fastabend, netdev, bpf On Wed, Jan 08, 2020 at 12:57:08PM +0800, Lingpeng Chen wrote: > Right now in tcp_bpf_recvmsg, sock read data first from sk_receive_queue > if not empty than psock->ingress_msg otherwise. If a FIN packet arrives > and there's also some data in psock->ingress_msg, the data in > psock->ingress_msg will be purged. It is always happen when request to a > HTTP1.0 server like python SimpleHTTPServer since the server send FIN > packet after data is sent out. > > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") > Reported-by: Arika Chen <eaglesora@gmail.com> > Suggested-by: Arika Chen <eaglesora@gmail.com> > Signed-off-by: Lingpeng Chen <forrest0579@gmail.com> > Signed-off-by: John Fastabend <john.fastabend@gmail.com> > --- > net/ipv4/tcp_bpf.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c > index e38705165ac9..f7e902868fce 100644 > --- a/net/ipv4/tcp_bpf.c > +++ b/net/ipv4/tcp_bpf.c > @@ -123,12 +123,13 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > > if (unlikely(flags & MSG_ERRQUEUE)) > return inet_recv_error(sk, msg, len, addr_len); Shouldn't we also move the error queue handling below the psock test as well and let tcp_recvmsg() natively do it in case of !psock? > - if (!skb_queue_empty(&sk->sk_receive_queue)) > - return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); > > psock = sk_psock_get(sk); > if (unlikely(!psock)) > return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); > + if (!skb_queue_empty(&sk->sk_receive_queue) && > + sk_psock_queue_empty(psock)) > + return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); > lock_sock(sk); > msg_bytes_ready: > copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags); > @@ -139,7 +140,7 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > timeo = sock_rcvtimeo(sk, nonblock); > data = tcp_bpf_wait_data(sk, psock, flags, timeo, &err); > if (data) { > - if (skb_queue_empty(&sk->sk_receive_queue)) > + if (!sk_psock_queue_empty(psock)) > goto msg_bytes_ready; > release_sock(sk); > sk_psock_put(sk, psock); > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue 2020-01-08 17:02 ` Daniel Borkmann @ 2020-01-08 18:01 ` John Fastabend 2020-01-08 18:17 ` Daniel Borkmann 0 siblings, 1 reply; 9+ messages in thread From: John Fastabend @ 2020-01-08 18:01 UTC (permalink / raw) To: Daniel Borkmann, Lingpeng Chen; +Cc: John Fastabend, netdev, bpf Daniel Borkmann wrote: > On Wed, Jan 08, 2020 at 12:57:08PM +0800, Lingpeng Chen wrote: > > Right now in tcp_bpf_recvmsg, sock read data first from sk_receive_queue > > if not empty than psock->ingress_msg otherwise. If a FIN packet arrives > > and there's also some data in psock->ingress_msg, the data in > > psock->ingress_msg will be purged. It is always happen when request to a > > HTTP1.0 server like python SimpleHTTPServer since the server send FIN > > packet after data is sent out. > > > > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") > > Reported-by: Arika Chen <eaglesora@gmail.com> > > Suggested-by: Arika Chen <eaglesora@gmail.com> > > Signed-off-by: Lingpeng Chen <forrest0579@gmail.com> > > Signed-off-by: John Fastabend <john.fastabend@gmail.com> > > --- > > net/ipv4/tcp_bpf.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c > > index e38705165ac9..f7e902868fce 100644 > > --- a/net/ipv4/tcp_bpf.c > > +++ b/net/ipv4/tcp_bpf.c > > @@ -123,12 +123,13 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > > > > if (unlikely(flags & MSG_ERRQUEUE)) > > return inet_recv_error(sk, msg, len, addr_len); > > Shouldn't we also move the error queue handling below the psock test as > well and let tcp_recvmsg() natively do it in case of !psock? > You mean the MSG_ERRQUEUE flag handling? If the user sets MSG_ERRQUEUE they expect to receive any queued errors it would be wrong to return psock data in this case if psock is attached and has data on queue and user passes MSG_ERRQUEUE flag. MSG_ERRQUEUE (since Linux 2.2) This flag specifies that queued errors should be received from the socket error queue. The error is passed in an ancillary message with a type dependent on the protocol (for IPv4 IP_RECVERR). The user should supply a buffer of sufficient size. See cmsg(3) and ip(7) for more information. The payload of the original packet that caused the error is passed as normal data via msg_iovec. The original destination address of the datagram that caused the error is supplied via msg_name. I believe it needs to be where it is. > > - if (!skb_queue_empty(&sk->sk_receive_queue)) > > - return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); > > > > psock = sk_psock_get(sk); > > if (unlikely(!psock)) > > return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); > > + if (!skb_queue_empty(&sk->sk_receive_queue) && > > + sk_psock_queue_empty(psock)) > > + return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); > > lock_sock(sk); > > msg_bytes_ready: > > copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags); > > @@ -139,7 +140,7 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > > timeo = sock_rcvtimeo(sk, nonblock); > > data = tcp_bpf_wait_data(sk, psock, flags, timeo, &err); > > if (data) { > > - if (skb_queue_empty(&sk->sk_receive_queue)) > > + if (!sk_psock_queue_empty(psock)) > > goto msg_bytes_ready; > > release_sock(sk); > > sk_psock_put(sk, psock); > > -- > > 2.17.1 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue 2020-01-08 18:01 ` John Fastabend @ 2020-01-08 18:17 ` Daniel Borkmann 2020-01-08 18:34 ` John Fastabend 0 siblings, 1 reply; 9+ messages in thread From: Daniel Borkmann @ 2020-01-08 18:17 UTC (permalink / raw) To: John Fastabend, Lingpeng Chen; +Cc: netdev, bpf On 1/8/20 7:01 PM, John Fastabend wrote: > Daniel Borkmann wrote: >> On Wed, Jan 08, 2020 at 12:57:08PM +0800, Lingpeng Chen wrote: >>> Right now in tcp_bpf_recvmsg, sock read data first from sk_receive_queue >>> if not empty than psock->ingress_msg otherwise. If a FIN packet arrives >>> and there's also some data in psock->ingress_msg, the data in >>> psock->ingress_msg will be purged. It is always happen when request to a >>> HTTP1.0 server like python SimpleHTTPServer since the server send FIN >>> packet after data is sent out. >>> >>> Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") >>> Reported-by: Arika Chen <eaglesora@gmail.com> >>> Suggested-by: Arika Chen <eaglesora@gmail.com> >>> Signed-off-by: Lingpeng Chen <forrest0579@gmail.com> >>> Signed-off-by: John Fastabend <john.fastabend@gmail.com> >>> --- >>> net/ipv4/tcp_bpf.c | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c >>> index e38705165ac9..f7e902868fce 100644 >>> --- a/net/ipv4/tcp_bpf.c >>> +++ b/net/ipv4/tcp_bpf.c >>> @@ -123,12 +123,13 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, >>> >>> if (unlikely(flags & MSG_ERRQUEUE)) >>> return inet_recv_error(sk, msg, len, addr_len); >> >> Shouldn't we also move the error queue handling below the psock test as >> well and let tcp_recvmsg() natively do it in case of !psock? >> > > You mean the MSG_ERRQUEUE flag handling? If the user sets MSG_ERRQUEUE > they expect to receive any queued errors it would be wrong to return > psock data in this case if psock is attached and has data on queue and > user passes MSG_ERRQUEUE flag. > > MSG_ERRQUEUE (since Linux 2.2) > This flag specifies that queued errors should be received from the socket > error queue. The error is passed in an ancillary message with a type > dependent on the protocol (for IPv4 IP_RECVERR). The user should supply > a buffer of sufficient size. See cmsg(3) and ip(7) for more information. > The payload of the original packet that caused the error is passed as > normal data via msg_iovec. The original destination address of the > datagram that caused the error is supplied via msg_name. > > I believe it needs to be where it is. I meant that it should have looked as follows (aka moving both below the psock test) ... psock = sk_psock_get(sk); if (unlikely(!psock)) return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); if (unlikely(flags & MSG_ERRQUEUE)) return inet_recv_error(sk, msg, len, addr_len); if (!skb_queue_empty(&sk->sk_receive_queue) && [...] ... since when detached it's handled already via tcp_recvmsg() internals. >>> - if (!skb_queue_empty(&sk->sk_receive_queue)) >>> - return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); >>> >>> psock = sk_psock_get(sk); >>> if (unlikely(!psock)) >>> return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); >>> + if (!skb_queue_empty(&sk->sk_receive_queue) && >>> + sk_psock_queue_empty(psock)) >>> + return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); >>> lock_sock(sk); >>> msg_bytes_ready: >>> copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags); >>> @@ -139,7 +140,7 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, >>> timeo = sock_rcvtimeo(sk, nonblock); >>> data = tcp_bpf_wait_data(sk, psock, flags, timeo, &err); >>> if (data) { >>> - if (skb_queue_empty(&sk->sk_receive_queue)) >>> + if (!sk_psock_queue_empty(psock)) >>> goto msg_bytes_ready; >>> release_sock(sk); >>> sk_psock_put(sk, psock); >>> -- >>> 2.17.1 >>> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue 2020-01-08 18:17 ` Daniel Borkmann @ 2020-01-08 18:34 ` John Fastabend 2020-01-09 1:48 ` [PATCH v2] " Lingpeng Chen 0 siblings, 1 reply; 9+ messages in thread From: John Fastabend @ 2020-01-08 18:34 UTC (permalink / raw) To: Daniel Borkmann, John Fastabend, Lingpeng Chen; +Cc: netdev, bpf Daniel Borkmann wrote: > On 1/8/20 7:01 PM, John Fastabend wrote: > > Daniel Borkmann wrote: > >> On Wed, Jan 08, 2020 at 12:57:08PM +0800, Lingpeng Chen wrote: > >>> Right now in tcp_bpf_recvmsg, sock read data first from sk_receive_queue > >>> if not empty than psock->ingress_msg otherwise. If a FIN packet arrives > >>> and there's also some data in psock->ingress_msg, the data in > >>> psock->ingress_msg will be purged. It is always happen when request to a > >>> HTTP1.0 server like python SimpleHTTPServer since the server send FIN > >>> packet after data is sent out. > >>> > >>> Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") > >>> Reported-by: Arika Chen <eaglesora@gmail.com> > >>> Suggested-by: Arika Chen <eaglesora@gmail.com> > >>> Signed-off-by: Lingpeng Chen <forrest0579@gmail.com> > >>> Signed-off-by: John Fastabend <john.fastabend@gmail.com> > >>> --- > >>> net/ipv4/tcp_bpf.c | 7 ++++--- > >>> 1 file changed, 4 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c > >>> index e38705165ac9..f7e902868fce 100644 > >>> --- a/net/ipv4/tcp_bpf.c > >>> +++ b/net/ipv4/tcp_bpf.c > >>> @@ -123,12 +123,13 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, > >>> > >>> if (unlikely(flags & MSG_ERRQUEUE)) > >>> return inet_recv_error(sk, msg, len, addr_len); > >> > >> Shouldn't we also move the error queue handling below the psock test as > >> well and let tcp_recvmsg() natively do it in case of !psock? > >> > > > > You mean the MSG_ERRQUEUE flag handling? If the user sets MSG_ERRQUEUE > > they expect to receive any queued errors it would be wrong to return > > psock data in this case if psock is attached and has data on queue and > > user passes MSG_ERRQUEUE flag. > > > > MSG_ERRQUEUE (since Linux 2.2) > > This flag specifies that queued errors should be received from the socket > > error queue. The error is passed in an ancillary message with a type > > dependent on the protocol (for IPv4 IP_RECVERR). The user should supply > > a buffer of sufficient size. See cmsg(3) and ip(7) for more information. > > The payload of the original packet that caused the error is passed as > > normal data via msg_iovec. The original destination address of the > > datagram that caused the error is supplied via msg_name. > > > > I believe it needs to be where it is. > > I meant that it should have looked as follows (aka moving both below the > psock test) ... > > psock = sk_psock_get(sk); > if (unlikely(!psock)) > return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); > if (unlikely(flags & MSG_ERRQUEUE)) > return inet_recv_error(sk, msg, len, addr_len); > if (!skb_queue_empty(&sk->sk_receive_queue) && [...] > > ... since when detached it's handled already via tcp_recvmsg() internals. > Ah, OK the 'if (unlikely(!psock))' is a very rare case so I'm not sure it matters much. But, I agree it is slightly nicer. Lingpeng, can you spin a v2 moving the MSG_ERRQUEUE check down and also keep the ACK and stable tags I don't think its a very big change the Acks can stick around IMO. Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] bpf/sockmap: read psock ingress_msg before sk_receive_queue 2020-01-08 18:34 ` John Fastabend @ 2020-01-09 1:48 ` Lingpeng Chen 2020-01-09 22:16 ` Daniel Borkmann 0 siblings, 1 reply; 9+ messages in thread From: Lingpeng Chen @ 2020-01-09 1:48 UTC (permalink / raw) To: John Fastabend Cc: Daniel Borkmann, Song Liu, netdev, bpf, Lingpeng Chen, stable Right now in tcp_bpf_recvmsg, sock read data first from sk_receive_queue if not empty than psock->ingress_msg otherwise. If a FIN packet arrives and there's also some data in psock->ingress_msg, the data in psock->ingress_msg will be purged. It is always happen when request to a HTTP1.0 server like python SimpleHTTPServer since the server send FIN packet after data is sent out. Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") Reported-by: Arika Chen <eaglesora@gmail.com> Suggested-by: Arika Chen <eaglesora@gmail.com> Signed-off-by: Lingpeng Chen <forrest0579@gmail.com> Signed-off-by: John Fastabend <john.fastabend@gmail.com> Cc: stable@vger.kernel.org # v4.20+ Acked-by: Song Liu <songliubraving@fb.com> --- net/ipv4/tcp_bpf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c index e38705165ac9..e6b08b5a0895 100644 --- a/net/ipv4/tcp_bpf.c +++ b/net/ipv4/tcp_bpf.c @@ -121,14 +121,14 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, struct sk_psock *psock; int copied, ret; - if (unlikely(flags & MSG_ERRQUEUE)) - return inet_recv_error(sk, msg, len, addr_len); - if (!skb_queue_empty(&sk->sk_receive_queue)) - return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); - psock = sk_psock_get(sk); if (unlikely(!psock)) return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); + if (unlikely(flags & MSG_ERRQUEUE)) + return inet_recv_error(sk, msg, len, addr_len); + if (!skb_queue_empty(&sk->sk_receive_queue) && + sk_psock_queue_empty(psock)) + return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len); lock_sock(sk); msg_bytes_ready: copied = __tcp_bpf_recvmsg(sk, psock, msg, len, flags); @@ -139,7 +139,7 @@ int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, timeo = sock_rcvtimeo(sk, nonblock); data = tcp_bpf_wait_data(sk, psock, flags, timeo, &err); if (data) { - if (skb_queue_empty(&sk->sk_receive_queue)) + if (!sk_psock_queue_empty(psock)) goto msg_bytes_ready; release_sock(sk); sk_psock_put(sk, psock); -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] bpf/sockmap: read psock ingress_msg before sk_receive_queue 2020-01-09 1:48 ` [PATCH v2] " Lingpeng Chen @ 2020-01-09 22:16 ` Daniel Borkmann 0 siblings, 0 replies; 9+ messages in thread From: Daniel Borkmann @ 2020-01-09 22:16 UTC (permalink / raw) To: Lingpeng Chen, John Fastabend; +Cc: Song Liu, netdev, bpf, stable On 1/9/20 2:48 AM, Lingpeng Chen wrote: > Right now in tcp_bpf_recvmsg, sock read data first from sk_receive_queue > if not empty than psock->ingress_msg otherwise. If a FIN packet arrives > and there's also some data in psock->ingress_msg, the data in > psock->ingress_msg will be purged. It is always happen when request to a > HTTP1.0 server like python SimpleHTTPServer since the server send FIN > packet after data is sent out. > > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") > Reported-by: Arika Chen <eaglesora@gmail.com> > Suggested-by: Arika Chen <eaglesora@gmail.com> > Signed-off-by: Lingpeng Chen <forrest0579@gmail.com> > Signed-off-by: John Fastabend <john.fastabend@gmail.com> > Cc: stable@vger.kernel.org # v4.20+ > Acked-by: Song Liu <songliubraving@fb.com> Applied to bpf, thanks! ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-01-09 22:16 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200107042247.16614-1-forrest0579@gmail.com> [not found] ` <5e14a5fe53ac8_67962afd051fc5c0ea@john-XPS-13-9370.notmuch> [not found] ` <CAH+Qyb+37gaWZzEvvXeX9ghsCYw1JyH_23S+1HW0ML-MZkcYfg@mail.gmail.com> 2020-01-08 3:54 ` [PATCH] bpf/sockmap: read psock ingress_msg before sk_receive_queue John Fastabend 2020-01-08 4:57 ` Lingpeng Chen 2020-01-08 16:50 ` Song Liu 2020-01-08 17:02 ` Daniel Borkmann 2020-01-08 18:01 ` John Fastabend 2020-01-08 18:17 ` Daniel Borkmann 2020-01-08 18:34 ` John Fastabend 2020-01-09 1:48 ` [PATCH v2] " Lingpeng Chen 2020-01-09 22:16 ` Daniel Borkmann
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).