Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e6dbe939 authored by Jesper Dangaard Brouer's avatar Jesper Dangaard Brouer Committed by David S. Miller
Browse files

Revert "net: thunderx: Add support for xdp redirect" 

This reverts commit aa136d0c.

As I previously[1] pointed out this implementation of XDP_REDIRECT is
wrong.  XDP_REDIRECT is a facility that must work between different
NIC drivers.  Another NIC driver can call ndo_xdp_xmit/nicvf_xdp_xmit,
but your driver patch assumes payload data (at top of page) will
contain a queue index and a DMA addr, this is not true and worse will
likely contain garbage.

Given you have not fixed this in due time (just reached v4.16-rc1),
the only option I see is a revert.

[1] http://lkml.kernel.org/r/20171211130902.482513d3@redhat.com



Cc: Sunil Goutham <sgoutham@cavium.com>
Cc: Christina Jacob <cjacob@caviumnetworks.com>
Cc: Aleksey Makarov <aleksey.makarov@cavium.com>
Fixes: aa136d0c ("net: thunderx: Add support for xdp redirect")
Signed-off-by: default avatarJesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent fae8b6f4

drivers/net/ethernet/cavium/thunder/nicvf_main.c

+26 −84
Original line number Original line Diff line number Diff line
@@ -67,11 +67,6 @@ module_param(cpi_alg, int, S_IRUGO); 
MODULE_PARM_DESC(cpi_alg,

MODULE_PARM_DESC(cpi_alg,

		 "PFC algorithm (0=none, 1=VLAN, 2=VLAN16, 3=IP Diffserv)");

		 "PFC algorithm (0=none, 1=VLAN, 2=VLAN16, 3=IP Diffserv)");





struct nicvf_xdp_tx {

	u64 dma_addr;

	u8  qidx;

};



static inline u8 nicvf_netdev_qidx(struct nicvf *nic, u8 qidx)

static inline u8 nicvf_netdev_qidx(struct nicvf *nic, u8 qidx)

{

{

	if (nic->sqs_mode)

	if (nic->sqs_mode)
@@ -507,29 +502,14 @@ static int nicvf_init_resources(struct nicvf *nic) 
	return 0;

	return 0;

}

}





static void nicvf_unmap_page(struct nicvf *nic, struct page *page, u64 dma_addr)

{

	/* Check if it's a recycled page, if not unmap the DMA mapping.

	 * Recycled page holds an extra reference.

	 */

	if (page_ref_count(page) == 1) {

		dma_addr &= PAGE_MASK;

		dma_unmap_page_attrs(&nic->pdev->dev, dma_addr,

				     RCV_FRAG_LEN + XDP_HEADROOM,

				     DMA_FROM_DEVICE,

				     DMA_ATTR_SKIP_CPU_SYNC);

	}

}



static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,

static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,

				struct cqe_rx_t *cqe_rx, struct snd_queue *sq,

				struct cqe_rx_t *cqe_rx, struct snd_queue *sq,

				struct rcv_queue *rq, struct sk_buff **skb)

				struct rcv_queue *rq, struct sk_buff **skb)

