netdev - Re: [xdp-hints] Re: [PATCH bpf-next v6 4/4] igc: Add launch time support to XDP ZC

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: <87bjvwqvtl.fsf@toke.dk>
Date: Fri, 24 Jan 2025 12:45:42 +0100
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Stanislav Fomichev <stfomichev@...il.com>, Florian Bezdeka
 <florian.bezdeka@...mens.com>
Cc: "Song, Yoong Siang" <yoong.siang.song@...el.com>, "Bouska, Zdenek"
 <zdenek.bouska@...mens.com>, "David S . Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Willem
 de Bruijn <willemb@...gle.com>, Donald Hunter <donald.hunter@...il.com>,
 Jonathan Corbet <corbet@....net>, Bjorn Topel <bjorn@...nel.org>,
 "Karlsson, Magnus" <magnus.karlsson@...el.com>, "Fijalkowski, Maciej"
 <maciej.fijalkowski@...el.com>, Jonathan Lemon <jonathan.lemon@...il.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Jesper Dangaard Brouer
 <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>, "Damato,
 Joe" <jdamato@...tly.com>, Stanislav Fomichev <sdf@...ichev.me>, Xuan Zhuo
 <xuanzhuo@...ux.alibaba.com>, Mina Almasry <almasrymina@...gle.com>,
 Daniel Jurgens <danielj@...dia.com>, Andrii Nakryiko <andrii@...nel.org>,
 Eduard Zingerman <eddyz87@...il.com>, Mykola Lysenko <mykolal@...com>,
 Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>,
 Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, Shuah Khan
 <shuah@...nel.org>, Alexandre Torgue <alexandre.torgue@...s.st.com>, Jose
 Abreu <joabreu@...opsys.com>, Maxime Coquelin <mcoquelin.stm32@...il.com>,
 "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
 <przemyslaw.kitszel@...el.com>, "netdev@...r.kernel.org"
 <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "linux-doc@...r.kernel.org"
 <linux-doc@...r.kernel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
 "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
 "linux-stm32@...md-mailman.stormreply.com"
 <linux-stm32@...md-mailman.stormreply.com>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>, "intel-wired-lan@...ts.osuosl.org"
 <intel-wired-lan@...ts.osuosl.org>, "xdp-hints@...-project.net"
 <xdp-hints@...-project.net>
Subject: Re: [xdp-hints] Re: [PATCH bpf-next v6 4/4] igc: Add launch time
 support to XDP ZC

Stanislav Fomichev <stfomichev@...il.com> writes:

> On 01/23, Florian Bezdeka wrote:
>> Hi all,
>> 
>> On Thu, 2025-01-23 at 16:41 +0000, Song, Yoong Siang wrote:
>> > On Thursday, January 23, 2025 11:40 PM, Bouska, Zdenek <zdenek.bouska@...mens.com> wrote:
>> > > 
>> > > Hi Siang,
>> > > 
>> > > I tested this patch series on 6.13 with Intel I226-LM (rev 04).
>> > > 
>> > > I also applied patch "selftests/bpf: Actuate tx_metadata_len in xdp_hw_metadata" [1]
>> > > and "selftests/bpf: Enable Tx hwtstamp in xdp_hw_metadata" [2] so that TX timestamps
>> > > work.
>> > > 
>> > > HW RX-timestamp was small (0.5956 instead of 1737373125.5956):
>> > > 
>> > > HW RX-time:   595572448 (sec:0.5956) delta to User RX-time sec:1737373124.9873 (1737373124987318.750 usec)
>> > > XDP RX-time:   1737373125582798388 (sec:1737373125.5828) delta to User RX-time sec:0.0001 (92.733 usec)
>> > > 
>> > > Igc's raw HW RX-timestamp in front of frame data was overwritten by BPF program on
>> > > line 90 in tools/testing/selftests/bpf: meta->hint_valid = 0;
>> > > 
>> > > "HW timestamp has been copied into local variable" comment is outdated on
>> > > line 2813 in drivers/net/ethernet/intel/igc/igc_main.c after
>> > > commit 069b142f5819 igc: Add support for PTP .getcyclesx64() [3].
>> > > 
>> > > Workaround is to add unused data to xdp_meta struct:
>> > > 
>> > > --- a/tools/testing/selftests/bpf/xdp_metadata.h
>> > > +++ b/tools/testing/selftests/bpf/xdp_metadata.h
>> > > @@ -49,4 +49,5 @@ struct xdp_meta {
>> > >                __s32 rx_vlan_tag_err;
>> > >        };
>> > >        enum xdp_meta_field hint_valid;
>> > > +       __u8 avoid_IGC_TS_HDR_LEN[16];
>> > > };
>> > > 
>> > 
>> > Hi Zdenek Bouska, 
>> > 
>> > Thanks for your help on testing this patch set.
>> > You are right, there is some issue with the Rx hw timestamp,
>> > I will submit the bug fix patch when the solution is finalized,
>> > but the fix will not be part of this launch time patch set.
>> > Until then, you can continue to use your WA.
>> 
>> I think there is no simple fix for that. That needs some discussion
>> around the "expectations" to the headroom / meta data area in front of
>> the actual packet data.
>
> By 'simple' you mean without some new UAPI to signal the size of that
> 'reserved area' by the driver? I don't see any other easy way out as well :-/

Yeah, I don't think we can impose UAPI restrictions on the metadata area
at this point. I guess the best we can do is to educate users that they
should call the timestamp kfunc before they modify the metadata?

-Toke