netdev - Re: [PATCH bpf-next v4 10/11] selftests/bpf: Add TX side to xdp_hw

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]

Date: Tue, 24 Oct 2023 09:41:36 -0700
From: Stanislav Fomichev <sdf@...gle.com>
To: "Song, Yoong Siang" <yoong.siang.song@...el.com>
Cc: "bpf@...r.kernel.org" <bpf@...r.kernel.org>, "ast@...nel.org" <ast@...nel.org>, 
	"daniel@...earbox.net" <daniel@...earbox.net>, "andrii@...nel.org" <andrii@...nel.org>, 
	"martin.lau@...ux.dev" <martin.lau@...ux.dev>, "song@...nel.org" <song@...nel.org>, "yhs@...com" <yhs@...com>, 
	"john.fastabend@...il.com" <john.fastabend@...il.com>, "kpsingh@...nel.org" <kpsingh@...nel.org>, 
	"haoluo@...gle.com" <haoluo@...gle.com>, "jolsa@...nel.org" <jolsa@...nel.org>, 
	"kuba@...nel.org" <kuba@...nel.org>, "toke@...nel.org" <toke@...nel.org>, 
	"willemb@...gle.com" <willemb@...gle.com>, "dsahern@...nel.org" <dsahern@...nel.org>, 
	"Karlsson, Magnus" <magnus.karlsson@...el.com>, "bjorn@...nel.org" <bjorn@...nel.org>, 
	"Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>, "hawk@...nel.org" <hawk@...nel.org>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, 
	"xdp-hints@...-project.net" <xdp-hints@...-project.net>
Subject: Re: [PATCH bpf-next v4 10/11] selftests/bpf: Add TX side to xdp_hw_metadata

On Mon, Oct 23, 2023 at 7:19 PM Song, Yoong Siang
<yoong.siang.song@...el.com> wrote:
>
> On Friday, October 20, 2023 1:50 AM Stanislav Fomichev <sdf@...gle.com> wrote:
> >When we get a packet on port 9091, we swap src/dst and send it out.
> >At this point we also request the timestamp and checksum offloads.
> >
> >Checksum offload is verified by looking at the tcpdump on the other side.
> >The tool prints pseudo-header csum and the final one it expects.
> >The final checksum actually matches the incoming packets checksum
> >because we only flip the src/dst and don't change the payload.
> >
> >Some other related changes:
> >- switched to zerocopy mode by default; new flag can be used to force
> >  old behavior
> >- request fixed tx_metadata_len headroom
> >- some other small fixes (umem size, fill idx+i, etc)
> >
> >mvbz3:~# ./xdp_hw_metadata eth3
> >...
> >xsk_ring_cons__peek: 1
> >0x19546f8: rx_desc[0]->addr=80100 addr=80100 comp_addr=80100
> >rx_hash: 0x80B7EA8B with RSS type:0x2A
> >rx_timestamp:  1697580171852147395 (sec:1697580171.8521)
> >HW RX-time:   1697580171852147395 (sec:1697580171.8521), delta to User RX-time sec:0.2797 (279673.082 usec)
> >XDP RX-time:   1697580172131699047 (sec:1697580172.1317), delta to User RX-time sec:0.0001 (121.430 usec)
> >0x19546f8: ping-pong with csum=3b8e (want d862) csum_start=54 csum_offset=6
> >0x19546f8: complete tx idx=0 addr=8
> >tx_timestamp:  1697580172056756493 (sec:1697580172.0568)
>
> Hi Stanislav,
>
> rx_timestamp is duplicating HW RX-time while tx_timestamp is duplicating HW TX-complete-time,
> so, I think can remove printing of rx_timestamp and tx_timestamp to avoid confusion.

That's fair, I think I'll do the following:

if (meta->rx_timestamp) {
  /* print all those reference points */
} else {
  printf("No rx_timestamp\n");
}

And the same for tx. So at least the users get a signal that the
timestamps weren't set.

> >HW TX-complete-time:   1697580172056756493 (sec:1697580172.0568), delta to User TX-complete-time sec:0.0852 (85175.537 usec)
> >XDP RX-time:   1697580172131699047 (sec:1697580172.1317), delta to User TX-complete-time sec:0.0102 (10232.983 usec)
> >HW RX-time:   1697580171852147395 (sec:1697580171.8521), delta to HW TX-complete-time sec:0.2046 (204609.098 usec)
> >0x19546f8: complete rx idx=128 addr=80100
> >
> >mvbz4:~# nc  -Nu -q1 ${MVBZ3_LINK_LOCAL_IP}%eth3 9091
> >
> >mvbz4:~# tcpdump -vvx -i eth3 udp
> >        tcpdump: listening on eth3, link-type EN10MB (Ethernet), snapshot length 262144
> >bytes
> >12:26:09.301074 IP6 (flowlabel 0x35fa5, hlim 127, next-header UDP (17) payload
> >length: 11) fe80::1270:fdff:fe48:1087.55807 > fe80::1270:fdff:fe48:1077.9091: [bad
> >udp cksum 0x3b8e -> 0xde7e!] UDP, length 3
> >        0x0000:  6003 5fa5 000b 117f fe80 0000 0000 0000
> >        0x0010:  1270 fdff fe48 1087 fe80 0000 0000 0000
> >        0x0020:  1270 fdff fe48 1077 d9ff 2383 000b 3b8e
> >        0x0030:  7864 70
> >12:26:09.301976 IP6 (flowlabel 0x35fa5, hlim 127, next-header UDP (17) payload
> >length: 11) fe80::1270:fdff:fe48:1077.9091 > fe80::1270:fdff:fe48:1087.55807: [udp
> >sum ok] UDP, length 3
> >        0x0000:  6003 5fa5 000b 117f fe80 0000 0000 0000
> >        0x0010:  1270 fdff fe48 1077 fe80 0000 0000 0000
> >        0x0020:  1270 fdff fe48 1087 2383 d9ff 000b de7e
> >        0x0030:  7864 70
> >
> >This reverts commit c3c9abc1d0c989e0be21d78cccd99076cc94ec44.
>
> It didn't looked like this patch is reverting something.
> If this is not a mistake, can you add the commit title behind the ID?

Ah, that's a leftover from my rebasing and reshuffling, will drop, thanks!