Skip to content

Commit

Permalink
Revert "net: thunderx: Add support for xdp redirect"
Browse files Browse the repository at this point in the history 
This reverts commit aa136d0.

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: aa136d0 ("net: thunderx: Add support for xdp redirect")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Jesper Dangaard Brouer authored and David S. Miller committed Feb 14, 2018
1 parent fae8b6f commit e6dbe93
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 94 deletions.
110 changes: 26 additions & 84 deletions drivers/net/ethernet/cavium/thunder/nicvf_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ module_param(cpi_alg, int, S_IRUGO);
MODULE_PARM_DESC(cpi_alg,
"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)
{
if (nic->sqs_mode)
Expand Down Expand Up @@ -507,29 +502,14 @@ static int nicvf_init_resources(struct nicvf *nic)
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,
struct cqe_rx_t *cqe_rx, struct snd_queue *sq,
struct rcv_queue *rq, struct sk_buff **skb)
{
struct xdp_buff xdp;
struct page *page;
struct nicvf_xdp_tx *xdp_tx = NULL;
u32 action;
u16 len, err, offset = 0;
u16 len, offset = 0;
u64 dma_addr, cpu_addr;
void *orig_data;

Expand All @@ -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);
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_set_data_meta_invalid(&xdp);
xdp.data_end = xdp.data + len;
Expand All @@ -563,7 +543,18 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,

switch (action) {
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 */
*skb = build_skb(xdp.data,
Expand All @@ -576,28 +567,25 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,
case XDP_TX:
nicvf_xdp_sq_append_pkt(nic, sq, (u64)xdp.data, dma_addr, len);
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:
bpf_warn_invalid_xdp_action(action);
/* fall through */
case XDP_ABORTED:
trace_xdp_exception(nic->netdev, prog, action);
/* fall through */
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);
return true;
}
Expand Down Expand Up @@ -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)
{
struct hwtstamp_config config;
Expand Down Expand Up @@ -1986,8 +1930,6 @@ static const struct net_device_ops nicvf_netdev_ops = {
.ndo_fix_features = nicvf_fix_features,
.ndo_set_features = nicvf_set_features,
.ndo_bpf = nicvf_xdp,
.ndo_xdp_xmit = nicvf_xdp_xmit,
.ndo_xdp_flush = nicvf_xdp_flush,
.ndo_do_ioctl = nicvf_ioctl,
};

Expand Down
11 changes: 5 additions & 6 deletions drivers/net/ethernet/cavium/thunder/nicvf_queues.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
if (rbdr->is_xdp)
buf_len += XDP_HEADROOM;
buf_len += XDP_PACKET_HEADROOM;

/* Check if it's recycled */
if (pgcache)
Expand All @@ -224,9 +224,8 @@ static inline int nicvf_alloc_rcv_buffer(struct nicvf *nic, struct rbdr *rbdr,
nic->rb_page = NULL;
return -ENOMEM;
}

if (pgcache)
pgcache->dma_addr = *rbuf + XDP_HEADROOM;
pgcache->dma_addr = *rbuf + XDP_PACKET_HEADROOM;
nic->rb_page_offset += buf_len;
}

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

if (subdesc_cnt > sq->xdp_free_cnt)
return -1;
return 0;

qentry = nicvf_get_sq_desc(sq, subdesc_cnt);

Expand All @@ -1255,7 +1254,7 @@ int nicvf_xdp_sq_append_pkt(struct nicvf *nic, struct snd_queue *sq,

sq->xdp_desc_cnt += subdesc_cnt;

return 0;
return 1;
}

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

len += XDP_HEADROOM;
len += XDP_PACKET_HEADROOM;
/* Receive buffers in XDP mode are mapped from page start */
dma_addr &= PAGE_MASK;
}
Expand Down
4 changes: 0 additions & 4 deletions drivers/net/ethernet/cavium/thunder/nicvf_queues.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#include <linux/netdevice.h>
#include <linux/iommu.h>
#include <linux/bpf.h>
#include <net/xdp.h>
#include "q_struct.h"

Expand Down Expand Up @@ -94,9 +93,6 @@
#define RCV_FRAG_LEN (SKB_DATA_ALIGN(DMA_BUFFER_LEN + NET_SKB_PAD) + \
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) * \
MAX_CQE_PER_PKT_XMIT)

Expand Down

0 comments on commit e6dbe93

Please sign in to comment.