XDP hardware hints discussion mail archive
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: "Maloor, Kishen" <kishen.maloor@intel.com>
Cc: "Zhang, Jessica" <jessica.zhang@intel.com>,
	"Desouza, Ederson" <ederson.desouza@intel.com>,
	"Joseph, Jithu" <jithu.joseph@intel.com>,
	"Gomes, Vinicius" <vinicius.gomes@intel.com>,
	brouer@redhat.com,
	XDP-hints working-group <xdp-hints@xdp-project.net>,
	Magnus Karlsson <magnus.karlsson@gmail.com>,
	BPF-dev-list <bpf@vger.kernel.org>,
	Walfred 'Fred' Tedeschi <walfred.tedeschi@intel.com>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>
Subject: Re: TXTIME (Launch Time) setting per XDP frame from userspace.
Date: Mon, 21 Jun 2021 23:06:35 +0200	[thread overview]
Message-ID: <20210621230635.3a83851c@carbon> (raw)
In-Reply-To: <00301BEA-4A94-4600-998A-BEDCF3330795@intel.com>


(Answer inlined below, please learn howto answer inline)

On Mon, 21 Jun 2021 19:08:54 +0000
"Maloor, Kishen" <kishen.maloor@intel.com> wrote:

> Hi Jesper,
> 
> I wanted to run some patches by you for your comments.

Thanks a lot for reaching out, I really appreciate it.

> Intel has a requirement to support an SO_TXTIME like capability with
> AF_XDP sockets, to set a precise per-packet transmission time from
> user space XDP applications.

This is great!  I also have a customer that have this requirement.
Thus, I'm very interested in collaborating on getting this feature
implemented in practice.


> It is also critical that this support be upstreamed into the Linux
> mainline.

I fully agree "upstream first".  For this to happen, we should discuss
the approach in a public forum as early as possible to get upstream
feedback.  Cc'ing bpf@vger.kernel.org.  Also adding our working group
mailing list as not everybody follow the kernel list Cc'ed
xdp-hints@xdp-project.net (https://lists.xdp-project.net/xdp-hints/).

Multiple of your Intel colleagues (from other departments) have also
reached out to me, and they are also on this xdp-hints mailing list.
It would be great if you can keep this list Cc'ed to keep everybody in
the loop.

Consider subscribing:
 https://lists.xdp-project.net/postorius/lists/xdp-hints.xdp-project.net/


> The implementation is roughly equivalent to that of SO_TXTIME for
> AF_PACKET. It includes:
>
> - a new XSK bind flag,

So, this feature is enabled for all packets on this AF_XDP socket.

> - libbpf API to set a TXTIME for a frame,
>
> - leverages the concept of UMEM headroom to store the per-packet
>   TXTIME (immediately preceding the XDP buff's data_hard_start),

This sounds like the XDP metadata area?
Or is this top of mem data-area (guess not, as you use +umem_headroom)?

What will happen when an XDP-prog use this same metadata area, and your
feature (patch below) is enabled (via AF_XDP) ?

E.g. can I set the TXTIME in XDP-prog (RX) and XDP_REDIRECT out an
interface with TXTIME enabled?


> - internally signals the NIC driver via the XDP descriptor when
>   there's a TXTIME bound to a frame, and

Looking at patch, if I don't call 'xsk_umem__set_txtime' (libbpf), then
the kernel (xp_raw_get_txtime) will pickup random-data left over by
previous user of the memory, right?

> - provides a kernel API to retrieve it in the driver so it may proceed
>   to set the Launch Time on the NIC.
> 
> It uses existing bitmasks and there's no overhead (i.e. not
> allocating any more memory than is already done), and there are no
> other dependencies.
>
> You may review the commit messages for all details.
> 
> kernel: https://github.com/kmaloor/bpfnext/commit/b72a336bcb8df4a26646a29962ca95d58172147f
> libbpf: https://github.com/kmaloor/libbpf/commit/eda68c1ad11ed60739837ad6e4fb256768231b55
> 
> We're currently testing these changes in conjunction with the igc
> driver.

Can you share the patches for the igc driver as well?

I want to see how you convert the "__s64 txtime" into the 30-bit
LaunchTime used[0] by the hardware (i225).  It would also be good to see
how this is used for driver igb/i210, that have a 25-bit LaunchTime.

Details on driver+hardware here: 
 [0] https://github.com/xdp-project/xdp-project/blob/master/areas/tsn/code01_follow_qdisc_TSN_offload.org


> Meanwhile, we'd appreciate any comments you may have, particularly as
> an expert with the kernel XDP subsystem.

I think this TXTIME work is just *one* use-case of "xdp-hints".  IMHO we
should use something like BTF to describe the metadata memory area
in-order to support more use-cases.

Being concrete .e.g. your XSK socket bind call could register a BTF-ID
that it tells the kernel about the layout. I guess, we need some
guidance from BTF experts on how kernel can check the driver supports
this LaunchTime feature, Andrii?

Other use-cases: (1) On TX we also want to support asking for TX-csum
by hardware. (2) Support setting VLAN via TX-descriptor (hint TSN can
use VLAN priority bits).

Again thanks for reaching out, lets find a solution together with
upstream.
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Kernel:

 s64 xp_raw_get_txtime(struct xsk_buff_pool *pool, u64 addr)
{
	return *(s64 *)((char *)xp_raw_get_data(pool, addr) - XDP_TXTIME_LEN);
}
EXPORT_SYMBOL(xp_raw_get_txtime);

libbpf:

 static inline void xsk_umem__set_txtime(void *umem_area, __u64 addr, __u32 umem_headroom, __s64 txtime)
{
	*(__s64 *)(&((char *)umem_area)[addr] + umem_headroom - XDP_TXTIME_LEN) = txtime;
}


       reply	other threads:[~2021-06-21 21:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <00301BEA-4A94-4600-998A-BEDCF3330795@intel.com>
2021-06-21 21:06 ` Jesper Dangaard Brouer [this message]
2021-06-21 23:04   ` Maloor, Kishen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.xdp-project.net/postorius/lists/xdp-hints.xdp-project.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210621230635.3a83851c@carbon \
    --to=brouer@redhat.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=ederson.desouza@intel.com \
    --cc=jessica.zhang@intel.com \
    --cc=jithu.joseph@intel.com \
    --cc=kishen.maloor@intel.com \
    --cc=magnus.karlsson@gmail.com \
    --cc=vinicius.gomes@intel.com \
    --cc=walfred.tedeschi@intel.com \
    --cc=xdp-hints@xdp-project.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox