Re: [xdp-hints] Re: [PATCH iwl-next,v4 1/1] igc: Add Tx hardware timestamp request for AF_XDP zero-copy packet

[Florian Bezdeka]

On Tue, 2024-03-26 at 14:55 +0000, Song, Yoong Siang wrote:
> On Tuesday, March 26, 2024 9:08 PM, Kurt Kanzenbach <kurt@xxxxxxxxxxxxx> 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@xxxxxxxxx/
> 
> Thanks & Regards
> Siang