| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Dec 2017 14:50:07 -0800
From: Saeed Mahameed <saeedm@...lanox.com>
To: Jesper Dangaard Brouer <brouer@...hat.com>,
Daniel Borkmann <borkmann@...earbox.net>,
Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: netdev@...r.kernel.org, dsahern@...il.com, gospo@...adcom.com,
bjorn.topel@...el.com, michael.chan@...adcom.com
Subject: Re: [bpf-next V1-RFC PATCH 12/14] xdp: generic XDP handling of
xdp_rxq_info
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 <brouer@...hat.com>
> ---
> 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 <net/dcbnl.h>
> #endif
> #include <net/netprio_cgroup.h>
> +#include <net/xdp.h>
>
> #include <linux/netdev_features.h>
> #include <linux/neighbour.h>
> @@ -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));
>
>
Powered by blists - more mailing lists