[bpf-next,V1-RFC,06/14] mlx4: setup xdp_rxq_info

Driver hook points for xdp_rxq_info:
 * init+reg: mlx4_en_create_rx_ring
 * unreg   : mlx4_en_destroy_rx_ring

Cc: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    3 ++-
 drivers/net/ethernet/mellanox/mlx4/en_rx.c     |   13 +++++++++++--
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   |    4 +++-
 3 files changed, 16 insertions(+), 4 deletions(-)

On 13/12/2017 1:19 PM, Jesper Dangaard Brouer wrote:
> Driver hook points for xdp_rxq_info:
>   * init+reg: mlx4_en_create_rx_ring
>   * unreg   : mlx4_en_destroy_rx_ring
> 
> Cc: Tariq Toukan <tariqt@mellanox.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
>   drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    3 ++-
>   drivers/net/ethernet/mellanox/mlx4/en_rx.c     |   13 +++++++++++--
>   drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   |    4 +++-
>   3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 99051a294fa6..0cfcf3089ae4 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -2172,8 +2172,9 @@ static int mlx4_en_alloc_resources(struct mlx4_en_priv *priv)
>   
>   		if (mlx4_en_create_rx_ring(priv, &priv->rx_ring[i],
>   					   prof->rx_ring_size, priv->stride,
> -					   node))
> +					   node, i))
>   			goto err;
> +
>   	}
>   
>   #ifdef CONFIG_RFS_ACCEL
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 85e28efcda33..2091c9734e6a 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -262,7 +262,7 @@ void mlx4_en_set_num_rx_rings(struct mlx4_en_dev *mdev)
>   
>   int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
>   			   struct mlx4_en_rx_ring **pring,
> -			   u32 size, u16 stride, int node)
> +			   u32 size, u16 stride, int node, int queue_index)
>   {
>   	struct mlx4_en_dev *mdev = priv->mdev;
>   	struct mlx4_en_rx_ring *ring;
> @@ -286,6 +286,12 @@ int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
>   	ring->log_stride = ffs(ring->stride) - 1;
>   	ring->buf_size = ring->size * ring->stride + TXBB_SIZE;
>   
> +	/* XDP RX-queue info */
> +	xdp_rxq_info_init(&ring->xdp_rxq);
> +	ring->xdp_rxq.dev = priv->dev;
> +	ring->xdp_rxq.queue_index = queue_index;
> +	xdp_rxq_info_reg(&ring->xdp_rxq);
> +
>   	tmp = size * roundup_pow_of_two(MLX4_EN_MAX_RX_FRAGS *
>   					sizeof(struct mlx4_en_rx_alloc));
>   	ring->rx_info = vzalloc_node(tmp, node);
> @@ -318,6 +324,7 @@ int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
>   	vfree(ring->rx_info);
>   	ring->rx_info = NULL;
>   err_ring:
> +	xdp_rxq_info_unreg(&ring->xdp_rxq);
>   	kfree(ring);
>   	*pring = NULL;
>   
> @@ -440,6 +447,7 @@ void mlx4_en_destroy_rx_ring(struct mlx4_en_priv *priv,
>   					lockdep_is_held(&mdev->state_lock));
>   	if (old_prog)
>   		bpf_prog_put(old_prog);
> +	xdp_rxq_info_unreg(&ring->xdp_rxq);
>   	mlx4_free_hwq_res(mdev->dev, &ring->wqres, size * stride + TXBB_SIZE);
>   	vfree(ring->rx_info);
>   	ring->rx_info = NULL;
> @@ -650,6 +658,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
>   	int cq_ring = cq->ring;
>   	bool doorbell_pending;
>   	struct mlx4_cqe *cqe;
> +	struct xdp_buff xdp;
>   	int polled = 0;
>   	int index;
>   
> @@ -664,6 +673,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
>   	/* Protect accesses to: ring->xdp_prog, priv->mac_hash list */
>   	rcu_read_lock();
>   	xdp_prog = rcu_dereference(ring->xdp_prog);
> +	xdp.rxq = &ring->xdp_rxq;

You moved this to update it only once, and not per packet. Right?
This is because all fields of struct xdp_buff used to be specific 
per-packet and filled in every iteration. Now you introduce a new field 
which holds a context.

Well, I still need to go over the infrastructure in your other patches, 
but from first glance it seems that we can use two separated structs: 
one for context, and one for per-packet info.

>   	doorbell_pending = 0;
>   
>   	/* We assume a 1:1 mapping between CQEs and Rx descriptors, so Rx
> @@ -748,7 +758,6 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
>   		 * read bytes but not past the end of the frag.
>   		 */
>   		if (xdp_prog) {
> -			struct xdp_buff xdp;
>   			dma_addr_t dma;
>   			void *orig_data;
>   			u32 act;
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> index 1856e279a7e0..bdfb4362b35a 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> @@ -46,6 +46,7 @@
>   #endif
>   #include <linux/cpu_rmap.h>
>   #include <linux/ptp_clock_kernel.h>
> +#include <net/xdp.h>
>   
>   #include <linux/mlx4/device.h>
>   #include <linux/mlx4/qp.h>
> @@ -353,6 +354,7 @@ struct mlx4_en_rx_ring {
>   	unsigned long dropped;
>   	int hwtstamp_rx_filter;
>   	cpumask_var_t affinity_mask;
> +	struct xdp_rxq_info xdp_rxq;
>   };
>   
>   struct mlx4_en_cq {
> @@ -716,7 +718,7 @@ void mlx4_en_set_num_rx_rings(struct mlx4_en_dev *mdev);
>   void mlx4_en_recover_from_oom(struct mlx4_en_priv *priv);
>   int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
>   			   struct mlx4_en_rx_ring **pring,
> -			   u32 size, u16 stride, int node);
> +			   u32 size, u16 stride, int node, int queue_index);
>   void mlx4_en_destroy_rx_ring(struct mlx4_en_priv *priv,
>   			     struct mlx4_en_rx_ring **pring,
>   			     u32 size, u16 stride);
>

On Wed, 13 Dec 2017 14:42:25 +0200
Tariq Toukan <tariqt@mellanox.com> wrote:

> > @@ -650,6 +658,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
> >   	int cq_ring = cq->ring;
> >   	bool doorbell_pending;
> >   	struct mlx4_cqe *cqe;
> > +	struct xdp_buff xdp;
> >   	int polled = 0;
> >   	int index;
> >   
> > @@ -664,6 +673,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
> >   	/* Protect accesses to: ring->xdp_prog, priv->mac_hash list */
> >   	rcu_read_lock();
> >   	xdp_prog = rcu_dereference(ring->xdp_prog);
> > +	xdp.rxq = &ring->xdp_rxq;  
> 
> You moved this to update it only once, and not per packet. Right?
> This is because all fields of struct xdp_buff used to be specific 
> per-packet and filled in every iteration. Now you introduce a new field 
> which holds a context.
> 
> Well, I still need to go over the infrastructure in your other patches, 
> but from first glance it seems that we can use two separated structs: 
> one for context, and one for per-packet info.

This are two separate structs.  I guess, what you are suggesting is
passing them as separate structs to bpf_prog_run_xdp() ?

The reason I like/want to have xdp_buff point to xdp_rxq_info, is that
I want this information transferred through to the XDP_REDIRECT calls.

The plan (after this patchset) is to implement a kfree_xdp_buff() that
can free/return xdp frames directly to the driver (needed in err cases
in redirect code) by checking if the rxq->dev have defined an
ndo_xdp_return() function.