Message-ID: <20180105235945.pv6y3nbecfczzyjh@ast-mbp.dhcp.thefacebook.com>
Date:   Fri, 5 Jan 2018 15:59:46 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Daniel Borkmann <borkmann@...earbox.net>, netdev@...r.kernel.org,
        dsahern@...il.com, gospo@...adcom.com, bjorn.topel@...el.com,
        michael.chan@...adcom.com
Subject: Re: [bpf-next V4 PATCH 00/14] xdp: new XDP rx-queue info concept

On Wed, Jan 03, 2018 at 11:25:08AM +0100, Jesper Dangaard Brouer wrote:
> V4:
> * Added reviewers/acks to patches
> * Fix patch desc in i40e that got out-of-sync with code
> * Add SPDX license headers for the two new files added in patch 14
> 
> V3:
> * Fixed bug in virtio_net driver
> * Removed export of xdp_rxq_info_init()
> 
> V2:
> * Changed API exposed to drivers
>   - Removed invocation of "init" in drivers, and only call "reg"
>     (Suggested by Saeed)
>   - Allow "reg" to fail and handle this in drivers
>     (Suggested by David Ahern)
> * Removed the SINKQ qtype, instead allow to register as "unused"
> * Also fixed some drivers during testing on actual HW (noted in patches)
> 
> There is a need for XDP to know more about the RX-queue a given XDP
> frames have arrived on.  For both the XDP bpf-prog and kernel side.
> 
> Instead of extending struct xdp_buff each time new info is needed,
> this patchset takes a different approach.  Struct xdp_buff is only
> extended with a pointer to a struct xdp_rxq_info (allowing for easier
> extending this later).  This xdp_rxq_info contains information related
> to how the driver have setup the individual RX-queue's.  This is
> read-mostly information, and all xdp_buff frames (in drivers
> napi_poll) point to the same xdp_rxq_info (per RX-queue).
> 
> We stress this data/cache-line is for read-mostly info.  This is NOT
> for dynamic per packet info, use the data_meta for such use-cases.
> 
> This patchset start out small, and only expose ingress_ifindex and the
> RX-queue index to the XDP/BPF program. Access to tangible info like
> the ingress ifindex and RX queue index, is fairly easy to comprehent.
> The other future use-cases could allow XDP frames to be recycled back
> to the originating device driver, by providing info on RX device and
> queue number.
> 
> As XDP doesn't have driver feature flags, and eBPF code due to
> bpf-tail-calls cannot determine that XDP driver invoke it, this
> patchset have to update every driver that support XDP.
> 
> For driver developers (review individual driver patches!):
> 
> The xdp_rxq_info is tied to the drivers RX-ring(s). Whenever a RX-ring
> modification require (temporary) stopping RX frames, then the
> xdp_rxq_info should (likely) also be unregistred and re-registered,
> especially if reallocating the pages in the ring. Make sure ethtool
> set_channels does the right thing. When replacing XDP prog, if and
> only if RX-ring need to be changed, then also re-register the
> xdp_rxq_info.
> 
> I'm Cc'ing the individual driver patches to the registered maintainers.
> 
> Testing:
> 
> I've only tested the NIC drivers I have hardware for.  The general
> test procedure is to (DUT = Device Under Test):
>  (1) run pktgen script pktgen_sample04_many_flows.sh       (against DUT)
>  (2) run samples/bpf program xdp_rxq_info --dev $DEV       (on DUT)
>  (3) runtime modify number of NIC queues via ethtool -L    (on DUT)
>  (4) runtime modify number of NIC ring-size via ethtool -G (on DUT)
> 
> Patch based on git tree bpf-next (at commit fb982666e380c1632a):
>  https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/

Applied, thank you Jesper.

I think Michael's suggested micro optimization for patch 7 can
be done as a follow up.

Powered by blists - more mailing lists