On 4/25/2024 7:42 AM, Willem de Bruijn wrote: > Abhishek Chauhan wrote: >> tstamp_type is now set based on actual clockid_t compressed >> into 2 bits. >> >> To make the design scalable for future needs this commit bring in >> the change to extend the tstamp_type:1 to tstamp_type:2 to support >> other clockid_t timestamp. >> >> We now support CLOCK_TAI as part of tstamp_type as part of this >> commit with exisiting support CLOCK_MONOTONIC and CLOCK_REALTIME. >> >> Link: https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@xxxxxxxxx/ >> Signed-off-by: Abhishek Chauhan <quic_abchauha@xxxxxxxxxxx> >> --- > >> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h >> index e464d0ebc9c1..3ad0de07d261 100644 >> --- a/include/linux/skbuff.h >> +++ b/include/linux/skbuff.h >> @@ -711,6 +711,8 @@ typedef unsigned char *sk_buff_data_t; >> enum skb_tstamp_type { >> SKB_CLOCK_REALTIME, >> SKB_CLOCK_MONOTONIC, >> + SKB_CLOCK_TAI, >> + __SKB_CLOCK_MAX = SKB_CLOCK_TAI, >> }; >> >> /** >> @@ -831,8 +833,8 @@ enum skb_tstamp_type { >> * @decrypted: Decrypted SKB >> * @slow_gro: state present at GRO time, slower prepare step required >> * @tstamp_type: When set, skb->tstamp has the >> - * delivery_time in mono clock base Otherwise, the >> - * timestamp is considered real clock base. >> + * delivery_time in mono clock base or clock base of skb->tstamp. > > drop "in mono clock base or " > Noted >> + * Otherwise, the timestamp is considered real clock base > Noted > drop this: whenever in realtime clock base, tstamp_type is zero, so > the above shorter statement always holds. > >> * @napi_id: id of the NAPI struct this skb came from >> * @sender_cpu: (aka @napi_id) source CPU in XPS >> * @alloc_cpu: CPU which did the skb allocation. >> @@ -960,7 +962,7 @@ struct sk_buff { >> /* private: */ >> __u8 __mono_tc_offset[0]; >> /* public: */ >> - __u8 tstamp_type:1; /* See skb_tstamp_type */ >> + __u8 tstamp_type:2; /* See skb_tstamp_type */ >> #ifdef CONFIG_NET_XGRESS >> __u8 tc_at_ingress:1; /* See TC_AT_INGRESS_MASK */ >> __u8 tc_skip_classify:1; >> @@ -1090,15 +1092,17 @@ struct sk_buff { >> #endif >> #define PKT_TYPE_OFFSET offsetof(struct sk_buff, __pkt_type_offset) >> >> -/* if you move tc_at_ingress or mono_delivery_time >> +/* if you move tc_at_ingress or tstamp_type:2 >> * around, you also must adapt these constants. >> */ >> #ifdef __BIG_ENDIAN_BITFIELD >> -#define SKB_MONO_DELIVERY_TIME_MASK (1 << 7) >> -#define TC_AT_INGRESS_MASK (1 << 6) >> +#define SKB_TSTAMP_TYPE_MASK (3 << 6) >> +#define SKB_TSTAMP_TYPE_RSH (6) >> +#define TC_AT_INGRESS_RSH (5) > > I had to find BPF_RSH to understand this abbreviation. > > use SHIFT instead of RSH, as that is so domain specific? > Noted! I will use complete words instead of abbreviations >> +#define TC_AT_INGRESS_MASK (1 << 5) >> #else >> -#define SKB_MONO_DELIVERY_TIME_MASK (1 << 0) >> -#define TC_AT_INGRESS_MASK (1 << 1) >> +#define SKB_TSTAMP_TYPE_MASK (3) >> +#define TC_AT_INGRESS_MASK (1 << 2) >> #endif >> #define SKB_BF_MONO_TC_OFFSET offsetof(struct sk_buff, __mono_tc_offset) >> > >> - if (skb->tstamp_type == BPF_SKB_TSTAMP_DELIVERY_MONO) { >> + if (skb->tstamp_type == BPF_SKB_TSTAMP_DELIVERY_MONO || >> + skb->tstamp_type == BPF_SKB_TSTAMP_DELIVERY_TAI) { > > Peculiar indentation? > Let me check why the indentation here is messed up. Ideally i run checkpatch(shows 0 errors or warnings) and also check before raising a patch. Internally it looks good but on the patch it shows differently. > Just FYI that I'm not the best person to review the BPF part. > Thankfully Martin is helping you with that. > I will wait for comments from Martin as well. > >
