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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fbf83e88-0c0c-22fb-998b-ee46d3002604@mellanox.com>
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