* [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb()
@ 2024-04-21 4:20 Philo Lu
2024-04-21 4:20 ` [PATCH net-next 1/2] tcp: move tcp_skb_cb->sacked flags to enum Philo Lu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Philo Lu @ 2024-04-21 4:20 UTC (permalink / raw)
To: netdev, bpf
Cc: edumazet, davem, martin.lau, ast, daniel, andrii, kuba, pabeni,
dsahern, xuanzhuo, fred.cc
Move TCPCB_EVER_RETRANS updating after the trace_tcp_retransmit_skb()
in __tcp_retransmit_skb(), and then we are aware of whether the skb has
ever been retransmitted in this tracepoint. This can be used, e.g., to get
retransmission efficiency by counting skbs w/ and w/o TCPCB_EVER_RETRANS
(through bpf tracing programs).
For this purpose, TCPCB_EVER_RETRANS is also needed to be exposed to bpf.
Previously, the flags are defined as macros in struct tcp_skb_cb. I moved them
out into a new enum, and then they can be accessed with vmlinux.h.
We have discussed to achieve this with BPF_SOCK_OPS in [0], and using
tracepoint is thought to be a better solution.
[0]
https://lore.kernel.org/all/20240417124622.35333-1-lulie@linux.alibaba.com/
Philo Lu (2):
tcp: move tcp_skb_cb->sacked flags to enum
tcp: update sacked after tracepoint in __tcp_retransmit_skb
include/net/tcp.h | 22 +++++++++++++---------
net/ipv4/tcp_output.c | 11 ++++++-----
2 files changed, 19 insertions(+), 14 deletions(-)
--
2.32.0.3.g01195cf9f
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 1/2] tcp: move tcp_skb_cb->sacked flags to enum
2024-04-21 4:20 [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb() Philo Lu
@ 2024-04-21 4:20 ` Philo Lu
2024-04-25 6:22 ` Eric Dumazet
2024-04-21 4:20 ` [PATCH net-next 2/2] tcp: update sacked after tracepoint in __tcp_retransmit_skb Philo Lu
2024-04-25 8:00 ` [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb() patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Philo Lu @ 2024-04-21 4:20 UTC (permalink / raw)
To: netdev, bpf
Cc: edumazet, davem, martin.lau, ast, daniel, andrii, kuba, pabeni,
dsahern, xuanzhuo, fred.cc
Move the flag definitions for tcp_skb_cb->sacked into a new enum named
tcp_skb_cb_sacked_flags, then we can get access to them in bpf via
vmlinux.h, e.g., in tracepoints.
This patch does not change any existing functionality.
Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
---
include/net/tcp.h | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index b935e1ae4caf8..ffc9371fe9dea 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -928,6 +928,19 @@ static inline u32 tcp_rsk_tsval(const struct tcp_request_sock *treq)
#define TCPHDR_SYN_ECN (TCPHDR_SYN | TCPHDR_ECE | TCPHDR_CWR)
+/* State flags for sacked in struct tcp_skb_cb */
+enum tcp_skb_cb_sacked_flags {
+ TCPCB_SACKED_ACKED = (1 << 0), /* SKB ACK'd by a SACK block */
+ TCPCB_SACKED_RETRANS = (1 << 1), /* SKB retransmitted */
+ TCPCB_LOST = (1 << 2), /* SKB is lost */
+ TCPCB_TAGBITS = (TCPCB_SACKED_ACKED | TCPCB_SACKED_RETRANS |
+ TCPCB_LOST), /* All tag bits */
+ TCPCB_REPAIRED = (1 << 4), /* SKB repaired (no skb_mstamp_ns) */
+ TCPCB_EVER_RETRANS = (1 << 7), /* Ever retransmitted frame */
+ TCPCB_RETRANS = (TCPCB_SACKED_RETRANS | TCPCB_EVER_RETRANS |
+ TCPCB_REPAIRED),
+};
+
/* This is what the send packet queuing engine uses to pass
* TCP per-packet control information to the transmission code.
* We also store the host-order sequence numbers in here too.
@@ -950,15 +963,6 @@ struct tcp_skb_cb {
__u8 tcp_flags; /* TCP header flags. (tcp[13]) */
__u8 sacked; /* State flags for SACK. */
-#define TCPCB_SACKED_ACKED 0x01 /* SKB ACK'd by a SACK block */
-#define TCPCB_SACKED_RETRANS 0x02 /* SKB retransmitted */
-#define TCPCB_LOST 0x04 /* SKB is lost */
-#define TCPCB_TAGBITS 0x07 /* All tag bits */
-#define TCPCB_REPAIRED 0x10 /* SKB repaired (no skb_mstamp_ns) */
-#define TCPCB_EVER_RETRANS 0x80 /* Ever retransmitted frame */
-#define TCPCB_RETRANS (TCPCB_SACKED_RETRANS|TCPCB_EVER_RETRANS| \
- TCPCB_REPAIRED)
-
__u8 ip_dsfield; /* IPv4 tos or IPv6 dsfield */
__u8 txstamp_ack:1, /* Record TX timestamp for ack? */
eor:1, /* Is skb MSG_EOR marked? */
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] tcp: update sacked after tracepoint in __tcp_retransmit_skb
2024-04-21 4:20 [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb() Philo Lu
2024-04-21 4:20 ` [PATCH net-next 1/2] tcp: move tcp_skb_cb->sacked flags to enum Philo Lu
@ 2024-04-21 4:20 ` Philo Lu
2024-04-25 6:25 ` Eric Dumazet
2024-04-25 8:00 ` [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb() patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Philo Lu @ 2024-04-21 4:20 UTC (permalink / raw)
To: netdev, bpf
Cc: edumazet, davem, martin.lau, ast, daniel, andrii, kuba, pabeni,
dsahern, xuanzhuo, fred.cc
Marking TCP_SKB_CB(skb)->sacked with TCPCB_EVER_RETRANS after the
traceopint (trace_tcp_retransmit_skb), then we can get the
retransmission efficiency by counting skbs w/ and w/o TCPCB_EVER_RETRANS
mark in this tracepoint.
We have discussed to achieve this with BPF_SOCK_OPS in [0], and using
tracepoint is thought to be a better solution.
[0]
https://lore.kernel.org/all/20240417124622.35333-1-lulie@linux.alibaba.com/
Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
---
net/ipv4/tcp_output.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 61119d42b0fd2..e19e74e005c1b 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3390,11 +3390,6 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
}
- /* To avoid taking spuriously low RTT samples based on a timestamp
- * for a transmit that never happened, always mark EVER_RETRANS
- */
- TCP_SKB_CB(skb)->sacked |= TCPCB_EVER_RETRANS;
-
if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RETRANS_CB_FLAG))
tcp_call_bpf_3arg(sk, BPF_SOCK_OPS_RETRANS_CB,
TCP_SKB_CB(skb)->seq, segs, err);
@@ -3404,6 +3399,12 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
} else if (err != -EBUSY) {
NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPRETRANSFAIL, segs);
}
+
+ /* To avoid taking spuriously low RTT samples based on a timestamp
+ * for a transmit that never happened, always mark EVER_RETRANS
+ */
+ TCP_SKB_CB(skb)->sacked |= TCPCB_EVER_RETRANS;
+
return err;
}
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] tcp: move tcp_skb_cb->sacked flags to enum
2024-04-21 4:20 ` [PATCH net-next 1/2] tcp: move tcp_skb_cb->sacked flags to enum Philo Lu
@ 2024-04-25 6:22 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2024-04-25 6:22 UTC (permalink / raw)
To: Philo Lu
Cc: netdev, bpf, davem, martin.lau, ast, daniel, andrii, kuba,
pabeni, dsahern, xuanzhuo, fred.cc
On Sun, Apr 21, 2024 at 6:20 AM Philo Lu <lulie@linux.alibaba.com> wrote:
>
> Move the flag definitions for tcp_skb_cb->sacked into a new enum named
> tcp_skb_cb_sacked_flags, then we can get access to them in bpf via
> vmlinux.h, e.g., in tracepoints.
>
> This patch does not change any existing functionality.
>
> Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
Suggested-by: Martin KaFai Lau <martin.lau@kernel.org>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] tcp: update sacked after tracepoint in __tcp_retransmit_skb
2024-04-21 4:20 ` [PATCH net-next 2/2] tcp: update sacked after tracepoint in __tcp_retransmit_skb Philo Lu
@ 2024-04-25 6:25 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2024-04-25 6:25 UTC (permalink / raw)
To: Philo Lu
Cc: netdev, bpf, davem, martin.lau, ast, daniel, andrii, kuba,
pabeni, dsahern, xuanzhuo, fred.cc
On Sun, Apr 21, 2024 at 6:20 AM Philo Lu <lulie@linux.alibaba.com> wrote:
>
> Marking TCP_SKB_CB(skb)->sacked with TCPCB_EVER_RETRANS after the
> traceopint (trace_tcp_retransmit_skb), then we can get the
> retransmission efficiency by counting skbs w/ and w/o TCPCB_EVER_RETRANS
> mark in this tracepoint.
>
> We have discussed to achieve this with BPF_SOCK_OPS in [0], and using
> tracepoint is thought to be a better solution.
>
> [0]
> https://lore.kernel.org/all/20240417124622.35333-1-lulie@linux.alibaba.com/
>
> Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb()
2024-04-21 4:20 [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb() Philo Lu
2024-04-21 4:20 ` [PATCH net-next 1/2] tcp: move tcp_skb_cb->sacked flags to enum Philo Lu
2024-04-21 4:20 ` [PATCH net-next 2/2] tcp: update sacked after tracepoint in __tcp_retransmit_skb Philo Lu
@ 2024-04-25 8:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-04-25 8:00 UTC (permalink / raw)
To: Philo Lu
Cc: netdev, bpf, edumazet, davem, martin.lau, ast, daniel, andrii,
kuba, pabeni, dsahern, xuanzhuo, fred.cc
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:
On Sun, 21 Apr 2024 12:20:07 +0800 you wrote:
> Move TCPCB_EVER_RETRANS updating after the trace_tcp_retransmit_skb()
> in __tcp_retransmit_skb(), and then we are aware of whether the skb has
> ever been retransmitted in this tracepoint. This can be used, e.g., to get
> retransmission efficiency by counting skbs w/ and w/o TCPCB_EVER_RETRANS
> (through bpf tracing programs).
>
> For this purpose, TCPCB_EVER_RETRANS is also needed to be exposed to bpf.
> Previously, the flags are defined as macros in struct tcp_skb_cb. I moved them
> out into a new enum, and then they can be accessed with vmlinux.h.
>
> [...]
Here is the summary with links:
- [net-next,1/2] tcp: move tcp_skb_cb->sacked flags to enum
https://git.kernel.org/netdev/net-next/c/14b5fb2145ca
- [net-next,2/2] tcp: update sacked after tracepoint in __tcp_retransmit_skb
https://git.kernel.org/netdev/net-next/c/2bf90a57f0e6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-25 8:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-21 4:20 [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb() Philo Lu
2024-04-21 4:20 ` [PATCH net-next 1/2] tcp: move tcp_skb_cb->sacked flags to enum Philo Lu
2024-04-25 6:22 ` Eric Dumazet
2024-04-21 4:20 ` [PATCH net-next 2/2] tcp: update sacked after tracepoint in __tcp_retransmit_skb Philo Lu
2024-04-25 6:25 ` Eric Dumazet
2024-04-25 8:00 ` [PATCH net-next 0/2] tcp: update TCPCB_EVER_RETRANS after trace_tcp_retransmit_skb() patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.