On 12/7/24 9:37 AM, Jason Xing wrote:
From: Jason Xing <kernelxing@xxxxxxxxxxx>
Handle the ACK timestamp case. Actually testing SKBTX_BPF flag
can work, but we need to Introduce a new txstamp_ack_bpf to avoid
cache line misses in tcp_ack_tstamp().
Signed-off-by: Jason Xing <kernelxing@xxxxxxxxxxx>
---
include/net/tcp.h | 3 ++-
include/uapi/linux/bpf.h | 5 +++++
net/core/skbuff.c | 9 ++++++---
net/ipv4/tcp_input.c | 3 ++-
net/ipv4/tcp_output.c | 5 +++++
tools/include/uapi/linux/bpf.h | 5 +++++
6 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index e9b37b76e894..8e5103d3c6b9 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -959,9 +959,10 @@ struct tcp_skb_cb {
__u8 sacked; /* State flags for SACK. */
__u8 ip_dsfield; /* IPv4 tos or IPv6 dsfield */
__u8 txstamp_ack:1, /* Record TX timestamp for ack? */
+ txstamp_ack_bpf:1, /* ack timestamp for bpf use */
After quickly peeking at patch 8, I realize that the new txstamp_ack_bpf bit is
not needed. SKBTX_BPF bit (in skb_shinfo(skb)->tx_flags) and the txstamp_ack_bpf
are always set together. Then only use the SKBTX_BPF bit should be as good.
eor:1, /* Is skb MSG_EOR marked? */
has_rxtstamp:1, /* SKB has a RX timestamp */
- unused:5;
+ unused:4;
__u32 ack_seq; /* Sequence number ACK'd */
union {
struct {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a6d761f07f67..a0aff1b4eb61 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -7032,6 +7032,11 @@ enum {
* feature is on. It indicates the
* recorded timestamp.
*/
+ BPF_SOCK_OPS_TS_ACK_OPT_CB, /* Called when all the skbs are
+ * acknowledged when SO_TIMESTAMPING
+ * feature is on. It indicates the
+ * recorded timestamp.
+ */
};
/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 73b15d6277f7..48b0c71e9522 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5553,6 +5553,9 @@ static void __skb_tstamp_tx_bpf(struct sock *sk, struct sk_buff *skb, int tstype
case SCM_TSTAMP_SND:
op = BPF_SOCK_OPS_TS_SW_OPT_CB;
break;
+ case SCM_TSTAMP_ACK:
+ op = BPF_SOCK_OPS_TS_ACK_OPT_CB;
+ break;
default:
return;
}
@@ -5632,9 +5635,9 @@ static bool skb_tstamp_is_set(const struct sk_buff *skb, int tstype, bool bpf_mo
return true;
return false;
case SCM_TSTAMP_ACK:
- if (TCP_SKB_CB(skb)->txstamp_ack)
- return true;
- return false;
+ flag = bpf_mode ? TCP_SKB_CB(skb)->txstamp_ack_bpf :
+ TCP_SKB_CB(skb)->txstamp_ack;
+ return !!flag;
}
return false;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 5bdf13ac26ef..82bb26f5b214 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3321,7 +3321,8 @@ static void tcp_ack_tstamp(struct sock *sk, struct sk_buff *skb,
const struct skb_shared_info *shinfo;
/* Avoid cache line misses to get skb_shinfo() and shinfo->tx_flags */
- if (likely(!TCP_SKB_CB(skb)->txstamp_ack))
+ if (likely(!TCP_SKB_CB(skb)->txstamp_ack &&
+ !TCP_SKB_CB(skb)->txstamp_ack_bpf))
Change the test here to:
if (likely(!TCP_SKB_CB(skb)->txstamp_ack &&
!(skb_shinfo(skb)->tx_flags & SKBTX_BPF)))
Does it make sense?
