tcp_bpf: Fix the sk_mem_uncharge logic in tcp_bpf_sendmsg #4521
+9
−8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![kernel-patches-daemon-bpf-rc[bot]](https://avatars.githubusercontent.com/u/70728002?s=60&v=4){kind=small}
|
Upstream branch: 2aa587f |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
4e4da1a
to
3a72ba0
Compare
|
Upstream branch: 19039f2 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
671efe3
to
82f4296
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
3a72ba0
to
0171a57
Compare
|
Upstream branch: 98cd619 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
82f4296
to
9a5975e
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
0171a57
to
8021bac
Compare
|
Upstream branch: db123e4 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
9a5975e
to
97ab4fb
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
8021bac
to
f7d2a2c
Compare
|
Upstream branch: 5ac9b4e |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
97ab4fb
to
1da5eaa
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
f7d2a2c
to
1ec8e25
Compare
|
Upstream branch: 5ac9b4e |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
1da5eaa
to
f5212fe
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
1ec8e25
to
c4a2567
Compare
|
Upstream branch: 42f7652 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
f5212fe
to
7613fde
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
c4a2567
to
981ebd1
Compare
|
Upstream branch: a552e2e |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
7613fde
to
1616186
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
e01e251
to
e7aa107
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
61000b6
to
294235a
Compare
|
Upstream branch: 6801cf7 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
e7aa107
to
21f7249
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
294235a
to
5299cd6
Compare
|
Upstream branch: 44d0469 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
21f7249
to
615c1d4
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
5299cd6
to
2656c44
Compare
|
Upstream branch: 44d0469 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
615c1d4
to
c084ba2
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
2656c44
to
f2157d7
Compare
|
Upstream branch: fb86c42 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
c084ba2
to
2bf471d
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
f2157d7
to
e691a86
Compare
|
Upstream branch: fb86c42 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
2bf471d
to
4250270
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
e691a86
to
6a7f1b4
Compare
|
Upstream branch: 9f8e716 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
4250270
to
21174ff
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
6a7f1b4
to
3fd60a8
Compare
|
Upstream branch: 9f8e716 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
21174ff
to
99954d6
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
3fd60a8
to
4a6dea2
Compare
|
Upstream branch: adc2186 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
99954d6
to
ff9c807
Compare
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
bpf_base
branch
from
4a6dea2
to
3be981a
Compare
added 2 commits
November 18, 2024 18:21
…n test_sockmap Add this to more comprehensively test the socket memory accounting logic in the __SK_REDIRECT and __SK_DROP cases of tcp_bpf_sendmsg. We don't have test when apply_bytes are not zero in test_txmsg_redir_wait_sndmem. test_send_large has opt->rate=2, it will invoke sendmsg two times. Specifically, the first sendmsg will trigger the case where the ret value of tcp_bpf_sendmsg_redir is less than 0; while the second sendmsg happens after the 3 seconds timeout, and it will trigger __SK_DROP because socket c2 has been removed from the sockmap/hash. Signed-off-by: Zijian Zhang <[email protected]>
The current sk memory accounting logic in __SK_REDIRECT is pre-uncharging
tosend bytes, which is either msg->sg.size or a smaller value apply_bytes.
Potential problems with this strategy are as follows:
- If the actual sent bytes are smaller than tosend, we need to charge some
bytes back, as in line 487, which is okay but seems not clean.
- When tosend is set to apply_bytes, as in line 417, and (ret < 0), we may
miss uncharging (msg->sg.size - apply_bytes) bytes.
415 tosend = msg->sg.size;
416 if (psock->apply_bytes && psock->apply_bytes < tosend)
417 tosend = psock->apply_bytes;
...
443 sk_msg_return(sk, msg, tosend);
444 release_sock(sk);
446 origsize = msg->sg.size;
447 ret = tcp_bpf_sendmsg_redir(sk_redir, redir_ingress,
448 msg, tosend, flags);
449 sent = origsize - msg->sg.size;
...
454 lock_sock(sk);
455 if (unlikely(ret < 0)) {
456 int free = sk_msg_free_nocharge(sk, msg);
458 if (!cork)
459 *copied -= free;
460 }
...
487 if (eval == __SK_REDIRECT)
488 sk_mem_charge(sk, tosend - sent);
When running the selftest test_txmsg_redir_wait_sndmem with txmsg_apply,
the following warning will be reported,
------------[ cut here ]------------
WARNING: CPU: 6 PID: 57 at net/ipv4/af_inet.c:156 inet_sock_destruct+0x190/0x1a0
Modules linked in:
CPU: 6 UID: 0 PID: 57 Comm: kworker/6:0 Not tainted 6.12.0-rc1.bm.1-amd64+ #43
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
Workqueue: events sk_psock_destroy
RIP: 0010:inet_sock_destruct+0x190/0x1a0
RSP: 0018:ffffad0a8021fe08 EFLAGS: 00010206
RAX: 0000000000000011 RBX: ffff9aab4475b900 RCX: ffff9aab481a0800
RDX: 0000000000000303 RSI: 0000000000000011 RDI: ffff9aab4475b900
RBP: ffff9aab4475b990 R08: 0000000000000000 R09: ffff9aab40050ec0
R10: 0000000000000000 R11: ffff9aae6fdb1d01 R12: ffff9aab49c60400
R13: ffff9aab49c60598 R14: ffff9aab49c60598 R15: dead000000000100
FS: 0000000000000000(0000) GS:ffff9aae6fd80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffec7e47bd8 CR3: 00000001a1a1c004 CR4: 0000000000770ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
? __warn+0x89/0x130
? inet_sock_destruct+0x190/0x1a0
? report_bug+0xfc/0x1e0
? handle_bug+0x5c/0xa0
? exc_invalid_op+0x17/0x70
? asm_exc_invalid_op+0x1a/0x20
? inet_sock_destruct+0x190/0x1a0
__sk_destruct+0x25/0x220
sk_psock_destroy+0x2b2/0x310
process_scheduled_works+0xa3/0x3e0
worker_thread+0x117/0x240
? __pfx_worker_thread+0x10/0x10
kthread+0xcf/0x100
? __pfx_kthread+0x10/0x10
ret_from_fork+0x31/0x40
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
---[ end trace 0000000000000000 ]---
In __SK_REDIRECT, a more concise way is delaying the uncharging after sent
bytes are finalized, and uncharge this value. When (ret < 0), we shall
invoke sk_msg_free.
Same thing happens in case __SK_DROP, when tosend is set to apply_bytes,
we may miss uncharging (msg->sg.size - apply_bytes) bytes. The same
warning will be reported in selftest.
468 case __SK_DROP:
469 default:
470 sk_msg_free_partial(sk, msg, tosend);
471 sk_msg_apply_bytes(psock, tosend);
472 *copied -= (tosend + delta);
473 return -EACCES;
So instead of sk_msg_free_partial we can do sk_msg_free here.
Fixes: 604326b ("bpf, sockmap: convert to generic sk_msg interface")
Fixes: 8ec95b9 ("bpf, sockmap: Fix the sk->sk_forward_alloc warning of sk_stream_kill_queues")
Signed-off-by: Zijian Zhang <[email protected]>
|
Upstream branch: adc2186 |
kernel-patches-daemon-bpf-rc
bot
force-pushed
the
series/899984=>bpf
branch
from
ff9c807
to
39fc4e6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: tcp_bpf: Fix the sk_mem_uncharge logic in tcp_bpf_sendmsg
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=899984