{

{

	struct xdp_buff xdp;

	struct xdp_buff xdp;

	struct page *page;

	struct page *page;

	struct nicvf_xdp_tx *xdp_tx = NULL;

	u32 action;

	u32 action;

	u16 len, err, offset = 0;

	u16 len, offset = 0;

	u64 dma_addr, cpu_addr;

	u64 dma_addr, cpu_addr;

	void *orig_data;

	void *orig_data;
@@ -543,7 +523,7 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog, 
	cpu_addr = (u64)phys_to_virt(cpu_addr);

	cpu_addr = (u64)phys_to_virt(cpu_addr);

	page = virt_to_page((void *)cpu_addr);

	page = virt_to_page((void *)cpu_addr);





	xdp.data_hard_start = page_address(page) + RCV_BUF_HEADROOM;

	xdp.data_hard_start = page_address(page);

	xdp.data = (void *)cpu_addr;

	xdp.data = (void *)cpu_addr;

	xdp_set_data_meta_invalid(&xdp);

	xdp_set_data_meta_invalid(&xdp);

	xdp.data_end = xdp.data + len;

	xdp.data_end = xdp.data + len;
@@ -563,7 +543,18 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog, 




	switch (action) {

	switch (action) {

	case XDP_PASS:

	case XDP_PASS:

		nicvf_unmap_page(nic, page, dma_addr);

		/* Check if it's a recycled page, if not

		 * unmap the DMA mapping.

		 *

		 * Recycled page holds an extra reference.

		 */

		if (page_ref_count(page) == 1) {

			dma_addr &= PAGE_MASK;

			dma_unmap_page_attrs(&nic->pdev->dev, dma_addr,

					     RCV_FRAG_LEN + XDP_PACKET_HEADROOM,

					     DMA_FROM_DEVICE,

					     DMA_ATTR_SKIP_CPU_SYNC);

		}





		/* Build SKB and pass on packet to network stack */

		/* Build SKB and pass on packet to network stack */

		*skb = build_skb(xdp.data,

		*skb = build_skb(xdp.data,
@@ -576,20 +567,6 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog, 
	case XDP_TX:

	case XDP_TX:

		nicvf_xdp_sq_append_pkt(nic, sq, (u64)xdp.data, dma_addr, len);

		nicvf_xdp_sq_append_pkt(nic, sq, (u64)xdp.data, dma_addr, len);

		return true;

		return true;

	case XDP_REDIRECT:

		/* Save DMA address for use while transmitting */

		xdp_tx = (struct nicvf_xdp_tx *)page_address(page);

		xdp_tx->dma_addr = dma_addr;

		xdp_tx->qidx = nicvf_netdev_qidx(nic, cqe_rx->rq_idx);



		err = xdp_do_redirect(nic->pnicvf->netdev, &xdp, prog);

		if (!err)

			return true;



		/* Free the page on error */

		nicvf_unmap_page(nic, page, dma_addr);

		put_page(page);

		break;

	default:

	default:

		bpf_warn_invalid_xdp_action(action);

		bpf_warn_invalid_xdp_action(action);

		/* fall through */

		/* fall through */
@@ -597,7 +574,18 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog, 
		trace_xdp_exception(nic->netdev, prog, action);

		trace_xdp_exception(nic->netdev, prog, action);

		/* fall through */

		/* fall through */

	case XDP_DROP:

	case XDP_DROP:

		nicvf_unmap_page(nic, page, dma_addr);

		/* Check if it's a recycled page, if not

		 * unmap the DMA mapping.

		 *

		 * Recycled page holds an extra reference.

		 */

		if (page_ref_count(page) == 1) {

			dma_addr &= PAGE_MASK;

			dma_unmap_page_attrs(&nic->pdev->dev, dma_addr,

					     RCV_FRAG_LEN + XDP_PACKET_HEADROOM,

					     DMA_FROM_DEVICE,

					     DMA_ATTR_SKIP_CPU_SYNC);

		}

		put_page(page);

		put_page(page);

		return true;

		return true;

	}

	}
@@ -1864,50 +1852,6 @@ static int nicvf_xdp(struct net_device *netdev, struct netdev_bpf *xdp) 
	}

	}

}

}





static int nicvf_xdp_xmit(struct net_device *netdev, struct xdp_buff *xdp)

{

	struct nicvf *nic = netdev_priv(netdev);

	struct nicvf *snic = nic;

	struct nicvf_xdp_tx *xdp_tx;

	struct snd_queue *sq;

	struct page *page;

	int err, qidx;



	if (!netif_running(netdev) || !nic->xdp_prog)

		return -EINVAL;



	page = virt_to_page(xdp->data);

	xdp_tx = (struct nicvf_xdp_tx *)page_address(page);

	qidx = xdp_tx->qidx;



	if (xdp_tx->qidx >= nic->xdp_tx_queues)

		return -EINVAL;



	/* Get secondary Qset's info */

	if (xdp_tx->qidx >= MAX_SND_QUEUES_PER_QS) {

		qidx = xdp_tx->qidx / MAX_SND_QUEUES_PER_QS;

		snic = (struct nicvf *)nic->snicvf[qidx - 1];

		if (!snic)

			return -EINVAL;

		qidx = xdp_tx->qidx % MAX_SND_QUEUES_PER_QS;

	}



	sq = &snic->qs->sq[qidx];

	err = nicvf_xdp_sq_append_pkt(snic, sq, (u64)xdp->data,

				      xdp_tx->dma_addr,

				      xdp->data_end - xdp->data);

	if (err)

		return -ENOMEM;



	nicvf_xdp_sq_doorbell(snic, sq, qidx);

	return 0;

}



static void nicvf_xdp_flush(struct net_device *dev)

{

	return;

}



static int nicvf_config_hwtstamp(struct net_device *netdev, struct ifreq *ifr)

static int nicvf_config_hwtstamp(struct net_device *netdev, struct ifreq *ifr)

