netdev - Re: [PATCH bpf-next v9 09/12] bpf: support SCM_TSTAMP_ACK of SO

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL+tcoCdimG6A8eJgOeterDmKM6by6k8_Jro1p6t6BPtVq-psA@mail.gmail.com>
Date: Sun, 9 Feb 2025 07:27:52 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, 
	pabeni@...hat.com, dsahern@...nel.org, 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, bpf@...r.kernel.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v9 09/12] bpf: support SCM_TSTAMP_ACK of SO_TIMESTAMPING

On Sun, Feb 9, 2025 at 1:54 AM Willem de Bruijn
<willemdebruijn.kernel@...il.com> 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@...il.com>
> > ---
> >  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.