Date:   Mon, 2 Aug 2021 16:42:25 +0200
From:   Lorenzo Bianconi <lorenzo@...nel.org>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Lorenzo Bianconi <lorenzo.bianconi@...hat.com>,
        Lorenzo Bianconi <me@...enzobianconi.net>,
        bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Shay Agroskin <shayagr@...zon.com>,
        john fastabend <john.fastabend@...il.com>,
        David Ahern <dsahern@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Eelco Chaudron <echaudro@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Saeed Mahameed <saeed@...nel.org>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        tirthendu.sarkar@...el.com,
        Toke Høiland-Jørgensen <toke@...hat.com>
Subject: Re: [PATCH v10 bpf-next 00/18] mvneta: introduce XDP multi-buffer
 support

[...]
> > > bpf_xdp_adjust_tail and bpf_xdp_copy helpers have been modified to take into
> > > account xdp multi-buff frames.
> > >
> 
> Seems like your changes are breaking selftests in no-alu32 mode ([0]).
> Please take a look.
> 
>   [0] https://github.com/kernel-patches/bpf/runs/3197530080?check_suite_focus=true

Hi Andrii,

for no-alu32 mode, it seems the verifiers is complaining when the program is
trying to access the packet payload using the updated value for offset:

if (data[offset] != 0xaa) /* marker *
 ...

as double check, if I use the proper constant value for
offset (1480 = 5000 (original offset) - 3520 (buff0 length))
the test if fine. I need to understand the root cause of the issue.

Regards,
Lorenzo

> 
> > > More info about the main idea behind this approach can be found here [1][2].
> > >
> > > Changes since v9:
> > > - introduce bpf_xdp_adjust_data helper and related selftest
> > > - add xdp_frags_size and xdp_frags_tsize fields in skb_shared_info
> > > - introduce xdp_update_skb_shared_info utility routine in ordere to not reset
> > >   frags array in skb_shared_info converting from a xdp_buff/xdp_frame to a skb
> > > - simplify bpf_xdp_copy routine
> > >
> > > Changes since v8:
> > > - add proper dma unmapping if XDP_TX fails on mvneta for a xdp multi-buff
> > > - switch back to skb_shared_info implementation from previous xdp_shared_info
> > >   one
> > > - avoid using a bietfield in xdp_buff/xdp_frame since it introduces performance
> > >   regressions. Tested now on 10G NIC (ixgbe) to verify there are no performance
> > >   penalties for regular codebase
> > > - add bpf_xdp_get_buff_len helper and remove frame_length field in xdp ctx
> > > - add data_len field in skb_shared_info struct
> > > - introduce XDP_FLAGS_FRAGS_PF_MEMALLOC flag
> > >
> > > Changes since v7:
> > > - rebase on top of bpf-next
> > > - fix sparse warnings
> > > - improve comments for frame_length in include/net/xdp.h
> > >
> > > Changes since v6:
> > > - the main difference respect to previous versions is the new approach proposed
> > >   by Eelco to pass full length of the packet to eBPF layer in XDP context
> > > - reintroduce multi-buff support to eBPF kself-tests
> > > - reintroduce multi-buff support to bpf_xdp_adjust_tail helper
> > > - introduce multi-buffer support to bpf_xdp_copy helper
> > > - rebase on top of bpf-next
> > >
> > > Changes since v5:
> > > - rebase on top of bpf-next
> > > - initialize mb bit in xdp_init_buff() and drop per-driver initialization
> > > - drop xdp->mb initialization in xdp_convert_zc_to_xdp_frame()
> > > - postpone introduction of frame_length field in XDP ctx to another series
> > > - minor changes
> > >
> > > Changes since v4:
> > > - rebase ontop of bpf-next
> > > - introduce xdp_shared_info to build xdp multi-buff instead of using the
> > >   skb_shared_info struct
> > > - introduce frame_length in xdp ctx
> > > - drop previous bpf helpers
> > > - fix bpf_xdp_adjust_tail for xdp multi-buff
> > > - introduce xdp multi-buff self-tests for bpf_xdp_adjust_tail
> > > - fix xdp_return_frame_bulk for xdp multi-buff
> > >
> > > Changes since v3:
> > > - rebase ontop of bpf-next
> > > - add patch 10/13 to copy back paged data from a xdp multi-buff frame to
> > >   userspace buffer for xdp multi-buff selftests
> > >
> > > Changes since v2:
> > > - add throughput measurements
> > > - drop bpf_xdp_adjust_mb_header bpf helper
> > > - introduce selftest for xdp multibuffer
> > > - addressed comments on bpf_xdp_get_frags_count
> > > - introduce xdp multi-buff support to cpumaps
> > >
> > > Changes since v1:
> > > - Fix use-after-free in xdp_return_{buff/frame}
> > > - Introduce bpf helpers
> > > - Introduce xdp_mb sample program
> > > - access skb_shared_info->nr_frags only on the last fragment
> > >
> > > Changes since RFC:
> > > - squash multi-buffer bit initialization in a single patch
> > > - add mvneta non-linear XDP buff support for tx side
> > >
> > > [0] https://netdevconf.info/0x14/session.html?talk-the-path-to-tcp-4k-mtu-and-rx-zerocopy
> > > [1] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
> > > [2] https://netdevconf.info/0x14/session.html?tutorial-add-XDP-support-to-a-NIC-driver (XDPmulti-buffers section)
> > >
> > > Eelco Chaudron (3):
> > >   bpf: add multi-buff support to the bpf_xdp_adjust_tail() API
> > >   bpf: add multi-buffer support to xdp copy helpers
> > >   bpf: update xdp_adjust_tail selftest to include multi-buffer
> > >
> > > Lorenzo Bianconi (15):
> > >   net: skbuff: add size metadata to skb_shared_info for xdp
> > >   xdp: introduce flags field in xdp_buff/xdp_frame
> > >   net: mvneta: update mb bit before passing the xdp buffer to eBPF layer
> > >   net: mvneta: simplify mvneta_swbm_add_rx_fragment management
> > >   net: xdp: add xdp_update_skb_shared_info utility routine
> > >   net: marvell: rely on xdp_update_skb_shared_info utility routine
> > >   xdp: add multi-buff support to xdp_return_{buff/frame}
> > >   net: mvneta: add multi buffer support to XDP_TX
> > >   net: mvneta: enable jumbo frames for XDP
> > >   bpf: introduce bpf_xdp_get_buff_len helper
> > >   bpf: move user_size out of bpf_test_init
> > >   bpf: introduce multibuff support to bpf_prog_test_run_xdp()
> > >   bpf: test_run: add xdp_shared_info pointer in bpf_test_finish
> > >     signature
> > >   net: xdp: introduce bpf_xdp_adjust_data helper
> > >   bpf: add bpf_xdp_adjust_data selftest
> > >
> > >  drivers/net/ethernet/marvell/mvneta.c         | 213 ++++++++++--------
> > >  include/linux/skbuff.h                        |   6 +-
> > >  include/net/xdp.h                             |  95 +++++++-
> > >  include/uapi/linux/bpf.h                      |  38 ++++
> > >  kernel/trace/bpf_trace.c                      |   3 +
> > >  net/bpf/test_run.c                            | 117 ++++++++--
> > >  net/core/filter.c                             | 210 ++++++++++++++++-
> > >  net/core/xdp.c                                |  76 ++++++-
> > >  tools/include/uapi/linux/bpf.h                |  38 ++++
> > >  .../bpf/prog_tests/xdp_adjust_data.c          |  55 +++++
> > >  .../bpf/prog_tests/xdp_adjust_tail.c          | 118 ++++++++++
> > >  .../selftests/bpf/prog_tests/xdp_bpf2bpf.c    | 151 +++++++++----
> > >  .../bpf/progs/test_xdp_adjust_tail_grow.c     |  10 +-
> > >  .../bpf/progs/test_xdp_adjust_tail_shrink.c   |  32 ++-
> > >  .../selftests/bpf/progs/test_xdp_bpf2bpf.c    |   2 +-
> > >  .../bpf/progs/test_xdp_update_frags.c         |  49 ++++
> > >  16 files changed, 1044 insertions(+), 169 deletions(-)
> > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_adjust_data.c
> > >  create mode 100644 tools/testing/selftests/bpf/progs/test_xdp_update_frags.c
> > >
> > > --
> > > 2.31.1
> > >

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists