XDP hardware hints discussion mail archive
 help / color / mirror / Atom feed
From: Magnus Karlsson <magnus.karlsson@gmail.com>
To: Jesper Dangaard Brouer <jbrouer@redhat.com>
Cc: brouer@redhat.com, Maryam Tahhan <mtahhan@redhat.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org,
	xdp-hints@xdp-project.net, larysa.zaremba@intel.com,
	memxor@gmail.com, Lorenzo Bianconi <lorenzo@kernel.org>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Daniel Borkmann <borkmann@iogearbox.net>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	dave@dtucker.co.uk, Magnus Karlsson <magnus.karlsson@intel.com>,
	bjorn@kernel.org, Alexander Lobakin <alexandr.lobakin@intel.com>
Subject: [xdp-hints] Re: [PATCH RFCv2 bpf-next 17/18] xsk: AF_XDP xdp-hints support in desc options
Date: Fri, 9 Sep 2022 14:44:37 +0200	[thread overview]
Message-ID: <CAJ8uoz01z8=RBPHo1zy-ZPDDX_fmuMFOraU4o0R5e0QLrWDsyQ@mail.gmail.com> (raw)
In-Reply-To: <51f40ca1-ccf2-bcc3-d20d-931ad0d22526@redhat.com>

On Fri, Sep 9, 2022 at 2:35 PM Jesper Dangaard Brouer
<jbrouer@redhat.com> wrote:
>
>
>
> On 09/09/2022 12.14, Magnus Karlsson wrote:
> > On Fri, Sep 9, 2022 at 11:42 AM Jesper Dangaard Brouer
> > <jbrouer@redhat.com> wrote:
> >>
> >>
> >> On 09/09/2022 10.12, Maryam Tahhan wrote:
> >>> <snip>
> >>>>>>>
> >>>>>>> * Instead encode this information into each metadata entry in the
> >>>>>>> metadata area, in some way so that a flags field is not needed (-1
> >>>>>>> signifies not valid, or whatever happens to make sense). This has the
> >>>>>>> drawback that the user might have to look at a large number of entries
> >>>>>>> just to find out there is nothing valid to read. To alleviate this, it
> >>>>>>> could be combined with the next suggestion.
> >>>>>>>
> >>>>>>> * Dedicate one bit in the options field to indicate that there is at
> >>>>>>> least one valid metadata entry in the metadata area. This could be
> >>>>>>> combined with the two approaches above. However, depending on what
> >>>>>>> metadata you have enabled, this bit might be pointless. If some
> >>>>>>> metadata is always valid, then it serves no purpose. But it might if
> >>>>>>> all enabled metadata is rarely valid, e.g., if you get an Rx timestamp
> >>>>>>> on one packet out of one thousand.
> >>>>>>>
> >>>>>
> >>>>> I like this option better! Except that I have hoped to get 2 bits ;-)
> >>>>
> >>>> I will give you two if you need it Jesper, no problem :-).
> >>>>
> >>>
> >>> Ok I will look at implementing and testing this and post an update.
> >>
> >> Perfect if you Maryam have cycles to work on this.
> >>
> >> Let me explain what I wanted the 2nd bit for.  I simply wanted to also
> >> transfer the XDP_FLAGS_HINTS_COMPAT_COMMON flag.  One could argue that
> >> is it redundant information as userspace AF_XDP will have to BTF decode
> >> all the know XDP-hints. Thus, it could know if a BTF type ID is
> >> compatible with the common struct.   This problem is performance as my
> >> userspace AF_XDP code will have to do more code (switch/jump-table or
> >> table lookup) to map IDs to common compat (to e.g. extract the RX-csum
> >> indication).  Getting this extra "common-compat" bit is actually a
> >> micro-optimization.  It is up to AF_XDP maintainers if they can spare
> >> this bit.
> >>
> >>
> >>> Thanks folks
> >>>
> >>>>> The performance advantage is that the AF_XDP descriptor bits will
> >>>>> already be cache-hot, and if it indicates no-metadata-hints the AF_XDP
> >>>>> application can avoid reading the metadata cache-line :-).
> >>>>
> >>>> Agreed. I prefer if we can keep it simple and fast like this.
> >>>>
> >>
> >> Great, lets proceed this way then.
> >>
> >>> <snip>
> >>>
> >>
> >> Thinking ahead: We will likely need 3 bits.
> >>
> >> The idea is that for TX-side, we set a bit indicating that AF_XDP have
> >> provided a valid XDP-hints layout (incl corresponding BTF ID). (I would
> >> overload and reuse "common-compat" bit if TX gets a common struct).
> >
> > I think we should reuse the "Rx metadata valid" flag for this since
> > this will not be used in the Tx case by definition. In the Tx case,
> > this bit would instead mean that the user has provided a valid
> > XDP-hints layout. It has a nice symmetry, on Rx it is set by the
> > kernel when it has put something relevant in the metadata area. On Tx,
> > it is set by user-space if it has put something relevant in the
> > metadata area.
>
> I generally like reusing the bit, *BUT* there is the problem of
> (existing) applications ignoring the desc-options bit and forwarding
> packets.  This would cause the "Rx metadata valid" flag to be seen as
> userspace having set the "TX-hints-bit" and kernel would use what is
> provided in metadata area (leftovers from RX-hints).  IMHO that will be
> hard to debug for end-users and likely break existing applications.

