Re: [RFC bpf-next v2 06/11] net: veth: Implement devtx timestamp kfuncs

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Stanislav Fomichev <sdf@google.com> writes:

> On Mon, Jun 26, 2023 at 3:00 PM Vinicius Costa Gomes
> <vinicius.gomes@intel.com> wrote:
>>
>> Stanislav Fomichev <sdf@google.com> writes:
>>
>> > On Fri, Jun 23, 2023 at 4:29 PM Vinicius Costa Gomes
>> > <vinicius.gomes@intel.com> wrote:
>> >>
>> >> Stanislav Fomichev <sdf@google.com> writes:
>> >>
>> >> > Have a software-based example for kfuncs to showcase how it
>> >> > can be used in the real devices and to have something to
>> >> > test against in the selftests.
>> >> >
>> >> > Both path (skb & xdp) are covered. Only the skb path is really
>> >> > tested though.
>> >> >
>> >> > Cc: netdev@vger.kernel.org
>> >> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
>> >>
>> >> Not really related to this patch, but to how it would work with
>> >> different drivers/hardware.
>> >>
>> >> In some of our hardware (the ones handled by igc/igb, for example), the
>> >> timestamp notification comes some time after the transmit completion
>> >> event.
>> >>
>> >> From what I could gather, the idea would be for the driver to "hold" the
>> >> completion until the timestamp is ready and then signal the completion
>> >> of the frame. Is that right?
>> >
>> > Yeah, that might be the option. Do you think it could work?
>> >
>>
>> For the skb and XDP cases, yeah, just holding the completion for a while
>> seems like it's going to work.
>>
>> XDP ZC looks more complicated to me, not sure if it's only a matter of
>> adding something like:
>
> [..]
>
>> void xsk_tx_completed_one(struct xsk_buff_pool *pool, struct xdp_buff *xdp);
>>
>> Or if more changes would be needed. I am trying to think about the case
>> that the user sent a single "timestamp" packet among a bunch of
>> "non-timestamp" packets.
>
> Since you're passing xdp_buff as an argument I'm assuming that is
> suggesting out-of-order completions?
> The completion queue is a single index, we can't do ooo stuff.
> So you'd have to hold a bunch of packets until you receive the
> timestamp completion; after this event, you can complete the whole
> batch (1 packet waiting for the timestamp + a bunch that have been
> transmitted afterwards but were still unacknowleged in the queue).
>
> (lmk if I've misinterpreted)

Not at all, it was me that wasn't aware that out-of-order completions
were out of the picture.

So, yeah, what you are proposing, accumulating the pending completions
while there's a pending timestamp request, seems the only way to go.

The logic seems simple enough, but the fact that the "timestamp ready"
interrupt is not associated with any queue seems that it will make
things a bit "interesting" to get it right :-) 

But I don't have any better suggestions.


Thank you,
-- 
Vinicius