| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <67a3881cd3846_14e083294bb@willemb.c.googlers.com.notmuch>
Date: Wed, 05 Feb 2025 10:47:40 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jason Xing <kerneljasonxing@...il.com>,
davem@...emloft.net,
edumazet@...gle.com,
kuba@...nel.org,
pabeni@...hat.com,
dsahern@...nel.org,
willemdebruijn.kernel@...il.com,
willemb@...gle.com,
ast@...nel.org,
daniel@...earbox.net,
andrii@...nel.org,
martin.lau@...ux.dev,
eddyz87@...il.com,
song@...nel.org,
yonghong.song@...ux.dev,
john.fastabend@...il.com,
kpsingh@...nel.org,
sdf@...ichev.me,
haoluo@...gle.com,
jolsa@...nel.org,
horms@...nel.org
Cc: bpf@...r.kernel.org,
netdev@...r.kernel.org,
Jason Xing <kerneljasonxing@...il.com>
Subject: Re: [PATCH bpf-next v8 09/12] bpf: support SCM_TSTAMP_ACK of
SO_TIMESTAMPING
Jason Xing wrote:
> Handle the ACK timestamp case. Actually testing SKBTX_BPF flag
> can work, but Introducing a new txstamp_ack_bpf to avoid cache
repeat comment: s/Introducing/introduce
> line misses in tcp_ack_tstamp() is needed. To be more specific,
> in most cases, normal flows would not access skb_shinfo as
> txstamp_ack is zero, so that this function won't appear in the
> hot spot lists. Introducing a new member txstamp_ack_bpf works
> similarly.
>
> Signed-off-by: Jason Xing <kerneljasonxing@...il.com>
> ---
> include/net/tcp.h | 3 ++-
> include/uapi/linux/bpf.h | 5 +++++
> net/core/skbuff.c | 3 +++
> net/ipv4/tcp_input.c | 3 ++-
> net/ipv4/tcp_output.c | 5 +++++
> tools/include/uapi/linux/bpf.h | 5 +++++
> 6 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 293047694710..88429e422301 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 */
> 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 4c3566f623c2..800122a8abe5 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -7047,6 +7047,11 @@ enum {
> * timestamp that hardware just
> * generates.
> */
> + BPF_SOCK_OPS_TS_ACK_OPT_CB, /* Called when all the skbs in the
> + * same sendmsg call are acked
> + * when SK_BPF_CB_TX_TIMESTAMPING
> + * feature is on.
> + */
> };
>
> /* 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 264435f989ad..a8463fef574a 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -5579,6 +5579,9 @@ static void skb_tstamp_tx_bpf(struct sk_buff *skb, struct sock *sk,
> if (!sw && hwtstamps)
> *skb_hwtstamps(skb) = *hwtstamps;
> break;
> + case SCM_TSTAMP_ACK:
> + op = BPF_SOCK_OPS_TS_ACK_OPT_CB;
> + break;
> default:
> return;
> }
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 62252702929d..c8945f5be31b 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3323,7 +3323,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))
Here and elsewhere: instead of requiring multiple tests, how about
extending txstamp_ack to a two-bit field, so that a single branch
suffices.
> return;
>
> shinfo = skb_shinfo(skb);
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 695749807c09..fc84ca669b76 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1556,6 +1556,7 @@ static void tcp_adjust_pcount(struct sock *sk, const struct sk_buff *skb, int de
> static bool tcp_has_tx_tstamp(const struct sk_buff *skb)
> {
> return TCP_SKB_CB(skb)->txstamp_ack ||
> + TCP_SKB_CB(skb)->txstamp_ack_bpf ||
> (skb_shinfo(skb)->tx_flags & SKBTX_ANY_TSTAMP);
> }
>
> @@ -1572,7 +1573,9 @@ static void tcp_fragment_tstamp(struct sk_buff *skb, struct sk_buff *skb2)
> shinfo2->tx_flags |= tsflags;
> swap(shinfo->tskey, shinfo2->tskey);
> TCP_SKB_CB(skb2)->txstamp_ack = TCP_SKB_CB(skb)->txstamp_ack;
> + TCP_SKB_CB(skb2)->txstamp_ack_bpf = TCP_SKB_CB(skb)->txstamp_ack_bpf;
> TCP_SKB_CB(skb)->txstamp_ack = 0;
> + TCP_SKB_CB(skb)->txstamp_ack_bpf = 0;
> }
> }
>
> @@ -3213,6 +3216,8 @@ void tcp_skb_collapse_tstamp(struct sk_buff *skb,
> shinfo->tskey = next_shinfo->tskey;
> TCP_SKB_CB(skb)->txstamp_ack |=
> TCP_SKB_CB(next_skb)->txstamp_ack;
> + TCP_SKB_CB(skb)->txstamp_ack_bpf |=
> + TCP_SKB_CB(next_skb)->txstamp_ack_bpf;
> }
> }
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 974b7f61d11f..06e68d772989 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -7040,6 +7040,11 @@ enum {
> * timestamp that hardware just
> * generates.
> */
> + BPF_SOCK_OPS_TS_ACK_OPT_CB, /* Called when all the skbs in the
> + * same sendmsg call are acked
> + * when SK_BPF_CB_TX_TIMESTAMPING
> + * feature is on.
> + */
> };
>
> /* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
> --
> 2.43.5
>
Powered by blists - more mailing lists