Good point. I buy this. We need separate Rx and Tx bits.

> > We can also reuse this bit when we get a notification
> > in the completion queue to indicate if the kernel has produced some
> > metadata on tx completions. This could be a Tx timestamp for example.
> >
>
> Big YES, reuse "Rx metadata valid" bit when we get a TX notification in
> completion queue.  This will be okay because it cannot be forgotten and
> misinterpreted as the kernel will have responsibility to update this bit.
>
> > So hopefully we could live with only two bits :-).
> >
>
> I still think we need three bits ;-)
> That should be enough to cover the 6 states:
>   - RX hints
>   - RX hints and compat
>   - TX hints
>   - TX hints and compat
>   - TX completion
>   - TX completion and compat
>
>
> >> But lets land RX-side first, but make sure we can easily extend for the
> >> TX-side.
>

  reply	other threads:[~2022-09-09 12:44 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-07 15:45 [xdp-hints] [PATCH RFCv2 bpf-next 00/18] XDP-hints: XDP gaining access to HW offload hints via BTF Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 01/18] libbpf: factor out BTF loading from load_module_btfs() Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 02/18] libbpf: try to load vmlinux BTF from the kernel first Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 03/18] libbpf: patch module BTF obj+type ID into BPF insns Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 04/18] net: create xdp_hints_common and set functions Jesper Dangaard Brouer
