From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesper Dangaard Brouer Subject: Re: [bpf-next V1-RFC PATCH 12/14] xdp: generic XDP handling of xdp_rxq_info Date: Mon, 18 Dec 2017 10:47:49 +0100 Message-ID: <20171218104749.3cb23edf@redhat.com> References: <151316391502.14967.13292358380181773729.stgit@firesoul> <151316402196.14967.14959244245059519467.stgit@firesoul> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Daniel Borkmann , Alexei Starovoitov , netdev@vger.kernel.org, dsahern@gmail.com, gospo@broadcom.com, bjorn.topel@intel.com, michael.chan@broadcom.com, brouer@redhat.com To: Saeed Mahameed Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47366 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933556AbdLRJr5 (ORCPT ); Mon, 18 Dec 2017 04:47:57 -0500 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 13 Dec 2017 14:50:07 -0800 Saeed Mahameed wrote: > On 12/13/2017 3:20 AM, Jesper Dangaard Brouer wrote: > > Hook points for xdp_rxq_info: > > * init+reg: netif_alloc_rx_queues > > * unreg : netif_free_rx_queues > > > > The net_device have some members (num_rx_queues + real_num_rx_queues) > > and data-area (dev->_rx with struct netdev_rx_queue's) that were > > primarily used for exporting information about RPS (CONFIG_RPS) queues > > to sysfs (CONFIG_SYSFS). > > > > For generic XDP extend struct netdev_rx_queue with the xdp_rxq_info, > > and remove some of the CONFIG_SYSFS ifdefs. > > > > Signed-off-by: Jesper Dangaard Brouer > > --- > > include/linux/netdevice.h | 2 ++ > > net/core/dev.c | 60 ++++++++++++++++++++++++++++++++++++++------- > > 2 files changed, 52 insertions(+), 10 deletions(-) > > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > index cc4ce7456e38..43595b037872 100644 > > --- a/include/linux/netdevice.h > > +++ b/include/linux/netdevice.h > > @@ -44,6 +44,7 @@ > > #include > > #endif > > #include > > +#include > > > > #include > > #include > > @@ -686,6 +687,7 @@ struct netdev_rx_queue { > > #endif > > struct kobject kobj; > > struct net_device *dev; > > + struct xdp_rxq_info xdp_rxq; > > } ____cacheline_aligned_in_smp; > > > > Instead of duplicating this xdp_rxq_info and have 2 instances of it for > drivers that do support XDP (the generic one and the driver internal > xdp_rxq_info), drivers can use the generic netdev_rx_queue.xdp_rxq to > register their own xdp_rxq_info. > I suggest the following API for drivers to use: > > xdp_rxq_info_reg(netdev, rxq_index) > { > rxqueue = dev->_rx + rxq_index; > xdp_rxq = rxqueue.xdp_rxq; > xdp_rxq_info_init(xdp_rxq); > xdp_rxq.dev = netdev; > xdp_rxq.queue_index = rxq_index; > } > > xdp_rxq_info_unreg(netdev, rxq_index) > { > ... > } > > This way you can avoid the xdp_rxq_info structure management by the > drivers them selves and reduce duplicated code to init, fill the > xdp_rxq_info per driver. Having the xdp_rxq_info struct memory associated with the net_device, and not the drivers RX-rings, make it harder for the drivers to use this API. Because drivers (e.g. i40e) to minimize "downtime" when e.g. changing numbers of queues (ethtool set_channels) allocate and setup new (RX+TX+XDP_TX)-rings _before_ taking down current RX-rings. Thus, you have the problem of driver wanting to register two xdp_rxq_info's at the same time, before unregistering the old. Another issue I've seen is that, drivers do funny tricks, and you cannot always assuming that rxq_index would be valid and bind directly to dev->_rx + rxq_index. E.g. in the i40e driver they have vsi->num_queue_pairs they iterate over, and one of the vsi->rx_rings[i] is a special flow-director (FDIR) ring that comes in the end of the rx_rings[] array but internally have queue_index zero. I was also hoping to unifying/integrating the generic-XDP REDIRECT code path, by assigning the net_device stored xdp_rxq_info another qtype "SKB-MODE-TYPE". Allowing us to share more of the native-XDP redirect-code path with SKB-XDP frames from generic-XDP. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer