netdev - Re: [xdp-hints] Re: [PATCH iwl-next,v4 1/1] igc: Add Tx hardware timestamp request for AF

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] [thread-next>] [day] [month] [year] [list]

Date: Wed, 27 Mar 2024 12:21:01 +0100
From: Florian Bezdeka <florian.bezdeka@...mens.com>
To: "Song, Yoong Siang" <yoong.siang.song@...el.com>, Kurt Kanzenbach
 <kurt@...utronix.de>, "Brandeburg, Jesse" <jesse.brandeburg@...el.com>, 
 "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "David S . Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Richard Cochran
 <richardcochran@...il.com>, Alexei Starovoitov <ast@...nel.org>, Daniel
 Borkmann <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>,
 John Fastabend <john.fastabend@...il.com>, Stanislav Fomichev
 <sdf@...gle.com>, "Gomes, Vinicius" <vinicius.gomes@...el.com>,
 "Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>
Cc: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>, 
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "bpf@...r.kernel.org" <bpf@...r.kernel.org>,  "xdp-hints@...-project.net"
 <xdp-hints@...-project.net>
Subject: Re: [xdp-hints] Re: [PATCH iwl-next,v4 1/1] igc: Add Tx hardware
 timestamp request for AF_XDP zero-copy packet

On Tue, 2024-03-26 at 14:55 +0000, Song, Yoong Siang wrote:
> On Tuesday, March 26, 2024 9:08 PM, Kurt Kanzenbach <kurt@...utronix.de> wrote:
> > Hi Florian,
> > 
> > On Tue Mar 26 2024, Florian Bezdeka wrote:
> > > On Mon, 2024-03-25 at 10:09 +0800, Song Yoong Siang wrote:
> > > > This patch adds support to per-packet Tx hardware timestamp request to
> > > > AF_XDP zero-copy packet via XDP Tx metadata framework. Please note that
> > > > user needs to enable Tx HW timestamp capability via igc_ioctl() with
> > > > SIOCSHWTSTAMP cmd before sending xsk Tx hardware timestamp request.
> > > > 
> > > > Same as implementation in RX timestamp XDP hints kfunc metadata, Timer 0
> > > > (adjustable clock) is used in xsk Tx hardware timestamp. i225/i226 have
> > > > four sets of timestamping registers. *skb and *xsk_tx_buffer pointers
> > > > are used to indicate whether the timestamping register is already occupied.
> > > 
> > > Let me make sure that I fully understand that: In my own words:
> > > 
> > > With that applied I'm able to get the point in time from the device
> > > when a specific frame made it to the wire. I have to enable that
> > > functionality using the mentioned ioctl() call first, and then check
> > > the meta area (located in the umem right before the frame payload)
> > > while consuming the completion queue/ring. Correct?
> 
> Hi Florian,
> 
> Yes, you are right. But before you pass the frame to driver, make sure
> you request Tx metadata hardware timestamp feature by setting
> XDP_TXMD_FLAGS_TIMESTAMP flag.
> You can refer to tools/testing/selftests/bpf/xdp_hw_metadata.c
> on how to do it. 

Got it. Thanks!

> 
> > > 
> > > If so, we now have a feedback channel for meta information for/from TX.
> > > Are there any plans - or would it be possible - to support Earliest
> > > TxTime First (NET_SCHED_ETF) QDisc based on that channel? In the past
> > > we had the problem that we we're missing a feedback channel to
> > > communicate back invalid lunch times.
> > 
> > Just asking: How would that work? AFAIK XDP bypasses the Qdisc
> > layer. Currently invalid Launch Times are accounted in the ETF Qdisc
> > itself. Does that mean every driver has to take care of it?
> > 
> > Thanks,
> > Kurt
> 
> Florian & Kurt,
> 
> Yes, me and Stanislav are trying to add Earliest TxTime First / Launch Time to the framework.
> Please refer to [1] for the patchset. The metadata framework will just pass the
> Launch time value to driver, and driver need to handle the rest.
> In the patchset, I am enabling it on stmmac driver only, but we need more drivers
> to check whether the design is feasible for different drivers, cause each driver is
> having their own limitation on launch time. Therefore, after this tx hwts patch accepted,
> I will try to enable launch time on igc driver, and submit new version. 

Nice to hear! Keep me in the loop and let me know if I could support
somehow.

> 
> Kurt is right that current metadata framework is lacking a way to feedback whether
> the launch time is invalid or not. Maybe we can try to enable launch time without feedback,
> then discuss about the status report design.

In case the launch time is invalid - couldn't we simply skip the frame
and "forward" it back to the application (completion queue/ring) after
adjusting some meta-information (like the TX timestamps in this patch)
telling the application what happened?

Thanks a lot!
Florian

> 
> [1] https://patchwork.kernel.org/project/netdevbpf/cover/20231203165129.1740512-1-yoong.siang.song@intel.com/
> 
> Thanks & Regards
> Siang