From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [bpf-next V1-RFC PATCH 08/14] nfp: setup xdp_rxq_info Date: Wed, 13 Dec 2017 18:34:27 -0800 Message-ID: <20171213183427.213f6206@cakuba.netronome.com> References: <151316391502.14967.13292358380181773729.stgit@firesoul> <151316400161.14967.13516011331263133183.stgit@firesoul> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Daniel Borkmann , Alexei Starovoitov , dsahern@gmail.com, oss-drivers@netronome.com, Simon Horman , netdev@vger.kernel.org, gospo@broadcom.com, bjorn.topel@intel.com, michael.chan@broadcom.com To: Jesper Dangaard Brouer Return-path: Received: from mail-qk0-f196.google.com ([209.85.220.196]:38670 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751101AbdLNCec (ORCPT ); Wed, 13 Dec 2017 21:34:32 -0500 Received: by mail-qk0-f196.google.com with SMTP id z203so4576598qkb.5 for ; Wed, 13 Dec 2017 18:34:32 -0800 (PST) In-Reply-To: <151316400161.14967.13516011331263133183.stgit@firesoul> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 13 Dec 2017 12:20:01 +0100, Jesper Dangaard Brouer wrote: > Driver hook points for xdp_rxq_info: > * init+reg: nfp_net_rx_ring_alloc > * unreg : nfp_net_rx_ring_free > > In struct nfp_net_rx_ring moved member @size into a hole on 64-bit. > Thus, the size remaines the same after adding member @xdp_rxq. > > Cc: oss-drivers@netronome.com > Cc: Jakub Kicinski > Cc: Simon Horman > Signed-off-by: Jesper Dangaard Brouer > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h > index 3801c52098d5..0e564cfabe7e 100644 > --- a/drivers/net/ethernet/netronome/nfp/nfp_net.h > +++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h > @@ -47,6 +47,7 @@ > #include > #include > #include > +#include > > #include "nfp_net_ctrl.h" > > @@ -350,6 +351,7 @@ struct nfp_net_rx_buf { > * @rxds: Virtual address of FL/RX ring in host memory > * @dma: DMA address of the FL/RX ring > * @size: Size, in bytes, of the FL/RX ring (needed to free) > + * @xdp_rxq: RX-ring info avail for XDP > */ > struct nfp_net_rx_ring { > struct nfp_net_r_vector *r_vec; > @@ -361,13 +363,14 @@ struct nfp_net_rx_ring { > u32 idx; > > int fl_qcidx; > + unsigned int size; > u8 __iomem *qcp_fl; > > struct nfp_net_rx_buf *rxbufs; > struct nfp_net_rx_desc *rxds; > > dma_addr_t dma; > - unsigned int size; > + struct xdp_rxq_info xdp_rxq; > } ____cacheline_aligned; The @size member is not in the hole on purpose. IIRC all the members up to @dma are in the first cacheline. All things which are not needed on the fast path are after @dma. IOW @size is not used on the fast path and the hole is for fast path stuff :) > /** > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c > index ad3e9f6a61e5..6474aecd0451 100644 > --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c > +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c > @@ -2252,6 +2253,7 @@ static void nfp_net_rx_ring_free(struct nfp_net_rx_ring *rx_ring) > struct nfp_net_r_vector *r_vec = rx_ring->r_vec; > struct nfp_net_dp *dp = &r_vec->nfp_net->dp; > > + xdp_rxq_info_unreg(&rx_ring->xdp_rxq); > kfree(rx_ring->rxbufs); > > if (rx_ring->rxds) > @@ -2277,6 +2279,12 @@ nfp_net_rx_ring_alloc(struct nfp_net_dp *dp, struct nfp_net_rx_ring *rx_ring) > { > int sz; > > + /* XDP RX-queue info */ > + xdp_rxq_info_init(&rx_ring->xdp_rxq); > + rx_ring->xdp_rxq.dev = dp->netdev; > + rx_ring->xdp_rxq.queue_index = rx_ring->idx; > + xdp_rxq_info_reg(&rx_ring->xdp_rxq); > + > rx_ring->cnt = dp->rxd_cnt; > rx_ring->size = sizeof(*rx_ring->rxds) * rx_ring->cnt; > rx_ring->rxds = dma_zalloc_coherent(dp->dev, rx_ring->size, The nfp driver implements the prepare/commit for reallocating rings. I don't think it matters now, but there can be 2 sets of rings with the same ID allocated during reconfiguration (see nfp_net_ring_reconfig()). Maybe place the register/unregister in nfp_net_open_stack() and nfp_net_close_stack() respectively? Perhaps that won't be necessary, only cleaner :) I'm not sure how is the redirect between drivers intended to work WRT freeing rings and unloading drivers while packets fly...