On 5/6/2024 12:04 PM, Willem de Bruijn wrote: > Abhishek Chauhan wrote: >> With changes in the design to forward CLOCK_TAI in the skbuff >> framework, existing selftest framework needs modification >> to handle forwarding of UDP packets with CLOCK_TAI as clockid. >> >> Link: https://lore.kernel.org/netdev/bc037db4-58bb-4861-ac31-a361a93841d3@xxxxxxxxx/ >> Signed-off-by: Abhishek Chauhan <quic_abchauha@xxxxxxxxxxx> >> --- >> tools/include/uapi/linux/bpf.h | 15 ++++--- >> .../selftests/bpf/prog_tests/ctx_rewrite.c | 10 +++-- >> .../selftests/bpf/prog_tests/tc_redirect.c | 3 -- >> .../selftests/bpf/progs/test_tc_dtime.c | 39 +++++++++---------- >> 4 files changed, 34 insertions(+), 33 deletions(-) >> >> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h >> index 90706a47f6ff..25ea393cf084 100644 >> --- a/tools/include/uapi/linux/bpf.h >> +++ b/tools/include/uapi/linux/bpf.h >> @@ -6207,12 +6207,17 @@ union { \ >> __u64 :64; \ >> } __attribute__((aligned(8))) >> >> +/* The enum used in skb->tstamp_type. It specifies the clock type >> + * of the time stored in the skb->tstamp. >> + */ >> enum { >> - BPF_SKB_TSTAMP_UNSPEC, >> - BPF_SKB_TSTAMP_DELIVERY_MONO, /* tstamp has mono delivery time */ >> - /* For any BPF_SKB_TSTAMP_* that the bpf prog cannot handle, >> - * the bpf prog should handle it like BPF_SKB_TSTAMP_UNSPEC >> - * and try to deduce it by ingress, egress or skb->sk->sk_clockid. >> + BPF_SKB_TSTAMP_UNSPEC = 0, /* DEPRECATED */ >> + BPF_SKB_TSTAMP_DELIVERY_MONO = 1, /* DEPRECATED */ >> + BPF_SKB_CLOCK_REALTIME = 0, >> + BPF_SKB_CLOCK_MONOTONIC = 1, >> + BPF_SKB_CLOCK_TAI = 2, >> + /* For any future BPF_SKB_CLOCK_* that the bpf prog cannot handle, >> + * the bpf prog can try to deduce it by ingress/egress/skb->sk->sk_clockid. >> */ >> }; >> >> diff --git a/tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c b/tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c >> index 3b7c57fe55a5..71940f4ef0fb 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c >> +++ b/tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c >> @@ -69,15 +69,17 @@ static struct test_case test_cases[] = { >> { >> N(SCHED_CLS, struct __sk_buff, tstamp), >> .read = "r11 = *(u8 *)($ctx + sk_buff::__mono_tc_offset);" >> - "w11 &= 3;" >> - "if w11 != 0x3 goto pc+2;" >> + "if w11 == 0x4 goto pc+1;" >> + "goto pc+4;" >> + "if w11 == 0x3 goto pc+1;" >> + "goto pc+2;" > > Not an expert on this code, and I see that the existing code already > has this below, but: isn't it odd and unnecessary to jump to an > unconditional jump statement? > I am closely looking into your comment and i will evalute it(Martin can correct me if the jumps are correct or not as i am new to BPF as well) but i found out that JSET = "&" and not "==". So the above two ins has to change from - "if w11 == 0x4 goto pc+1;" ==>(needs to be corrected to) "if w11 & 0x4 goto pc+1;" "if w11 == 0x3 goto pc+1;" ==> (needs to be correct to) "if w11 & 0x3 goto pc+1;" >> "$dst = 0;" >> "goto pc+1;" >> "$dst = *(u64 *)($ctx + sk_buff::tstamp);", >> .write = "r11 = *(u8 *)($ctx + sk_buff::__mono_tc_offset);" >> - "if w11 & 0x2 goto pc+1;" >> + "if w11 & 0x4 goto pc+1;" >> "goto pc+2;" >> - "w11 &= -2;" >> + "w11 &= -3;" Martin, Also i am not sure why the the dissembly complains because the value of SKB_TSTAMP_TYPE_MASK = 3 and we are negating it ~3 = -3. Can't match disassembly(left) with pattern(right): r11 = *(u8 *)(r1 +129) ; r11 = *(u8 *)($ctx + sk_buff::__mono_tc_offset) if w11 & 0x4 goto pc+1 ; if w11 & 0x4 goto pc+1 goto pc+2 ; goto pc+2 w11 &= -4 ; w11 &= -3 >> "*(u8 *)($ctx + sk_buff::__mono_tc_offset) = r11;" >> "*(u64 *)($ctx + sk_buff::tstamp) = $src;", >> },