2022-09-09 10:49   ` [xdp-hints] " Burakov, Anatoly
2022-09-09 14:13     ` Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 05/18] net: add net_device feature flag for XDP-hints Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 06/18] xdp: controlling XDP-hints from BPF-prog via helper Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 07/18] i40e: Refactor i40e_ptp_rx_hwtstamp Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 08/18] i40e: refactor i40e_rx_checksum with helper Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 09/18] bpf: export btf functions for modules Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 10/18] btf: Add helper for kernel modules to lookup full BTF ID Jesper Dangaard Brouer
2022-09-07 15:45 ` [xdp-hints] [PATCH RFCv2 bpf-next 11/18] i40e: add XDP-hints handling Jesper Dangaard Brouer
2022-09-07 15:46 ` [xdp-hints] [PATCH RFCv2 bpf-next 12/18] net: use XDP-hints in xdp_frame to SKB conversion Jesper Dangaard Brouer
2022-09-07 15:46 ` [xdp-hints] [PATCH RFCv2 bpf-next 13/18] mvneta: add XDP-hints support Jesper Dangaard Brouer
2022-09-07 15:46 ` [xdp-hints] [PATCH RFCv2 bpf-next 14/18] i40e: Add xdp_hints_union Jesper Dangaard Brouer
2022-09-07 15:46 ` [xdp-hints] [PATCH RFCv2 bpf-next 15/18] ixgbe: enable xdp-hints Jesper Dangaard Brouer
2022-09-07 15:46 ` [xdp-hints] [PATCH RFCv2 bpf-next 16/18] ixgbe: add rx timestamp xdp hints support Jesper Dangaard Brouer
2022-09-07 15:46 ` [xdp-hints] [PATCH RFCv2 bpf-next 17/18] xsk: AF_XDP xdp-hints support in desc options Jesper Dangaard Brouer
2022-09-08  8:06   ` [xdp-hints] " Magnus Karlsson
2022-09-08 10:10     ` Maryam Tahhan
2022-09-08 15:04       ` Jesper Dangaard Brouer
2022-09-09  6:43         ` Magnus Karlsson
2022-09-09  8:12           ` Maryam Tahhan
2022-09-09  9:42             ` Jesper Dangaard Brouer
2022-09-09 10:14               ` Magnus Karlsson
2022-09-09 12:35                 ` Jesper Dangaard Brouer
2022-09-09 12:44                   ` Magnus Karlsson [this message]
2022-09-07 15:46 ` [xdp-hints] [PATCH RFCv2 bpf-next 18/18] ixgbe: AF_XDP xdp-hints processing in ixgbe_clean_rx_irq_zc Jesper Dangaard Brouer
2022-09-08  9:30 ` [xdp-hints] Re: [PATCH RFCv2 bpf-next 00/18] XDP-hints: XDP gaining access to HW offload hints via BTF Alexander Lobakin
2022-09-09 13:48   ` Jesper Dangaard Brouer
2022-10-03 23:55 ` sdf
2022-10-04  9:29   ` Jesper Dangaard Brouer
2022-10-04 18:26     ` Stanislav Fomichev
2022-10-05  0:25       ` Martin KaFai Lau
2022-10-05  0:59         ` Jakub Kicinski
2022-10-05  1:02           ` Stanislav Fomichev
2022-10-05  1:24             ` Jakub Kicinski
2022-10-05  2:15               ` Stanislav Fomichev
2022-10-05 19:26                 ` Martin KaFai Lau
2022-10-06  9:14                   ` Magnus Karlsson
2022-10-06 15:29                     ` Jesper Dangaard Brouer
2022-10-11  6:29                       ` Martin KaFai Lau
2022-10-11 11:57                         ` Jesper Dangaard Brouer
2022-10-05 10:06             ` Toke Høiland-Jørgensen
2022-10-05 18:47               ` sdf
2022-10-06  8:19                 ` Maryam Tahhan
2022-10-06 17:22                   ` sdf
2022-10-05 14:19             ` Jesper Dangaard Brouer
2022-10-06 14:59               ` Jakub Kicinski
2022-10-05 13:43         ` Jesper Dangaard Brouer
2022-10-05 16:29       ` Jesper Dangaard Brouer
2022-10-05 18:43         ` sdf
2022-10-06 17:47           ` Jesper Dangaard Brouer
2022-10-07 15:05             ` David Ahern
2022-10-05 13:14     ` Burakov, Anatoly

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='CAJ8uoz01z8=RBPHo1zy-ZPDDX_fmuMFOraU4o0R5e0QLrWDsyQ@mail.gmail.com' \
    --to=magnus.karlsson@gmail.com \
    --cc=alexandr.lobakin@intel.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=bjorn@kernel.org \
    --cc=borkmann@iogearbox.net \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=dave@dtucker.co.uk \
    --cc=jbrouer@redhat.com \
    --cc=larysa.zaremba@intel.com \
    --cc=lorenzo@kernel.org \
    --cc=magnus.karlsson@intel.com \
    --cc=memxor@gmail.com \
    --cc=mtahhan@redhat.com \
    --cc=netdev@vger.kernel.org \
    --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