From mboxrd@z Thu Jan 1 00:00:00 1970 From: Saeed Mahameed Subject: Re: [bpf-next V1-RFC PATCH 12/14] xdp: generic XDP handling of xdp_rxq_info Date: Wed, 13 Dec 2017 14:50:07 -0800 Message-ID: References: <151316391502.14967.13292358380181773729.stgit@firesoul> <151316402196.14967.14959244245059519467.stgit@firesoul> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, dsahern@gmail.com, gospo@broadcom.com, bjorn.topel@intel.com, michael.chan@broadcom.com To: Jesper Dangaard Brouer , Daniel Borkmann , Alexei Starovoitov Return-path: Received: from mail-eopbgr50056.outbound.protection.outlook.com ([40.107.5.56]:40384 "EHLO EUR03-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751121AbdLMWuV (ORCPT ); Wed, 13 Dec 2017 17:50:21 -0500 In-Reply-To: <151316402196.14967.14959244245059519467.stgit@firesoul> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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. -Saeed. > /* > diff --git a/net/core/dev.c b/net/core/dev.c > index 6bea8931bb62..44932d6194a2 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -3873,9 +3873,33 @@ static int enqueue_to_backlog(struct sk_buff *skb, int cpu, > return NET_RX_DROP; > } > > +static struct netdev_rx_queue *netif_get_rxqueue(struct sk_buff *skb) > +{ > + struct net_device *dev = skb->dev; > + struct netdev_rx_queue *rxqueue; > + > + rxqueue = dev->_rx; > + > + if (skb_rx_queue_recorded(skb)) { > + u16 index = skb_get_rx_queue(skb); > + > + if (unlikely(index >= dev->real_num_rx_queues)) { > + WARN_ONCE(dev->real_num_rx_queues > 1, > + "%s received packet on queue %u, but number " > + "of RX queues is %u\n", > + dev->name, index, dev->real_num_rx_queues); > + > + return rxqueue; /* Return first rxqueue */ > + } > + rxqueue += index; > + } > + return rxqueue; > +} > + > static u32 netif_receive_generic_xdp(struct sk_buff *skb, > struct bpf_prog *xdp_prog) > { > + struct netdev_rx_queue *rxqueue; > u32 metalen, act = XDP_DROP; > struct xdp_buff xdp; > void *orig_data; > @@ -3919,6 +3943,9 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb, > xdp.data_hard_start = skb->data - skb_headroom(skb); > orig_data = xdp.data; > > + rxqueue = netif_get_rxqueue(skb); > + xdp.rxq = &rxqueue->xdp_rxq; > + > act = bpf_prog_run_xdp(xdp_prog, &xdp); > > off = xdp.data - orig_data; > @@ -7538,7 +7565,6 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev, > } > EXPORT_SYMBOL(netif_stacked_transfer_operstate); > > -#ifdef CONFIG_SYSFS > static int netif_alloc_rx_queues(struct net_device *dev) > { > unsigned int i, count = dev->num_rx_queues; > @@ -7553,11 +7579,31 @@ static int netif_alloc_rx_queues(struct net_device *dev) > > dev->_rx = rx; > > - for (i = 0; i < count; i++) > + for (i = 0; i < count; i++) { > rx[i].dev = dev; > + > + /* XDP RX-queue setup */ > + xdp_rxq_info_init(&rx[i].xdp_rxq); > + rx[i].xdp_rxq.dev = dev; > + rx[i].xdp_rxq.queue_index = i; > + xdp_rxq_info_reg(&rx[i].xdp_rxq); > + } > return 0; > } > -#endif > + > +static void netif_free_rx_queues(struct net_device *dev) > +{ > + unsigned int i, count = dev->num_rx_queues; > + struct netdev_rx_queue *rx; > + > + if (!dev->_rx) /* netif_alloc_rx_queues alloc failed */ > + return; > + > + rx = dev->_rx; > + > + for (i = 0; i < count; i++) > + xdp_rxq_info_unreg(&rx[i].xdp_rxq); > +} > > static void netdev_init_one_queue(struct net_device *dev, > struct netdev_queue *queue, void *_unused) > @@ -8118,12 +8164,10 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, > return NULL; > } > > -#ifdef CONFIG_SYSFS > if (rxqs < 1) { > pr_err("alloc_netdev: Unable to allocate device with zero RX queues\n"); > return NULL; > } > -#endif > > alloc_size = sizeof(struct net_device); > if (sizeof_priv) { > @@ -8180,12 +8224,10 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, > if (netif_alloc_netdev_queues(dev)) > goto free_all; > > -#ifdef CONFIG_SYSFS > dev->num_rx_queues = rxqs; > dev->real_num_rx_queues = rxqs; > if (netif_alloc_rx_queues(dev)) > goto free_all; > -#endif > > strcpy(dev->name, name); > dev->name_assign_type = name_assign_type; > @@ -8224,9 +8266,7 @@ void free_netdev(struct net_device *dev) > > might_sleep(); > netif_free_tx_queues(dev); > -#ifdef CONFIG_SYSFS > - kvfree(dev->_rx); > -#endif > + netif_free_rx_queues(dev); > > kfree(rcu_dereference_protected(dev->ingress_queue, 1)); > >