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

This reverts commit aa136d0c82fcd6af14535853c30e219e02b2692d.

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: aa136d0c82fc ("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>

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index b68cde9f17d2b10c08ca60c7a4d8c80f2d095b97..7d9c5ffbd0412191fc112a5eb7174ef318786276 100644 (file)
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -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)
@@ -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;
 
@@ -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;
@@ -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,
@@ -576,20 +567,6 @@ 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 */
@@ -597,7 +574,18 @@ static inline bool nicvf_xdp_rx(struct nicvf *nic, struct bpf_prog *prog,
                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;
        }
@@ -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;
@@ -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,
 };
 

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 3eae9ff9b53a69413c986bb189ece2dc84f597ea..d42704d0748434d92eb17c74b0b0530aaa50f536 100644 (file)
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -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)
@@ -224,9 +224,8 @@ ret:
                        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;
        }
 
@@ -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);
 
@@ -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
@@ -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;
        }

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
index ce1eed7a6d63bd2671b5a3706d03e1f7e9e08010..5e9a03cf1b4d30f220f789934275fb9f2c9ce660 100644 (file)
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
@@ -11,7 +11,6 @@
 
 #include <linux/netdevice.h>
 #include <linux/iommu.h>
-#include <linux/bpf.h>
 #include <net/xdp.h>
 #include "q_struct.h"
 
@@ -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)