{

{

	struct hwtstamp_config config;

	struct hwtstamp_config config;
@@ -1986,8 +1930,6 @@ static const struct net_device_ops nicvf_netdev_ops = { 
	.ndo_fix_features       = nicvf_fix_features,

	.ndo_fix_features       = nicvf_fix_features,

	.ndo_set_features       = nicvf_set_features,

	.ndo_set_features       = nicvf_set_features,

	.ndo_bpf		= nicvf_xdp,

	.ndo_bpf		= nicvf_xdp,

	.ndo_xdp_xmit		= nicvf_xdp_xmit,

	.ndo_xdp_flush          = nicvf_xdp_flush,

	.ndo_do_ioctl           = nicvf_ioctl,

	.ndo_do_ioctl           = nicvf_ioctl,

};

};

drivers/net/ethernet/cavium/thunder/nicvf_queues.c

+5 −6
Original line number Original line Diff line number Diff line
@@ -204,7 +204,7 @@ static inline int nicvf_alloc_rcv_buffer(struct nicvf *nic, struct rbdr *rbdr, 




	/* Reserve space for header modifications by BPF program */

	/* Reserve space for header modifications by BPF program */

	if (rbdr->is_xdp)

	if (rbdr->is_xdp)

		buf_len += XDP_HEADROOM;

		buf_len += XDP_PACKET_HEADROOM;





	/* Check if it's recycled */

	/* Check if it's recycled */

	if (pgcache)

	if (pgcache)
@@ -224,9 +224,8 @@ static inline int nicvf_alloc_rcv_buffer(struct nicvf *nic, struct rbdr *rbdr, 
			nic->rb_page = NULL;

			nic->rb_page = NULL;

			return -ENOMEM;

			return -ENOMEM;

		}

		}



		if (pgcache)

		if (pgcache)

			pgcache->dma_addr = *rbuf + XDP_HEADROOM;

			pgcache->dma_addr = *rbuf + XDP_PACKET_HEADROOM;

		nic->rb_page_offset += buf_len;

		nic->rb_page_offset += buf_len;

	}

	}
@@ -1244,7 +1243,7 @@ int nicvf_xdp_sq_append_pkt(struct nicvf *nic, struct snd_queue *sq, 
	int qentry;

	int qentry;





	if (subdesc_cnt > sq->xdp_free_cnt)

	if (subdesc_cnt > sq->xdp_free_cnt)

		return -1;

		return 0;





	qentry = nicvf_get_sq_desc(sq, subdesc_cnt);

	qentry = nicvf_get_sq_desc(sq, subdesc_cnt);
@@ -1255,7 +1254,7 @@ int nicvf_xdp_sq_append_pkt(struct nicvf *nic, struct snd_queue *sq, 




	sq->xdp_desc_cnt += subdesc_cnt;

	sq->xdp_desc_cnt += subdesc_cnt;





	return 0;

	return 1;

}

}





/* Calculate no of SQ subdescriptors needed to transmit all

/* Calculate no of SQ subdescriptors needed to transmit all
@@ -1656,7 +1655,7 @@ static void nicvf_unmap_rcv_buffer(struct nicvf *nic, u64 dma_addr, 
		if (page_ref_count(page) != 1)

		if (page_ref_count(page) != 1)

			return;

			return;





		len += XDP_HEADROOM;

		len += XDP_PACKET_HEADROOM;

		/* Receive buffers in XDP mode are mapped from page start */

		/* Receive buffers in XDP mode are mapped from page start */

		dma_addr &= PAGE_MASK;

		dma_addr &= PAGE_MASK;

	}

	}

drivers/net/ethernet/cavium/thunder/nicvf_queues.h

+0 −4
Original line number Original line Diff line number Diff line
@@ -11,7 +11,6 @@ 




#include <linux/netdevice.h>

#include <linux/netdevice.h>

#include <linux/iommu.h>

#include <linux/iommu.h>

#include <linux/bpf.h>

#include <net/xdp.h>

#include <net/xdp.h>

#include "q_struct.h"

#include "q_struct.h"
@@ -94,9 +93,6 @@ 
#define RCV_FRAG_LEN	 (SKB_DATA_ALIGN(DMA_BUFFER_LEN + NET_SKB_PAD) + \

#define RCV_FRAG_LEN	 (SKB_DATA_ALIGN(DMA_BUFFER_LEN + NET_SKB_PAD) + \

			 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))

			 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))





#define RCV_BUF_HEADROOM	128 /* To store dma address for XDP redirect */

#define XDP_HEADROOM		(XDP_PACKET_HEADROOM + RCV_BUF_HEADROOM)



#define MAX_CQES_FOR_TX		((SND_QUEUE_LEN / MIN_SQ_DESC_PER_PKT_XMIT) * \

#define MAX_CQES_FOR_TX		((SND_QUEUE_LEN / MIN_SQ_DESC_PER_PKT_XMIT) * \

				 MAX_CQE_PER_PKT_XMIT)

				 MAX_CQE_PER_PKT_XMIT)