On Sun, Feb 9, 2025 at 1:54 AM Willem de Bruijn
<willemdebruijn.kernel@xxxxxxxxx> wrote:
>
> Jason Xing wrote:
> > Support the ACK timestamp case. Extend txstamp_ack to two bits:
> > 1 stands for SO_TIMESTAMPING mode, 2 bpf extension. The latter
> > will be used later.
> >
> > Signed-off-by: Jason Xing <kerneljasonxing@xxxxxxxxx>
> > ---
> > include/net/tcp.h | 4 ++--
> > include/uapi/linux/bpf.h | 5 +++++
> > net/core/skbuff.c | 5 ++++-
> > tools/include/uapi/linux/bpf.h | 5 +++++
> > 4 files changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 4c4dca59352b..ef30f3605e04 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -958,10 +958,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? */
> > + __u8 txstamp_ack:2, /* Record TX timestamp for ack? */
> > 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 e71a9b53e7bc..c04e788125a7 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -7044,6 +7044,11 @@ enum {
> > * SK_BPF_CB_TX_TIMESTAMPING feature
> > * is on.
> > */
> > + 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 ca1ba4252ca5..c0f4d6f6583d 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -5549,7 +5549,7 @@ static bool skb_tstamp_tx_report_so_timestamping(struct sk_buff *skb,
> > return skb_shinfo(skb)->tx_flags & (sw ? SKBTX_SW_TSTAMP :
> > SKBTX_HW_TSTAMP_NOBPF);
> > case SCM_TSTAMP_ACK:
> > - return TCP_SKB_CB(skb)->txstamp_ack;
> > + return TCP_SKB_CB(skb)->txstamp_ack == 1;
>
> For the two to coexist, this should be txstamp_ack & 1
>
> And in the patch that introduces the BPF bit, txstamp_ack |= 2, rather than txstamp_ack = 2.
>
> And let's define labels rather than use constants directly:
>
> #define TSTAMP_ACK_SK 0x1
> #define TSTAMP_ACK_BPF 0x2
Thanks. Will do it.
