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

[Song, Yoong Siang]

On Wednesday, March 27, 2024 7:21 PM, Florian Bezdeka <florian.bezdeka@xxxxxxxxxxx> wrote:
>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.

Sure, will keep you in loop.
Do you mind to share, which Ethernet driver you are working on?

>
>>
>> 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?

I not sure whether driver level need to do the validation job for launch time,
Or we can just let the NIC behave according to its own design,
whether it want to drop the packet, or transmit the packet immediately, or transmit the packet at maximum supported launch time. 
We can discuss further when I submit new version of [1].

>
>Thanks a lot!
>Florian
>
>>
>> [1]
>https://patchwork.kernel.org/project/netdevbpf/cover/20231203165129.1740512-1-
>yoong.siang.song@xxxxxxxxx/
>>
>> Thanks & Regards
>> Siang