[xdp-hints] Re: [RFC bpf-next v2 10/14] ice: Support rx timestamp metadata for xdp

[Stanislav Fomichev <sdf@google.com>]

On Fri, Nov 4, 2022 at 7:38 AM Alexander Lobakin
<alexandr.lobakin@intel.com> wrote:
>
> From: Stanislav Fomichev <sdf@google.com>
> Date: Thu,3 Nov 2022 20:25:28 -0700
>
> > COMPILE-TESTED ONLY!
> >
> > Cc: John Fastabend <john.fastabend@gmail.com>
> > Cc: David Ahern <dsahern@gmail.com>
> > Cc: Martin KaFai Lau <martin.lau@linux.dev>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Willem de Bruijn <willemb@google.com>
> > Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> > Cc: Anatoly Burakov <anatoly.burakov@intel.com>
> > Cc: Alexander Lobakin <alexandr.lobakin@intel.com>
> > Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
> > Cc: Maryam Tahhan <mtahhan@redhat.com>
> > Cc: xdp-hints@xdp-project.net
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice.h      |  5 ++
> >  drivers/net/ethernet/intel/ice/ice_main.c |  1 +
> >  drivers/net/ethernet/intel/ice/ice_txrx.c | 75 +++++++++++++++++++++++
> >  3 files changed, 81 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> > index f88ee051e71c..c51a392d64a4 100644
> > --- a/drivers/net/ethernet/intel/ice/ice.h
> > +++ b/drivers/net/ethernet/intel/ice/ice.h
> > @@ -925,6 +925,11 @@ int ice_open_internal(struct net_device *netdev);
> >  int ice_stop(struct net_device *netdev);
> >  void ice_service_task_schedule(struct ice_pf *pf);
> >
> > +struct bpf_insn;
> > +struct bpf_patch;
> > +void ice_unroll_kfunc(const struct bpf_prog *prog, u32 func_id,
> > +                   struct bpf_patch *patch);
> > +
> >  /**
> >   * ice_set_rdma_cap - enable RDMA support
> >   * @pf: PF struct
> > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> > index 1f27dc20b4f1..8ddc6851ef86 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > @@ -9109,4 +9109,5 @@ static const struct net_device_ops ice_netdev_ops = {
> >       .ndo_xdp_xmit = ice_xdp_xmit,
> >       .ndo_xsk_wakeup = ice_xsk_wakeup,
> >       .ndo_get_devlink_port = ice_get_devlink_port,
> > +     .ndo_unroll_kfunc = ice_unroll_kfunc,
> >  };
> > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
> > index 1b6afa168501..e9b5e883753e 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/netdevice.h>
> >  #include <linux/prefetch.h>
> >  #include <linux/bpf_trace.h>
> > +#include <linux/bpf_patch.h>
> >  #include <net/dsfield.h>
> >  #include <net/mpls.h>
> >  #include <net/xdp.h>
> > @@ -1098,8 +1099,80 @@ ice_is_non_eop(struct ice_rx_ring *rx_ring, union ice_32b_rx_flex_desc *rx_desc)
> >
> >  struct ice_xdp_buff {
> >       struct xdp_buff xdp;
> > +     struct ice_rx_ring *rx_ring;
> > +     union ice_32b_rx_flex_desc *rx_desc;
> >  };
> >
> > +void ice_unroll_kfunc(const struct bpf_prog *prog, u32 func_id,
> > +                   struct bpf_patch *patch)
> > +{
> > +     if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_EXPORT_TO_SKB)) {
> > +             return xdp_metadata_export_to_skb(prog, patch);
>
> Hey,
>
> FYI, our team wants to write a follow-up patch with ice support
> added, not like a draft, more of a mature code. I'm thinking of
> calling ice C function which would process Rx descriptors from
> that BPF code from the unrolling callback -- otherwise,
> implementing a couple hundred C code lines from ice_txrx_lib.c
> would be a bit too much :D

Sounds good! I would gladly drop all/most of the driver changes for
the non-rfc posting :-)
I'll probably have a mlx4 one because there is a chance I might find
HW, but the rest I'll drop most likely.
(they are here to show how the driver changes might look like, hence
compile-tested only)

Btw, does it make sense to have some small xsk selftest binary that
can be used to test the metadata with the real device?
The one I'm having right now depends on veth/namespaces; having a
similar one for the real hardware to qualify it sounds useful?
Something simple that sets up af_xdp for all queues, divers some
traffic, and exposes to the userspace consumer all the info about
frame metadata...
Or maybe there is something I can reuse already?


> > +     } else if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP_SUPPORTED)) {
> > +             /* return true; */
> > +             bpf_patch_append(patch, BPF_MOV64_IMM(BPF_REG_0, 1));
> > +     } else if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP)) {
>
> [...]
>
> > --
> > 2.38.1.431.g37b22c650d-goog
>
> Thanks,
> Olek