Skip to content

Commit 901546f

Browse files
jrfastabborkmann
authored andcommitted
bpf, sockmap: Handle fin correctly
The sockmap code is returning EAGAIN after a FIN packet is received and no more data is on the receive queue. Correct behavior is to return 0 to the user and the user can then close the socket. The EAGAIN causes many apps to retry which masks the problem. Eventually the socket is evicted from the sockmap because its released from sockmap sock free handling. The issue creates a delay and can cause some errors on application side. To fix this check on sk_msg_recvmsg side if length is zero and FIN flag is set then set return to zero. A selftest will be added to check this condition. Fixes: 04919be ("tcp: Introduce tcp_read_skb()") Signed-off-by: John Fastabend <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Tested-by: William Findlay <[email protected]> Reviewed-by: Jakub Sitnicki <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent 405df89 commit 901546f

File tree

1 file changed

+31
-0
lines changed

1 file changed

+31
-0
lines changed

net/ipv4/tcp_bpf.c

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,24 @@ static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
174174
return ret;
175175
}
176176

177+
static bool is_next_msg_fin(struct sk_psock *psock)
178+
{
179+
struct scatterlist *sge;
180+
struct sk_msg *msg_rx;
181+
int i;
182+
183+
msg_rx = sk_psock_peek_msg(psock);
184+
i = msg_rx->sg.start;
185+
sge = sk_msg_elem(msg_rx, i);
186+
if (!sge->length) {
187+
struct sk_buff *skb = msg_rx->skb;
188+
189+
if (skb && TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
190+
return true;
191+
}
192+
return false;
193+
}
194+
177195
static int tcp_bpf_recvmsg_parser(struct sock *sk,
178196
struct msghdr *msg,
179197
size_t len,
@@ -196,6 +214,19 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
196214
lock_sock(sk);
197215
msg_bytes_ready:
198216
copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
217+
/* The typical case for EFAULT is the socket was gracefully
218+
* shutdown with a FIN pkt. So check here the other case is
219+
* some error on copy_page_to_iter which would be unexpected.
220+
* On fin return correct return code to zero.
221+
*/
222+
if (copied == -EFAULT) {
223+
bool is_fin = is_next_msg_fin(psock);
224+
225+
if (is_fin) {
226+
copied = 0;
227+
goto out;
228+
}
229+
}
199230
if (!copied) {
200231
long timeo;
201232
int data;

0 commit comments

Comments
 (0)