[PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[Michael Chan]

The first 10 patches refactor the code (rx/tx code paths and ring logic)
and add the basic infrastructure to support XDP.  The 11th patch adds
basic ndo_xdp to support XDP_DROP and XDP_PASS only.  The 12th patch
completes the series with XDP_TX.

Thanks to Andy Gospodarek for testing and uncovering some bugs.

v2: Addressed review comments from Alexei Starovoitov, Jakub Kicinski,
and David Miller:
	- Added missing dma syncs.
	- Added XDP headroom support.
	- Added tracing in exception path.
	- Clarified a parameter change.

Michael Chan (12):
  bnxt_en: Refactor rx SKB function.
  bnxt_en: Don't use DEFINE_DMA_UNMAP_ADDR to store DMA address in RX
    path.
  bnxt_en: Add bp->rx_dir field for rx buffer DMA direction.
  bnxt_en: Parameterize RX buffer offsets.
  bnxt_en: Add RX page mode support.
  bnxt_en: Use event bit map in RX path.
  bnxt_en: Centralize logic to reserve rings.
  bnxt_en: Add tx ring mapping logic.
  bnxt_en: Add a set of TX rings to support XDP.
  bnxt_en: Refactor tx completion path.
  bnxt_en: Add basic XDP support.
  bnxt_en: Add support for XDP_TX action.

 drivers/net/ethernet/broadcom/Kconfig             |   8 +
 drivers/net/ethernet/broadcom/bnxt/Makefile       |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 395 ++++++++++++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  62 +++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  53 ++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c     | 247 ++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h     |  29 ++
 7 files changed, 646 insertions(+), 150 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h

-- 
1.8.3.1

[PATCH net-next v2 01/12] bnxt_en: Refactor rx SKB function.

[Michael Chan]

Minor refactoring of bnxt_rx_skb() so that it can easily be replaced by
a new function that handles packets in a single page.  Also, use a
function pointer bp->rx_skb_func() to switch to a new function when
we add the new mode in the next patch.

Add a new field data_ptr that points to the packet data in the
bnxt_sw_rx_bd structure.  The original data field is changed to void
pointer so that it can either hold the kmalloc'ed data or a page
pointer.

The last parameter of bnxt_rx_skb() which was the length parameter is
changed to include the payload offset of the packet in the upper 16 bit.
The offset is needed to support the rx page mode and is not used in
this existing function.

v2: Changed the name of the last parameter to offset_and_len to make the
code more clear.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 44 ++++++++++++++++++++++---------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h | 11 ++++++--
 2 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index aff3dc1..48fb719 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -607,6 +607,7 @@ static inline int bnxt_alloc_rx_data(struct bnxt *bp,
 		return -ENOMEM;
 
 	rx_buf->data = data;
+	rx_buf->data_ptr = data + BNXT_RX_OFFSET;
 	dma_unmap_addr_set(rx_buf, mapping, mapping);
 
 	rxbd->rx_bd_haddr = cpu_to_le64(mapping);
@@ -615,7 +616,7 @@ static inline int bnxt_alloc_rx_data(struct bnxt *bp,
 }
 
 static void bnxt_reuse_rx_data(struct bnxt_rx_ring_info *rxr, u16 cons,
-			       u8 *data)
+			       void *data)
 {
 	u16 prod = rxr->rx_prod;
 	struct bnxt_sw_rx_bd *cons_rx_buf, *prod_rx_buf;
@@ -625,6 +626,7 @@ static void bnxt_reuse_rx_data(struct bnxt_rx_ring_info *rxr, u16 cons,
 	cons_rx_buf = &rxr->rx_buf_ring[cons];
 
 	prod_rx_buf->data = data;
+	prod_rx_buf->data_ptr = cons_rx_buf->data_ptr;
 
 	dma_unmap_addr_set(prod_rx_buf, mapping,
 			   dma_unmap_addr(cons_rx_buf, mapping));
@@ -755,8 +757,8 @@ static void bnxt_reuse_rx_agg_bufs(struct bnxt_napi *bnapi, u16 cp_cons,
 
 static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
 				   struct bnxt_rx_ring_info *rxr, u16 cons,
-				   u16 prod, u8 *data, dma_addr_t dma_addr,
-				   unsigned int len)
+				   u16 prod, void *data, dma_addr_t dma_addr,
+				   unsigned int offset_and_len)
 {
 	int err;
 	struct sk_buff *skb;
@@ -776,7 +778,7 @@ static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
 	}
 
 	skb_reserve(skb, BNXT_RX_OFFSET);
-	skb_put(skb, len);
+	skb_put(skb, offset_and_len & 0xffff);
 	return skb;
 }
 
@@ -878,7 +880,8 @@ static inline struct sk_buff *bnxt_copy_skb(struct bnxt_napi *bnapi, u8 *data,
 	dma_sync_single_for_cpu(&pdev->dev, mapping,
 				bp->rx_copy_thresh, PCI_DMA_FROMDEVICE);
 
-	memcpy(skb->data - BNXT_RX_OFFSET, data, len + BNXT_RX_OFFSET);
+	memcpy(skb->data - NET_IP_ALIGN, data - NET_IP_ALIGN,
+	       len + NET_IP_ALIGN);
 
 	dma_sync_single_for_device(&pdev->dev, mapping,
 				   bp->rx_copy_thresh,
@@ -951,6 +954,7 @@ static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
 	}
 
 	prod_rx_buf->data = tpa_info->data;
+	prod_rx_buf->data_ptr = tpa_info->data_ptr;
 
 	mapping = tpa_info->mapping;
 	dma_unmap_addr_set(prod_rx_buf, mapping, mapping);
@@ -960,6 +964,7 @@ static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
 	prod_bd->rx_bd_haddr = cpu_to_le64(mapping);
 
 	tpa_info->data = cons_rx_buf->data;
+	tpa_info->data_ptr = cons_rx_buf->data_ptr;
 	cons_rx_buf->data = NULL;
 	tpa_info->mapping = dma_unmap_addr(cons_rx_buf, mapping);
 
@@ -1192,12 +1197,13 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 	struct bnxt_cp_ring_info *cpr = &bnapi->cp_ring;
 	struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
 	u8 agg_id = TPA_END_AGG_ID(tpa_end);
-	u8 *data, agg_bufs;
+	u8 *data_ptr, agg_bufs;
 	u16 cp_cons = RING_CMP(*raw_cons);
 	unsigned int len;
 	struct bnxt_tpa_info *tpa_info;
 	dma_addr_t mapping;
 	struct sk_buff *skb;
+	void *data;
 
 	if (unlikely(bnapi->in_reset)) {
 		int rc = bnxt_discard_rx(bp, bnapi, raw_cons, tpa_end);
@@ -1209,7 +1215,8 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 
 	tpa_info = &rxr->rx_tpa[agg_id];
 	data = tpa_info->data;
-	prefetch(data);
+	data_ptr = tpa_info->data_ptr;
+	prefetch(data_ptr);
 	len = tpa_info->len;
 	mapping = tpa_info->mapping;
 
@@ -1232,7 +1239,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 	}
 
 	if (len <= bp->rx_copy_thresh) {
-		skb = bnxt_copy_skb(bnapi, data, len, mapping);
+		skb = bnxt_copy_skb(bnapi, data_ptr, len, mapping);
 		if (!skb) {
 			bnxt_abort_tpa(bp, bnapi, cp_cons, agg_bufs);
 			return NULL;
@@ -1248,6 +1255,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 		}
 
 		tpa_info->data = new_data;
+		tpa_info->data_ptr = new_data + BNXT_RX_OFFSET;
 		tpa_info->mapping = new_mapping;
 
 		skb = build_skb(data, 0);
@@ -1316,9 +1324,10 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 	u16 cons, prod, cp_cons = RING_CMP(tmp_raw_cons);
 	struct bnxt_sw_rx_bd *rx_buf;
 	unsigned int len;
-	u8 *data, agg_bufs, cmp_type;
+	u8 *data_ptr, agg_bufs, cmp_type;
 	dma_addr_t dma_addr;
 	struct sk_buff *skb;
+	void *data;
 	int rc = 0;
 
 	rxcmp = (struct rx_cmp *)
@@ -1363,13 +1372,14 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 	cons = rxcmp->rx_cmp_opaque;
 	rx_buf = &rxr->rx_buf_ring[cons];
 	data = rx_buf->data;
+	data_ptr = rx_buf->data_ptr;
 	if (unlikely(cons != rxr->rx_next_cons)) {
 		int rc1 = bnxt_discard_rx(bp, bnapi, raw_cons, rxcmp);
 
 		bnxt_sched_reset(bp, rxr);
 		return rc1;
 	}
-	prefetch(data);
+	prefetch(data_ptr);
 
 	agg_bufs = (le32_to_cpu(rxcmp->rx_cmp_misc_v1) & RX_CMP_AGG_BUFS) >>
 				RX_CMP_AGG_BUFS_SHIFT;
@@ -1396,14 +1406,14 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 	dma_addr = dma_unmap_addr(rx_buf, mapping);
 
 	if (len <= bp->rx_copy_thresh) {
-		skb = bnxt_copy_skb(bnapi, data, len, dma_addr);
+		skb = bnxt_copy_skb(bnapi, data_ptr, len, dma_addr);
 		bnxt_reuse_rx_data(rxr, cons, data);
 		if (!skb) {
 			rc = -ENOMEM;
 			goto next_rx;
 		}
 	} else {
-		skb = bnxt_rx_skb(bp, rxr, cons, prod, data, dma_addr, len);
+		skb = bp->rx_skb_func(bp, rxr, cons, prod, data, dma_addr, len);
 		if (!skb) {
 			rc = -ENOMEM;
 			goto next_rx;
@@ -1880,7 +1890,7 @@ static void bnxt_free_rx_skbs(struct bnxt *bp)
 
 		for (j = 0; j < max_idx; j++) {
 			struct bnxt_sw_rx_bd *rx_buf = &rxr->rx_buf_ring[j];
-			u8 *data = rx_buf->data;
+			void *data = rx_buf->data;
 
 			if (!data)
 				continue;
@@ -2326,6 +2336,7 @@ static int bnxt_init_one_rx_ring(struct bnxt *bp, int ring_nr)
 					return -ENOMEM;
 
 				rxr->rx_tpa[i].data = data;
+				rxr->rx_tpa[i].data_ptr = data + BNXT_RX_OFFSET;
 				rxr->rx_tpa[i].mapping = mapping;
 			}
 		} else {
@@ -2550,6 +2561,12 @@ void bnxt_set_ring_params(struct bnxt *bp)
 	bp->cp_ring_mask = bp->cp_bit - 1;
 }
 
+static int bnxt_set_rx_skb_mode(struct bnxt *bp)
+{
+	bp->rx_skb_func = bnxt_rx_skb;
+	return 0;
+}
+
 static void bnxt_free_vnic_attributes(struct bnxt *bp)
 {
 	int i;
@@ -7299,6 +7316,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	bnxt_hwrm_func_qcfg(bp);
 	bnxt_hwrm_port_led_qcaps(bp);
 
+	bnxt_set_rx_skb_mode(bp);
 	bnxt_set_tpa_flags(bp);
 	bnxt_set_ring_params(bp);
 	bnxt_set_max_func_irqs(bp, max_irqs);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 52a1cc0..33302b0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -516,7 +516,8 @@ struct bnxt_sw_tx_bd {
 };
 
 struct bnxt_sw_rx_bd {
-	u8			*data;
+	void			*data;
+	u8			*data_ptr;
 	DEFINE_DMA_UNMAP_ADDR(mapping);
 };
 
@@ -576,7 +577,8 @@ struct bnxt_tx_ring_info {
 };
 
 struct bnxt_tpa_info {
-	u8			*data;
+	void			*data;
+	u8			*data_ptr;
 	dma_addr_t		mapping;
 	u16			len;
 	unsigned short		gso_type;
@@ -988,6 +990,11 @@ struct bnxt {
 	struct sk_buff *	(*gro_func)(struct bnxt_tpa_info *, int, int,
 					    struct sk_buff *);
 
+	struct sk_buff *	(*rx_skb_func)(struct bnxt *,
+					       struct bnxt_rx_ring_info *,
+					       u16, u16, void *, dma_addr_t,
+					       unsigned int);
+
 	u32			rx_buf_size;
 	u32			rx_buf_use_size;	/* useable size */
 	u32			rx_ring_size;
-- 
1.8.3.1

[PATCH net-next v2 02/12] bnxt_en: Don't use DEFINE_DMA_UNMAP_ADDR to store DMA address in RX path.

[Michael Chan]

To support XDP_TX, we need the RX buffer's DMA address to transmit the
packet.  Convert the DMA address field to a permanent field in
bnxt_sw_rx_bd.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 21 +++++++++------------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  2 +-
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 48fb719..c67d74d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -608,7 +608,7 @@ static inline int bnxt_alloc_rx_data(struct bnxt *bp,
 
 	rx_buf->data = data;
 	rx_buf->data_ptr = data + BNXT_RX_OFFSET;
-	dma_unmap_addr_set(rx_buf, mapping, mapping);
+	rx_buf->mapping = mapping;
 
 	rxbd->rx_bd_haddr = cpu_to_le64(mapping);
 
@@ -628,8 +628,7 @@ static void bnxt_reuse_rx_data(struct bnxt_rx_ring_info *rxr, u16 cons,
 	prod_rx_buf->data = data;
 	prod_rx_buf->data_ptr = cons_rx_buf->data_ptr;
 
-	dma_unmap_addr_set(prod_rx_buf, mapping,
-			   dma_unmap_addr(cons_rx_buf, mapping));
+	prod_rx_buf->mapping = cons_rx_buf->mapping;
 
 	prod_bd = &rxr->rx_desc_ring[RX_RING(prod)][RX_IDX(prod)];
 	cons_bd = &rxr->rx_desc_ring[RX_RING(cons)][RX_IDX(cons)];
@@ -814,7 +813,7 @@ static struct sk_buff *bnxt_rx_pages(struct bnxt *bp, struct bnxt_napi *bnapi,
 		 * a sw_prod index that equals the cons index, so we
 		 * need to clear the cons entry now.
 		 */
-		mapping = dma_unmap_addr(cons_rx_buf, mapping);
+		mapping = cons_rx_buf->mapping;
 		page = cons_rx_buf->page;
 		cons_rx_buf->page = NULL;
 
@@ -957,7 +956,7 @@ static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
 	prod_rx_buf->data_ptr = tpa_info->data_ptr;
 
 	mapping = tpa_info->mapping;
-	dma_unmap_addr_set(prod_rx_buf, mapping, mapping);
+	prod_rx_buf->mapping = mapping;
 
 	prod_bd = &rxr->rx_desc_ring[RX_RING(prod)][RX_IDX(prod)];
 
@@ -966,7 +965,7 @@ static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
 	tpa_info->data = cons_rx_buf->data;
 	tpa_info->data_ptr = cons_rx_buf->data_ptr;
 	cons_rx_buf->data = NULL;
-	tpa_info->mapping = dma_unmap_addr(cons_rx_buf, mapping);
+	tpa_info->mapping = cons_rx_buf->mapping;
 
 	tpa_info->len =
 		le32_to_cpu(tpa_start->rx_tpa_start_cmp_len_flags_type) >>
@@ -1403,7 +1402,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 	}
 
 	len = le32_to_cpu(rxcmp->rx_cmp_len_flags_type) >> RX_CMP_LEN_SHIFT;
-	dma_addr = dma_unmap_addr(rx_buf, mapping);
+	dma_addr = rx_buf->mapping;
 
 	if (len <= bp->rx_copy_thresh) {
 		skb = bnxt_copy_skb(bnapi, data_ptr, len, dma_addr);
@@ -1878,7 +1877,7 @@ static void bnxt_free_rx_skbs(struct bnxt *bp)
 
 				dma_unmap_single(
 					&pdev->dev,
-					dma_unmap_addr(tpa_info, mapping),
+					tpa_info->mapping,
 					bp->rx_buf_use_size,
 					PCI_DMA_FROMDEVICE);
 
@@ -1895,8 +1894,7 @@ static void bnxt_free_rx_skbs(struct bnxt *bp)
 			if (!data)
 				continue;
 
-			dma_unmap_single(&pdev->dev,
-					 dma_unmap_addr(rx_buf, mapping),
+			dma_unmap_single(&pdev->dev, rx_buf->mapping,
 					 bp->rx_buf_use_size,
 					 PCI_DMA_FROMDEVICE);
 
@@ -1913,8 +1911,7 @@ static void bnxt_free_rx_skbs(struct bnxt *bp)
 			if (!page)
 				continue;
 
-			dma_unmap_page(&pdev->dev,
-				       dma_unmap_addr(rx_agg_buf, mapping),
+			dma_unmap_page(&pdev->dev, rx_agg_buf->mapping,
 				       BNXT_RX_PAGE_SIZE, PCI_DMA_FROMDEVICE);
 
 			rx_agg_buf->page = NULL;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 33302b0..1707c8b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -518,7 +518,7 @@ struct bnxt_sw_tx_bd {
 struct bnxt_sw_rx_bd {
 	void			*data;
 	u8			*data_ptr;
-	DEFINE_DMA_UNMAP_ADDR(mapping);
+	dma_addr_t		mapping;
 };
 
 struct bnxt_sw_rx_agg_bd {
-- 
1.8.3.1

[PATCH net-next v2 03/12] bnxt_en: Add bp->rx_dir field for rx buffer DMA direction.

[Michael Chan]

When driver is running in XDP mode, rx buffers are DMA mapped as
DMA_BIDIRECTIONAL.  Add a field so the code will map/unmap rx buffers
according to this field.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 27 ++++++++++++---------------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  1 +
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index c67d74d..7a6a1f8 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -584,7 +584,7 @@ static inline u8 *__bnxt_alloc_rx_data(struct bnxt *bp, dma_addr_t *mapping,
 		return NULL;
 
 	*mapping = dma_map_single(&pdev->dev, data + BNXT_RX_DMA_OFFSET,
-				  bp->rx_buf_use_size, PCI_DMA_FROMDEVICE);
+				  bp->rx_buf_use_size, bp->rx_dir);
 
 	if (dma_mapping_error(&pdev->dev, *mapping)) {
 		kfree(data);
@@ -770,7 +770,7 @@ static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
 
 	skb = build_skb(data, 0);
 	dma_unmap_single(&bp->pdev->dev, dma_addr, bp->rx_buf_use_size,
-			 PCI_DMA_FROMDEVICE);
+			 bp->rx_dir);
 	if (!skb) {
 		kfree(data);
 		return NULL;
@@ -876,15 +876,14 @@ static inline struct sk_buff *bnxt_copy_skb(struct bnxt_napi *bnapi, u8 *data,
 	if (!skb)
 		return NULL;
 
-	dma_sync_single_for_cpu(&pdev->dev, mapping,
-				bp->rx_copy_thresh, PCI_DMA_FROMDEVICE);
+	dma_sync_single_for_cpu(&pdev->dev, mapping, bp->rx_copy_thresh,
+				bp->rx_dir);
 
 	memcpy(skb->data - NET_IP_ALIGN, data - NET_IP_ALIGN,
 	       len + NET_IP_ALIGN);
 
-	dma_sync_single_for_device(&pdev->dev, mapping,
-				   bp->rx_copy_thresh,
-				   PCI_DMA_FROMDEVICE);
+	dma_sync_single_for_device(&pdev->dev, mapping, bp->rx_copy_thresh,
+				   bp->rx_dir);
 
 	skb_put(skb, len);
 	return skb;
@@ -1259,7 +1258,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 
 		skb = build_skb(data, 0);
 		dma_unmap_single(&bp->pdev->dev, mapping, bp->rx_buf_use_size,
-				 PCI_DMA_FROMDEVICE);
+				 bp->rx_dir);
 
 		if (!skb) {
 			kfree(data);
@@ -1875,11 +1874,9 @@ static void bnxt_free_rx_skbs(struct bnxt *bp)
 				if (!data)
 					continue;
 
-				dma_unmap_single(
-					&pdev->dev,
-					tpa_info->mapping,
-					bp->rx_buf_use_size,
-					PCI_DMA_FROMDEVICE);
+				dma_unmap_single(&pdev->dev, tpa_info->mapping,
+						 bp->rx_buf_use_size,
+						 bp->rx_dir);
 
 				tpa_info->data = NULL;
 
@@ -1895,8 +1892,7 @@ static void bnxt_free_rx_skbs(struct bnxt *bp)
 				continue;
 
 			dma_unmap_single(&pdev->dev, rx_buf->mapping,
-					 bp->rx_buf_use_size,
-					 PCI_DMA_FROMDEVICE);
+					 bp->rx_buf_use_size, bp->rx_dir);
 
 			rx_buf->data = NULL;
 
@@ -2560,6 +2556,7 @@ void bnxt_set_ring_params(struct bnxt *bp)
 
 static int bnxt_set_rx_skb_mode(struct bnxt *bp)
 {
+	bp->rx_dir = DMA_FROM_DEVICE;
 	bp->rx_skb_func = bnxt_rx_skb;
 	return 0;
 }
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 1707c8b..b8bad06 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -997,6 +997,7 @@ struct bnxt {
 
 	u32			rx_buf_size;
 	u32			rx_buf_use_size;	/* useable size */
+	enum dma_data_direction	rx_dir;
 	u32			rx_ring_size;
 	u32			rx_agg_ring_size;
 	u32			rx_copy_thresh;
-- 
1.8.3.1

[PATCH net-next v2 04/12] bnxt_en: Parameterize RX buffer offsets.

[Michael Chan]

Convert the global constants BNXT_RX_OFFSET and BNXT_RX_DMA_OFFSET to
device parameters.  This will make it easier to support XDP with
headroom support which requires different RX buffer offsets.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 15 +++++++++------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  2 ++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 7a6a1f8..77d1074 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -583,7 +583,7 @@ static inline u8 *__bnxt_alloc_rx_data(struct bnxt *bp, dma_addr_t *mapping,
 	if (!data)
 		return NULL;
 
-	*mapping = dma_map_single(&pdev->dev, data + BNXT_RX_DMA_OFFSET,
+	*mapping = dma_map_single(&pdev->dev, data + bp->rx_dma_offset,
 				  bp->rx_buf_use_size, bp->rx_dir);
 
 	if (dma_mapping_error(&pdev->dev, *mapping)) {
@@ -607,7 +607,7 @@ static inline int bnxt_alloc_rx_data(struct bnxt *bp,
 		return -ENOMEM;
 
 	rx_buf->data = data;
-	rx_buf->data_ptr = data + BNXT_RX_OFFSET;
+	rx_buf->data_ptr = data + bp->rx_offset;
 	rx_buf->mapping = mapping;
 
 	rxbd->rx_bd_haddr = cpu_to_le64(mapping);
@@ -776,7 +776,7 @@ static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
 		return NULL;
 	}
 
-	skb_reserve(skb, BNXT_RX_OFFSET);
+	skb_reserve(skb, bp->rx_offset);
 	skb_put(skb, offset_and_len & 0xffff);
 	return skb;
 }
@@ -1253,7 +1253,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 		}
 
 		tpa_info->data = new_data;
-		tpa_info->data_ptr = new_data + BNXT_RX_OFFSET;
+		tpa_info->data_ptr = new_data + bp->rx_offset;
 		tpa_info->mapping = new_mapping;
 
 		skb = build_skb(data, 0);
@@ -1265,7 +1265,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 			bnxt_abort_tpa(bp, bnapi, cp_cons, agg_bufs);
 			return NULL;
 		}
-		skb_reserve(skb, BNXT_RX_OFFSET);
+		skb_reserve(skb, bp->rx_offset);
 		skb_put(skb, len);
 	}
 
@@ -2329,7 +2329,7 @@ static int bnxt_init_one_rx_ring(struct bnxt *bp, int ring_nr)
 					return -ENOMEM;
 
 				rxr->rx_tpa[i].data = data;
-				rxr->rx_tpa[i].data_ptr = data + BNXT_RX_OFFSET;
+				rxr->rx_tpa[i].data_ptr = data + bp->rx_offset;
 				rxr->rx_tpa[i].mapping = mapping;
 			}
 		} else {
@@ -2345,6 +2345,9 @@ static int bnxt_init_rx_rings(struct bnxt *bp)
 {
 	int i, rc = 0;
 
+	bp->rx_offset = BNXT_RX_OFFSET;
+	bp->rx_dma_offset = BNXT_RX_DMA_OFFSET;
+
 	for (i = 0; i < bp->rx_nr_rings; i++) {
 		rc = bnxt_init_one_rx_ring(bp, i);
 		if (rc)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index b8bad06..5e0ec9b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -997,6 +997,8 @@ struct bnxt {
 
 	u32			rx_buf_size;
 	u32			rx_buf_use_size;	/* useable size */
+	u16			rx_offset;
+	u16			rx_dma_offset;
 	enum dma_data_direction	rx_dir;
 	u32			rx_ring_size;
 	u32			rx_agg_ring_size;
-- 
1.8.3.1

[PATCH net-next v2 05/12] bnxt_en: Add RX page mode support.

[Michael Chan]

This mode is to support XDP.  In this mode, each rx ring is configured
with page sized buffers for linear placement of each packet.  MTU will be
restricted to what the page sized buffers can support.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 133 ++++++++++++++++++++++++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |   7 ++
 2 files changed, 124 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 77d1074..61f041b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -573,6 +573,25 @@ static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
 	}
 }
 
+static struct page *__bnxt_alloc_rx_page(struct bnxt *bp, dma_addr_t *mapping,
+					 gfp_t gfp)
+{
+	struct device *dev = &bp->pdev->dev;
+	struct page *page;
+
+	page = alloc_page(gfp);
+	if (!page)
+		return NULL;
+
+	*mapping = dma_map_page(dev, page, 0, PAGE_SIZE, bp->rx_dir);
+	if (dma_mapping_error(dev, *mapping)) {
+		__free_page(page);
+		return NULL;
+	}
+	*mapping += bp->rx_dma_offset;
+	return page;
+}
+
 static inline u8 *__bnxt_alloc_rx_data(struct bnxt *bp, dma_addr_t *mapping,
 				       gfp_t gfp)
 {
@@ -599,19 +618,28 @@ static inline int bnxt_alloc_rx_data(struct bnxt *bp,
 {
 	struct rx_bd *rxbd = &rxr->rx_desc_ring[RX_RING(prod)][RX_IDX(prod)];
 	struct bnxt_sw_rx_bd *rx_buf = &rxr->rx_buf_ring[prod];
-	u8 *data;
 	dma_addr_t mapping;
 
-	data = __bnxt_alloc_rx_data(bp, &mapping, gfp);
-	if (!data)
-		return -ENOMEM;
+	if (BNXT_RX_PAGE_MODE(bp)) {
+		struct page *page = __bnxt_alloc_rx_page(bp, &mapping, gfp);
 
-	rx_buf->data = data;
-	rx_buf->data_ptr = data + bp->rx_offset;
+		if (!page)
+			return -ENOMEM;
+
+		rx_buf->data = page;
+		rx_buf->data_ptr = page_address(page) + bp->rx_offset;
+	} else {
+		u8 *data = __bnxt_alloc_rx_data(bp, &mapping, gfp);
+
+		if (!data)
+			return -ENOMEM;
+
+		rx_buf->data = data;
+		rx_buf->data_ptr = data + bp->rx_offset;
+	}
 	rx_buf->mapping = mapping;
 
 	rxbd->rx_bd_haddr = cpu_to_le64(mapping);
-
 	return 0;
 }
 
@@ -754,6 +782,52 @@ static void bnxt_reuse_rx_agg_bufs(struct bnxt_napi *bnapi, u16 cp_cons,
 	rxr->rx_sw_agg_prod = sw_prod;
 }
 
+static struct sk_buff *bnxt_rx_page_skb(struct bnxt *bp,
+					struct bnxt_rx_ring_info *rxr,
+					u16 cons, u16 prod,
+					void *data, dma_addr_t dma_addr,
+					unsigned int offset_and_len)
+{
+	struct bnxt_sw_rx_bd *cons_rx_buf = &rxr->rx_buf_ring[cons];
+	unsigned int payload = offset_and_len >> 16;
+	unsigned int len = offset_and_len & 0xffff;
+	struct skb_frag_struct *frag;
+	struct page *page = data;
+	struct sk_buff *skb;
+	char *data_ptr;
+	int err;
+
+	err = bnxt_alloc_rx_data(bp, rxr, prod, GFP_ATOMIC);
+	if (unlikely(err)) {
+		bnxt_reuse_rx_data(rxr, cons, data);
+		return NULL;
+	}
+	dma_addr -= bp->rx_dma_offset;
+	dma_unmap_page(&bp->pdev->dev, dma_addr, PAGE_SIZE, bp->rx_dir);
+
+	data_ptr = cons_rx_buf->data_ptr;
+	if (unlikely(!payload))
+		payload = eth_get_headlen(data_ptr, len);
+
+	skb = napi_alloc_skb(&rxr->bnapi->napi, payload);
+	if (!skb) {
+		__free_page(page);
+		return NULL;
+	}
+
+	skb_add_rx_frag(skb, 0, page, bp->rx_offset, len, PAGE_SIZE);
+	memcpy(skb->data - NET_IP_ALIGN, data_ptr - NET_IP_ALIGN,
+	       payload + NET_IP_ALIGN);
+
+	frag = &skb_shinfo(skb)->frags[0];
+	skb_frag_size_sub(frag, payload);
+	frag->page_offset += payload;
+	skb->data_len -= payload;
+	skb->tail += payload;
+
+	return skb;
+}
+
 static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
 				   struct bnxt_rx_ring_info *rxr, u16 cons,
 				   u16 prod, void *data, dma_addr_t dma_addr,
@@ -1411,7 +1485,12 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 			goto next_rx;
 		}
 	} else {
-		skb = bp->rx_skb_func(bp, rxr, cons, prod, data, dma_addr, len);
+		unsigned int payload_len;
+
+		payload_len = (le32_to_cpu(rxcmp->rx_cmp_misc_v1) &
+			       RX_CMP_PAYLOAD_OFFSET) | len;
+		skb = bp->rx_skb_func(bp, rxr, cons, prod, data, dma_addr,
+				      payload_len);
 		if (!skb) {
 			rc = -ENOMEM;
 			goto next_rx;
@@ -1896,7 +1975,10 @@ static void bnxt_free_rx_skbs(struct bnxt *bp)
 
 			rx_buf->data = NULL;
 
-			kfree(data);
+			if (BNXT_RX_PAGE_MODE(bp))
+				__free_page(data);
+			else
+				kfree(data);
 		}
 
 		for (j = 0; j < max_agg_idx; j++) {
@@ -2345,8 +2427,13 @@ static int bnxt_init_rx_rings(struct bnxt *bp)
 {
 	int i, rc = 0;
 
-	bp->rx_offset = BNXT_RX_OFFSET;
-	bp->rx_dma_offset = BNXT_RX_DMA_OFFSET;
+	if (BNXT_RX_PAGE_MODE(bp)) {
+		bp->rx_offset = NET_IP_ALIGN;
+		bp->rx_dma_offset = 0;
+	} else {
+		bp->rx_offset = BNXT_RX_OFFSET;
+		bp->rx_dma_offset = BNXT_RX_DMA_OFFSET;
+	}
 
 	for (i = 0; i < bp->rx_nr_rings; i++) {
 		rc = bnxt_init_one_rx_ring(bp, i);
@@ -2557,10 +2644,24 @@ void bnxt_set_ring_params(struct bnxt *bp)
 	bp->cp_ring_mask = bp->cp_bit - 1;
 }
 
-static int bnxt_set_rx_skb_mode(struct bnxt *bp)
+int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode)
 {
-	bp->rx_dir = DMA_FROM_DEVICE;
-	bp->rx_skb_func = bnxt_rx_skb;
+	if (page_mode) {
+		if (bp->dev->mtu > BNXT_MAX_PAGE_MODE_MTU)
+			return -EOPNOTSUPP;
+		bp->dev->max_mtu = BNXT_MAX_PAGE_MODE_MTU;
+		bp->flags &= ~BNXT_FLAG_AGG_RINGS;
+		bp->flags |= BNXT_FLAG_NO_AGG_RINGS | BNXT_FLAG_RX_PAGE_MODE;
+		bp->dev->hw_features &= ~NETIF_F_LRO;
+		bp->dev->features &= ~NETIF_F_LRO;
+		bp->rx_dir = DMA_BIDIRECTIONAL;
+		bp->rx_skb_func = bnxt_rx_page_skb;
+	} else {
+		bp->dev->max_mtu = BNXT_MAX_MTU;
+		bp->flags &= ~BNXT_FLAG_RX_PAGE_MODE;
+		bp->rx_dir = DMA_FROM_DEVICE;
+		bp->rx_skb_func = bnxt_rx_skb;
+	}
 	return 0;
 }
 
@@ -7272,7 +7373,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	/* MTU range: 60 - 9500 */
 	dev->min_mtu = ETH_ZLEN;
-	dev->max_mtu = 9500;
+	dev->max_mtu = BNXT_MAX_MTU;
 
 	bnxt_dcb_init(bp);
 
@@ -7313,7 +7414,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	bnxt_hwrm_func_qcfg(bp);
 	bnxt_hwrm_port_led_qcaps(bp);
 
-	bnxt_set_rx_skb_mode(bp);
+	bnxt_set_rx_skb_mode(bp, false);
 	bnxt_set_tpa_flags(bp);
 	bnxt_set_ring_params(bp);
 	bnxt_set_max_func_irqs(bp, max_irqs);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 5e0ec9b..f5a9516 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -416,6 +416,10 @@ struct rx_tpa_end_cmp_ext {
 
 #define BNXT_RX_PAGE_SIZE (1 << BNXT_RX_PAGE_SHIFT)
 
+#define BNXT_MAX_MTU		9500
+#define BNXT_MAX_PAGE_MODE_MTU	\
+	((unsigned int)PAGE_SIZE - VLAN_ETH_HLEN - NET_IP_ALIGN)
+
 #define BNXT_MIN_PKT_SIZE	52
 
 #define BNXT_NUM_TESTS(bp)	0
@@ -967,6 +971,7 @@ struct bnxt {
 	#define BNXT_FLAG_ROCE_CAP	(BNXT_FLAG_ROCEV1_CAP |	\
 					 BNXT_FLAG_ROCEV2_CAP)
 	#define BNXT_FLAG_NO_AGG_RINGS	0x20000
+	#define BNXT_FLAG_RX_PAGE_MODE	0x40000
 	#define BNXT_FLAG_CHIP_NITRO_A0	0x1000000
 
 	#define BNXT_FLAG_ALL_CONFIG_FEATS (BNXT_FLAG_TPA |		\
@@ -978,6 +983,7 @@ struct bnxt {
 #define BNXT_NPAR(bp)		((bp)->port_partition_type)
 #define BNXT_SINGLE_PF(bp)	(BNXT_PF(bp) && !BNXT_NPAR(bp))
 #define BNXT_CHIP_TYPE_NITRO_A0(bp) ((bp)->flags & BNXT_FLAG_CHIP_NITRO_A0)
+#define BNXT_RX_PAGE_MODE(bp)	((bp)->flags & BNXT_FLAG_RX_PAGE_MODE)
 
 	struct bnxt_en_dev	*edev;
 	struct bnxt_en_dev *	(*ulp_probe)(struct net_device *);
@@ -1170,6 +1176,7 @@ struct bnxt {
 #define BNXT_MAX_PHY_I2C_RESP_SIZE		64
 
 void bnxt_set_ring_params(struct bnxt *);
+int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode);
 void bnxt_hwrm_cmd_hdr_init(struct bnxt *, void *, u16, u16, u16);
 int _hwrm_send_message(struct bnxt *, void *, u32, int);
 int hwrm_send_message(struct bnxt *, void *, u32, int);
-- 
1.8.3.1

[PATCH net-next v2 06/12] bnxt_en: Use event bit map in RX path.

[Michael Chan]

In the current code, we have separate rx_event and agg_event parameters
to keep track of rx and aggregation events.  Combine these events into
an u8 event mask with different bits defined for different events.  This
way, it is easier to expand the logic to include XDP tx events.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 28 +++++++++++++---------------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  3 +++
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 61f041b..ef42c32 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1264,7 +1264,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 					   u32 *raw_cons,
 					   struct rx_tpa_end_cmp *tpa_end,
 					   struct rx_tpa_end_cmp_ext *tpa_end1,
-					   bool *agg_event)
+					   u8 *event)
 {
 	struct bnxt_cp_ring_info *cpr = &bnapi->cp_ring;
 	struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
@@ -1299,7 +1299,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
 		if (!bnxt_agg_bufs_valid(bp, cpr, agg_bufs, raw_cons))
 			return ERR_PTR(-EBUSY);
 
-		*agg_event = true;
+		*event |= BNXT_AGG_EVENT;
 		cp_cons = NEXT_CMP(cp_cons);
 	}
 
@@ -1385,7 +1385,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
  * -EIO    - packet aborted due to hw error indicated in BD
  */
 static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
-		       bool *agg_event)
+		       u8 *event)
 {
 	struct bnxt_cp_ring_info *cpr = &bnapi->cp_ring;
 	struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
@@ -1426,8 +1426,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 	} else if (cmp_type == CMP_TYPE_RX_L2_TPA_END_CMP) {
 		skb = bnxt_tpa_end(bp, bnapi, &tmp_raw_cons,
 				   (struct rx_tpa_end_cmp *)rxcmp,
-				   (struct rx_tpa_end_cmp_ext *)rxcmp1,
-				   agg_event);
+				   (struct rx_tpa_end_cmp_ext *)rxcmp1, event);
 
 		if (unlikely(IS_ERR(skb)))
 			return -EBUSY;
@@ -1461,7 +1460,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 			return -EBUSY;
 
 		cp_cons = NEXT_CMP(cp_cons);
-		*agg_event = true;
+		*event |= BNXT_AGG_EVENT;
 	}
 
 	rx_buf->data = NULL;
@@ -1714,8 +1713,7 @@ static int bnxt_poll_work(struct bnxt *bp, struct bnxt_napi *bnapi, int budget)
 	u32 cons;
 	int tx_pkts = 0;
 	int rx_pkts = 0;
-	bool rx_event = false;
-	bool agg_event = false;
+	u8 event = 0;
 	struct tx_cmp *txcmp;
 
 	while (1) {
@@ -1737,12 +1735,12 @@ static int bnxt_poll_work(struct bnxt *bp, struct bnxt_napi *bnapi, int budget)
 			if (unlikely(tx_pkts > bp->tx_wake_thresh))
 				rx_pkts = budget;
 		} else if ((TX_CMP_TYPE(txcmp) & 0x30) == 0x10) {
-			rc = bnxt_rx_pkt(bp, bnapi, &raw_cons, &agg_event);
+			rc = bnxt_rx_pkt(bp, bnapi, &raw_cons, &event);
 			if (likely(rc >= 0))
 				rx_pkts += rc;
 			else if (rc == -EBUSY)	/* partial completion */
 				break;
-			rx_event = true;
+			event |= BNXT_RX_EVENT;
 		} else if (unlikely((TX_CMP_TYPE(txcmp) ==
 				     CMPL_BASE_TYPE_HWRM_DONE) ||
 				    (TX_CMP_TYPE(txcmp) ==
@@ -1767,12 +1765,12 @@ static int bnxt_poll_work(struct bnxt *bp, struct bnxt_napi *bnapi, int budget)
 	if (tx_pkts)
 		bnxt_tx_int(bp, bnapi, tx_pkts);
 
-	if (rx_event) {
+	if (event & BNXT_RX_EVENT) {
 		struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
 
 		writel(DB_KEY_RX | rxr->rx_prod, rxr->rx_doorbell);
 		writel(DB_KEY_RX | rxr->rx_prod, rxr->rx_doorbell);
-		if (agg_event) {
+		if (event & BNXT_AGG_EVENT) {
 			writel(DB_KEY_RX | rxr->rx_agg_prod,
 			       rxr->rx_agg_doorbell);
 			writel(DB_KEY_RX | rxr->rx_agg_prod,
@@ -1793,7 +1791,7 @@ static int bnxt_poll_nitroa0(struct napi_struct *napi, int budget)
 	u32 cp_cons, tmp_raw_cons;
 	u32 raw_cons = cpr->cp_raw_cons;
 	u32 rx_pkts = 0;
-	bool agg_event = false;
+	u8 event = 0;
 
 	while (1) {
 		int rc;
@@ -1817,7 +1815,7 @@ static int bnxt_poll_nitroa0(struct napi_struct *napi, int budget)
 			rxcmp1->rx_cmp_cfa_code_errors_v2 |=
 				cpu_to_le32(RX_CMPL_ERRORS_CRC_ERROR);
 
-			rc = bnxt_rx_pkt(bp, bnapi, &raw_cons, &agg_event);
+			rc = bnxt_rx_pkt(bp, bnapi, &raw_cons, &event);
 			if (likely(rc == -EIO))
 				rx_pkts++;
 			else if (rc == -EBUSY)	/* partial completion */
@@ -1840,7 +1838,7 @@ static int bnxt_poll_nitroa0(struct napi_struct *napi, int budget)
 	writel(DB_KEY_RX | rxr->rx_prod, rxr->rx_doorbell);
 	writel(DB_KEY_RX | rxr->rx_prod, rxr->rx_doorbell);
 
-	if (agg_event) {
+	if (event & BNXT_AGG_EVENT) {
 		writel(DB_KEY_RX | rxr->rx_agg_prod, rxr->rx_agg_doorbell);
 		writel(DB_KEY_RX | rxr->rx_agg_prod, rxr->rx_agg_doorbell);
 	}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index f5a9516..630e26a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -511,6 +511,9 @@ struct rx_tpa_end_cmp_ext {
 #define BNXT_HWRM_REQS_PER_PAGE		(BNXT_PAGE_SIZE /	\
 					 BNXT_HWRM_REQ_MAX_SIZE)
 
+#define BNXT_RX_EVENT	1
+#define BNXT_AGG_EVENT	2
+
 struct bnxt_sw_tx_bd {
 	struct sk_buff		*skb;
 	DEFINE_DMA_UNMAP_ADDR(mapping);
-- 
1.8.3.1

[PATCH net-next v2 07/12] bnxt_en: Centralize logic to reserve rings.

[Michael Chan]

Currently, bnxt_setup_tc() and bnxt_set_channels() have similar and
duplicated code to check and reserve rx and tx rings.  Add a new
function bnxt_reserve_rings() to centralize the logic.  This will
make it easier to add XDP_TX support which requires allocating a
new set of TX rings.

Also, the tx ring checking logic in bnxt_setup_msix() can be removed.
The rings have been reserved before hand.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 68 ++++++++++++++---------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 39 +++----------
 3 files changed, 52 insertions(+), 57 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index ef42c32..91fc72d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -4209,7 +4209,7 @@ int __bnxt_hwrm_get_tx_rings(struct bnxt *bp, u16 fid, int *tx_rings)
 	return rc;
 }
 
-int bnxt_hwrm_reserve_tx_rings(struct bnxt *bp, int *tx_rings)
+static int bnxt_hwrm_reserve_tx_rings(struct bnxt *bp, int *tx_rings)
 {
 	struct hwrm_func_cfg_input req = {0};
 	int rc;
@@ -5002,19 +5002,12 @@ static void bnxt_setup_msix(struct bnxt *bp)
 
 	tcs = netdev_get_num_tc(dev);
 	if (tcs > 1) {
-		bp->tx_nr_rings_per_tc = bp->tx_nr_rings / tcs;
-		if (bp->tx_nr_rings_per_tc == 0) {
-			netdev_reset_tc(dev);
-			bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
-		} else {
-			int i, off, count;
+		int i, off, count;
 
-			bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tcs;
-			for (i = 0; i < tcs; i++) {
-				count = bp->tx_nr_rings_per_tc;
-				off = i * count;
-				netdev_set_tc_queue(dev, i, count, off);
-			}
+		for (i = 0; i < tcs; i++) {
+			count = bp->tx_nr_rings_per_tc;
+			off = i * count;
+			netdev_set_tc_queue(dev, i, count, off);
 		}
 	}
 
@@ -6576,6 +6569,37 @@ static void bnxt_sp_task(struct work_struct *work)
 	clear_bit(BNXT_STATE_IN_SP_TASK, &bp->state);
 }
 
+/* Under rtnl_lock */
+int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs)
+{
+	int max_rx, max_tx, tx_sets = 1;
+	int tx_rings_needed;
+	bool sh = true;
+	int rc;
+
+	if (!(bp->flags & BNXT_FLAG_SHARED_RINGS))
+		sh = false;
+
+	if (tcs)
+		tx_sets = tcs;
+
+	rc = bnxt_get_max_rings(bp, &max_rx, &max_tx, sh);
+	if (rc)
+		return rc;
+
+	if (max_rx < rx)
+		return -ENOMEM;
+
+	tx_rings_needed = tx * tx_sets;
+	if (max_tx < tx_rings_needed)
+		return -ENOMEM;
+
+	if (bnxt_hwrm_reserve_tx_rings(bp, &tx_rings_needed) ||
+	    tx_rings_needed < (tx * tx_sets))
+		return -ENOMEM;
+	return 0;
+}
+
 static int bnxt_init_board(struct pci_dev *pdev, struct net_device *dev)
 {
 	int rc;
@@ -6738,6 +6762,7 @@ int bnxt_setup_mq_tc(struct net_device *dev, u8 tc)
 {
 	struct bnxt *bp = netdev_priv(dev);
 	bool sh = false;
+	int rc;
 
 	if (tc > bp->max_tc) {
 		netdev_err(dev, "too many traffic classes requested: %d Max supported is %d\n",
@@ -6751,19 +6776,10 @@ int bnxt_setup_mq_tc(struct net_device *dev, u8 tc)
 	if (bp->flags & BNXT_FLAG_SHARED_RINGS)
 		sh = true;
 
-	if (tc) {
-		int max_rx_rings, max_tx_rings, req_tx_rings, rsv_tx_rings, rc;
-
-		req_tx_rings = bp->tx_nr_rings_per_tc * tc;
-		rc = bnxt_get_max_rings(bp, &max_rx_rings, &max_tx_rings, sh);
-		if (rc || req_tx_rings > max_tx_rings)
-			return -ENOMEM;
-
-		rsv_tx_rings = req_tx_rings;
-		if (bnxt_hwrm_reserve_tx_rings(bp, &rsv_tx_rings) ||
-		    rsv_tx_rings < req_tx_rings)
-			return -ENOMEM;
-	}
+	rc = bnxt_reserve_rings(bp, bp->tx_nr_rings_per_tc,
+				bp->rx_nr_rings, tc);
+	if (rc)
+		return rc;
 
 	/* Needs to close the device and do hw resource re-allocations */
 	if (netif_running(bp->dev))
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 630e26a..e1d9056 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1188,7 +1188,6 @@ int bnxt_hwrm_func_rgtr_async_events(struct bnxt *bp, unsigned long *bmap,
 				     int bmap_size);
 int bnxt_hwrm_vnic_cfg(struct bnxt *bp, u16 vnic_id);
 int __bnxt_hwrm_get_tx_rings(struct bnxt *bp, u16 fid, int *tx_rings);
-int bnxt_hwrm_reserve_tx_rings(struct bnxt *bp, int *tx_rings);
 int bnxt_hwrm_set_coal(struct bnxt *);
 unsigned int bnxt_get_max_func_stat_ctxs(struct bnxt *bp);
 void bnxt_set_max_func_stat_ctxs(struct bnxt *bp, unsigned int max);
@@ -1202,6 +1201,7 @@ int bnxt_hwrm_func_rgtr_async_events(struct bnxt *bp, unsigned long *bmap,
 int bnxt_hwrm_fw_set_time(struct bnxt *);
 int bnxt_open_nic(struct bnxt *, bool, bool);
 int bnxt_close_nic(struct bnxt *, bool, bool);
+int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs);
 int bnxt_setup_mq_tc(struct net_device *dev, u8 tc);
 int bnxt_get_max_rings(struct bnxt *, int *, int *, bool);
 void bnxt_restore_pf_fw_resources(struct bnxt *bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 24818e1..6f2568d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -387,10 +387,9 @@ static int bnxt_set_channels(struct net_device *dev,
 			     struct ethtool_channels *channel)
 {
 	struct bnxt *bp = netdev_priv(dev);
-	int max_rx_rings, max_tx_rings, tcs;
-	int req_tx_rings, rsv_tx_rings;
-	u32 rc = 0;
+	int req_tx_rings, req_rx_rings, tcs;
 	bool sh = false;
+	int rc = 0;
 
 	if (channel->other_count)
 		return -EINVAL;
@@ -410,32 +409,14 @@ static int bnxt_set_channels(struct net_device *dev,
 	if (channel->combined_count)
 		sh = true;
 
-	bnxt_get_max_rings(bp, &max_rx_rings, &max_tx_rings, sh);
-
 	tcs = netdev_get_num_tc(dev);
-	if (tcs > 1)
-		max_tx_rings /= tcs;
-
-	if (sh &&
-	    channel->combined_count > max_t(int, max_rx_rings, max_tx_rings))
-		return -ENOMEM;
-
-	if (!sh && (channel->rx_count > max_rx_rings ||
-		    channel->tx_count > max_tx_rings))
-		return -ENOMEM;
 
 	req_tx_rings = sh ? channel->combined_count : channel->tx_count;
-	req_tx_rings = min_t(int, req_tx_rings, max_tx_rings);
-	if (tcs > 1)
-		req_tx_rings *= tcs;
-
-	rsv_tx_rings = req_tx_rings;
-	if (bnxt_hwrm_reserve_tx_rings(bp, &rsv_tx_rings))
-		return -ENOMEM;
-
-	if (rsv_tx_rings < req_tx_rings) {
-		netdev_warn(dev, "Unable to allocate the requested tx rings\n");
-		return -ENOMEM;
+	req_rx_rings = sh ? channel->combined_count : channel->rx_count;
+	rc = bnxt_reserve_rings(bp, req_tx_rings, req_rx_rings, tcs);
+	if (rc) {
+		netdev_warn(dev, "Unable to allocate the requested rings\n");
+		return rc;
 	}
 
 	if (netif_running(dev)) {
@@ -454,10 +435,8 @@ static int bnxt_set_channels(struct net_device *dev,
 
 	if (sh) {
 		bp->flags |= BNXT_FLAG_SHARED_RINGS;
-		bp->rx_nr_rings = min_t(int, channel->combined_count,
-					max_rx_rings);
-		bp->tx_nr_rings_per_tc = min_t(int, channel->combined_count,
-					       max_tx_rings);
+		bp->rx_nr_rings = channel->combined_count;
+		bp->tx_nr_rings_per_tc = channel->combined_count;
 	} else {
 		bp->flags &= ~BNXT_FLAG_SHARED_RINGS;
 		bp->rx_nr_rings = channel->rx_count;
-- 
1.8.3.1

[PATCH net-next v2 08/12] bnxt_en: Add tx ring mapping logic.

[Michael Chan]

To support XDP_TX, we need to add a set of dedicated TX rings, each
associated with the NAPI of an RX ring.  To assign XDP rings and regular
rings in a flexible way, we add a bp->tx_ring_map[] array to do the
remapping.  The netdev txq index is stored in the new field txq_index
so that we can retrieve the netdev txq when handling TX completions.
In this patch, before we introduce XDP_TX, the mapping is 1:1.

v2: Fixed a bug in bnxt_tx_int().

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 15 ++++++++++++---
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  2 ++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 91fc72d..5d402e957 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -262,8 +262,8 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		return NETDEV_TX_OK;
 	}
 
-	txr = &bp->tx_ring[i];
 	txq = netdev_get_tx_queue(dev, i);
+	txr = &bp->tx_ring[bp->tx_ring_map[i]];
 	prod = txr->tx_prod;
 
 	free_size = bnxt_tx_avail(bp, txr);
@@ -509,8 +509,7 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, struct net_device *dev)
 static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
 {
 	struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
-	int index = txr - &bp->tx_ring[0];
-	struct netdev_queue *txq = netdev_get_tx_queue(bp->dev, index);
+	struct netdev_queue *txq = netdev_get_tx_queue(bp->dev, txr->txq_index);
 	u16 cons = txr->tx_cons;
 	struct pci_dev *pdev = bp->pdev;
 	int i;
@@ -2972,6 +2971,8 @@ static void bnxt_free_mem(struct bnxt *bp, bool irq_re_init)
 		bnxt_free_stats(bp);
 		bnxt_free_ring_grps(bp);
 		bnxt_free_vnics(bp);
+		kfree(bp->tx_ring_map);
+		bp->tx_ring_map = NULL;
 		kfree(bp->tx_ring);
 		bp->tx_ring = NULL;
 		kfree(bp->rx_ring);
@@ -3024,6 +3025,12 @@ static int bnxt_alloc_mem(struct bnxt *bp, bool irq_re_init)
 		if (!bp->tx_ring)
 			return -ENOMEM;
 
+		bp->tx_ring_map = kcalloc(bp->tx_nr_rings, sizeof(u16),
+					  GFP_KERNEL);
+
+		if (!bp->tx_ring_map)
+			return -ENOMEM;
+
 		if (bp->flags & BNXT_FLAG_SHARED_RINGS)
 			j = 0;
 		else
@@ -3032,6 +3039,8 @@ static int bnxt_alloc_mem(struct bnxt *bp, bool irq_re_init)
 		for (i = 0; i < bp->tx_nr_rings; i++, j++) {
 			bp->tx_ring[i].bnapi = bp->bnapi[j];
 			bp->bnapi[j]->tx_ring = &bp->tx_ring[i];
+			bp->tx_ring_map[i] = i;
+			bp->tx_ring[i].txq_index = i;
 		}
 
 		rc = bnxt_alloc_stats(bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index e1d9056..0a7f045 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -566,6 +566,7 @@ struct bnxt_tx_ring_info {
 	struct bnxt_napi	*bnapi;
 	u16			tx_prod;
 	u16			tx_cons;
+	u16			txq_index;
 	void __iomem		*tx_doorbell;
 
 	struct tx_bd		*tx_desc_ring[MAX_TX_PAGES];
@@ -995,6 +996,7 @@ struct bnxt {
 
 	struct bnxt_rx_ring_info	*rx_ring;
 	struct bnxt_tx_ring_info	*tx_ring;
+	u16			*tx_ring_map;
 
 	struct sk_buff *	(*gro_func)(struct bnxt_tpa_info *, int, int,
 					    struct sk_buff *);
-- 
1.8.3.1

[PATCH net-next v2 09/12] bnxt_en: Add a set of TX rings to support XDP.

[Michael Chan]

Add logic for an extra set of TX rings for XDP.  If enabled, this
set of TX rings equals the number of RX rings and shares the same
IRQ as the RX ring set.  A new field bp->tx_nr_rings_xdp is added
to keep track of these TX XDP rings.  Adjust all other relevant functions
to handle bp->tx_nr_rings_xdp.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 21 +++++++++++++--------
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  3 ++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 16 ++++++++++++----
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 5d402e957..0292bea 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -2215,6 +2215,8 @@ static int bnxt_alloc_tx_rings(struct bnxt *bp)
 			memset(txr->tx_push, 0, sizeof(struct tx_push_bd));
 		}
 		ring->queue_id = bp->q_info[j].queue_id;
+		if (i < bp->tx_nr_rings_xdp)
+			continue;
 		if (i % bp->tx_nr_rings_per_tc == (bp->tx_nr_rings_per_tc - 1))
 			j++;
 	}
@@ -3039,8 +3041,10 @@ static int bnxt_alloc_mem(struct bnxt *bp, bool irq_re_init)
 		for (i = 0; i < bp->tx_nr_rings; i++, j++) {
 			bp->tx_ring[i].bnapi = bp->bnapi[j];
 			bp->bnapi[j]->tx_ring = &bp->tx_ring[i];
-			bp->tx_ring_map[i] = i;
-			bp->tx_ring[i].txq_index = i;
+			bp->tx_ring_map[i] = bp->tx_nr_rings_xdp + i;
+			if (i >= bp->tx_nr_rings_xdp)
+				bp->tx_ring[i].txq_index = i -
+					bp->tx_nr_rings_xdp;
 		}
 
 		rc = bnxt_alloc_stats(bp);
@@ -4963,7 +4967,8 @@ static int bnxt_set_real_num_queues(struct bnxt *bp)
 	int rc;
 	struct net_device *dev = bp->dev;
 
-	rc = netif_set_real_num_tx_queues(dev, bp->tx_nr_rings);
+	rc = netif_set_real_num_tx_queues(dev, bp->tx_nr_rings -
+					  bp->tx_nr_rings_xdp);
 	if (rc)
 		return rc;
 
@@ -6579,7 +6584,7 @@ static void bnxt_sp_task(struct work_struct *work)
 }
 
 /* Under rtnl_lock */
-int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs)
+int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs, int tx_xdp)
 {
 	int max_rx, max_tx, tx_sets = 1;
 	int tx_rings_needed;
@@ -6599,12 +6604,12 @@ int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs)
 	if (max_rx < rx)
 		return -ENOMEM;
 
-	tx_rings_needed = tx * tx_sets;
+	tx_rings_needed = tx * tx_sets + tx_xdp;
 	if (max_tx < tx_rings_needed)
 		return -ENOMEM;
 
 	if (bnxt_hwrm_reserve_tx_rings(bp, &tx_rings_needed) ||
-	    tx_rings_needed < (tx * tx_sets))
+	    tx_rings_needed < (tx * tx_sets + tx_xdp))
 		return -ENOMEM;
 	return 0;
 }
@@ -6785,8 +6790,8 @@ int bnxt_setup_mq_tc(struct net_device *dev, u8 tc)
 	if (bp->flags & BNXT_FLAG_SHARED_RINGS)
 		sh = true;
 
-	rc = bnxt_reserve_rings(bp, bp->tx_nr_rings_per_tc,
-				bp->rx_nr_rings, tc);
+	rc = bnxt_reserve_rings(bp, bp->tx_nr_rings_per_tc, bp->rx_nr_rings,
+				tc, bp->tx_nr_rings_xdp);
 	if (rc)
 		return rc;
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 0a7f045..9bc908f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1026,6 +1026,7 @@ struct bnxt {
 	int			tx_nr_pages;
 	int			tx_nr_rings;
 	int			tx_nr_rings_per_tc;
+	int			tx_nr_rings_xdp;
 
 	int			tx_wake_thresh;
 	int			tx_push_thresh;
@@ -1203,7 +1204,7 @@ int bnxt_hwrm_func_rgtr_async_events(struct bnxt *bp, unsigned long *bmap,
 int bnxt_hwrm_fw_set_time(struct bnxt *);
 int bnxt_open_nic(struct bnxt *, bool, bool);
 int bnxt_close_nic(struct bnxt *, bool, bool);
-int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs);
+int bnxt_reserve_rings(struct bnxt *bp, int tx, int rx, int tcs, int tx_xdp);
 int bnxt_setup_mq_tc(struct net_device *dev, u8 tc);
 int bnxt_get_max_rings(struct bnxt *, int *, int *, bool);
 void bnxt_restore_pf_fw_resources(struct bnxt *bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 6f2568d..7aa248d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -389,6 +389,7 @@ static int bnxt_set_channels(struct net_device *dev,
 	struct bnxt *bp = netdev_priv(dev);
 	int req_tx_rings, req_rx_rings, tcs;
 	bool sh = false;
+	int tx_xdp = 0;
 	int rc = 0;
 
 	if (channel->other_count)
@@ -413,7 +414,14 @@ static int bnxt_set_channels(struct net_device *dev,
 
 	req_tx_rings = sh ? channel->combined_count : channel->tx_count;
 	req_rx_rings = sh ? channel->combined_count : channel->rx_count;
-	rc = bnxt_reserve_rings(bp, req_tx_rings, req_rx_rings, tcs);
+	if (bp->tx_nr_rings_xdp) {
+		if (!sh) {
+			netdev_err(dev, "Only combined mode supported when XDP is enabled.\n");
+			return -EINVAL;
+		}
+		tx_xdp = req_rx_rings;
+	}
+	rc = bnxt_reserve_rings(bp, req_tx_rings, req_rx_rings, tcs, tx_xdp);
 	if (rc) {
 		netdev_warn(dev, "Unable to allocate the requested rings\n");
 		return rc;
@@ -442,10 +450,10 @@ static int bnxt_set_channels(struct net_device *dev,
 		bp->rx_nr_rings = channel->rx_count;
 		bp->tx_nr_rings_per_tc = channel->tx_count;
 	}
-
-	bp->tx_nr_rings = bp->tx_nr_rings_per_tc;
+	bp->tx_nr_rings_xdp = tx_xdp;
+	bp->tx_nr_rings = bp->tx_nr_rings_per_tc + tx_xdp;
 	if (tcs > 1)
-		bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tcs;
+		bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tcs + tx_xdp;
 
 	bp->cp_nr_rings = sh ? max_t(int, bp->tx_nr_rings, bp->rx_nr_rings) :
 			       bp->tx_nr_rings + bp->rx_nr_rings;
-- 
1.8.3.1

[PATCH net-next v2 10/12] bnxt_en: Refactor tx completion path.

[Michael Chan]

XDP_TX requires a different function to handle completion.  Add a
function pointer to handle tx completion logic.  Regular TX rings
will be assigned the current bnxt_tx_int() for the ->tx_int()
function pointer.

Also, add a struct page pointer as a union with the current skb pointer
to the struct bnxt_sw_tx_bd.  XDP TX rings will use the struct page
pointer.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c |  5 ++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h | 10 +++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 0292bea..ff651a0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1762,7 +1762,7 @@ static int bnxt_poll_work(struct bnxt *bp, struct bnxt_napi *bnapi, int budget)
 	BNXT_CP_DB(cpr->cp_doorbell, cpr->cp_raw_cons);
 
 	if (tx_pkts)
-		bnxt_tx_int(bp, bnapi, tx_pkts);
+		bnapi->tx_int(bp, bnapi, tx_pkts);
 
 	if (event & BNXT_RX_EVENT) {
 		struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
@@ -3045,6 +3045,9 @@ static int bnxt_alloc_mem(struct bnxt *bp, bool irq_re_init)
 			if (i >= bp->tx_nr_rings_xdp)
 				bp->tx_ring[i].txq_index = i -
 					bp->tx_nr_rings_xdp;
+			else
+				bp->bnapi[j]->flags |= BNXT_NAPI_FLAG_XDP;
+			bp->bnapi[j]->tx_int = bnxt_tx_int;
 		}
 
 		rc = bnxt_alloc_stats(bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 9bc908f..6bf9444 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -515,7 +515,10 @@ struct rx_tpa_end_cmp_ext {
 #define BNXT_AGG_EVENT	2
 
 struct bnxt_sw_tx_bd {
-	struct sk_buff		*skb;
+	union {
+		struct sk_buff		*skb;
+		struct page		*page;
+	};
 	DEFINE_DMA_UNMAP_ADDR(mapping);
 	u8			is_gso;
 	u8			is_push;
@@ -664,6 +667,11 @@ struct bnxt_napi {
 	struct bnxt_rx_ring_info	*rx_ring;
 	struct bnxt_tx_ring_info	*tx_ring;
 
+	void			(*tx_int)(struct bnxt *, struct bnxt_napi *,
+					  int);
+	u32			flags;
+#define BNXT_NAPI_FLAG_XDP	0x1
+
 	bool			in_reset;
 };
 
-- 
1.8.3.1

[PATCH net-next v2 11/12] bnxt_en: Add basic XDP support.

[Michael Chan]

Add basic ndo_xdp support to setup and query program, configure the NIC
to run in rx page mode, and support XDP_PASS, XDP_DROP, XDP_ABORTED
actions only.  Use Kconfig option to enable XDP support.

v2: Added trace_xdp_exception()
    Added dma_syncs.
    Added XDP headroom support.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Tested-by: Andy Gospodarek <gospo@broadcom.com>
---
 drivers/net/ethernet/broadcom/Kconfig         |   8 ++
 drivers/net/ethernet/broadcom/bnxt/Makefile   |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  33 +++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |   9 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 157 ++++++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h |  27 +++++
 6 files changed, 229 insertions(+), 7 deletions(-)
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h

diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index 940fb24..ca543a4 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -213,4 +213,12 @@ config BNXT_DCB
 
 	  If unsure, say N.
 
+config BNXT_XDP
+	bool "Xpress Data Path (XDP) driver support"
+	default n
+	depends on BNXT && BPF
+	---help---
+	  Say Y here if you want to enable XDP in the driver to support
+	  eBPF programs in the fast path.
+
 endif # NET_VENDOR_BROADCOM
diff --git a/drivers/net/ethernet/broadcom/bnxt/Makefile b/drivers/net/ethernet/broadcom/bnxt/Makefile
index 6082ed1..a7ca45b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/Makefile
+++ b/drivers/net/ethernet/broadcom/bnxt/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_BNXT) += bnxt_en.o
 
-bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o
+bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o bnxt_dcb.o bnxt_ulp.o bnxt_xdp.o
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index ff651a0..4613702 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -33,6 +33,7 @@
 #include <linux/if.h>
 #include <linux/if_vlan.h>
 #include <linux/rtc.h>
+#include <linux/bpf.h>
 #include <net/ip.h>
 #include <net/tcp.h>
 #include <net/udp.h>
@@ -53,6 +54,7 @@
 #include "bnxt_sriov.h"
 #include "bnxt_ethtool.h"
 #include "bnxt_dcb.h"
+#include "bnxt_xdp.h"
 
 #define BNXT_TX_TIMEOUT		(5 * HZ)
 
@@ -642,8 +644,7 @@ static inline int bnxt_alloc_rx_data(struct bnxt *bp,
 	return 0;
 }
 
-static void bnxt_reuse_rx_data(struct bnxt_rx_ring_info *rxr, u16 cons,
-			       void *data)
+void bnxt_reuse_rx_data(struct bnxt_rx_ring_info *rxr, u16 cons, void *data)
 {
 	u16 prod = rxr->rx_prod;
 	struct bnxt_sw_rx_bd *cons_rx_buf, *prod_rx_buf;
@@ -1475,6 +1476,11 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
 	len = le32_to_cpu(rxcmp->rx_cmp_len_flags_type) >> RX_CMP_LEN_SHIFT;
 	dma_addr = rx_buf->mapping;
 
+	if (bnxt_rx_xdp(bp, rxr, cons, data, len, event)) {
+		rc = 1;
+		goto next_rx;
+	}
+
 	if (len <= bp->rx_copy_thresh) {
 		skb = bnxt_copy_skb(bnapi, data_ptr, len, dma_addr);
 		bnxt_reuse_rx_data(rxr, cons, data);
@@ -2077,6 +2083,9 @@ static void bnxt_free_rx_rings(struct bnxt *bp)
 		struct bnxt_rx_ring_info *rxr = &bp->rx_ring[i];
 		struct bnxt_ring_struct *ring;
 
+		if (rxr->xdp_prog)
+			bpf_prog_put(rxr->xdp_prog);
+
 		kfree(rxr->rx_tpa);
 		rxr->rx_tpa = NULL;
 
@@ -2364,6 +2373,15 @@ static int bnxt_init_one_rx_ring(struct bnxt *bp, int ring_nr)
 	ring = &rxr->rx_ring_struct;
 	bnxt_init_rxbd_pages(ring, type);
 
+	if (BNXT_RX_PAGE_MODE(bp) && bp->xdp_prog) {
+		rxr->xdp_prog = bpf_prog_add(bp->xdp_prog, 1);
+		if (IS_ERR(rxr->xdp_prog)) {
+			int rc = PTR_ERR(rxr->xdp_prog);
+
+			rxr->xdp_prog = NULL;
+			return rc;
+		}
+	}
 	prod = rxr->rx_prod;
 	for (i = 0; i < bp->rx_ring_size; i++) {
 		if (bnxt_alloc_rx_data(bp, rxr, prod, GFP_KERNEL) != 0) {
@@ -2427,8 +2445,8 @@ static int bnxt_init_rx_rings(struct bnxt *bp)
 	int i, rc = 0;
 
 	if (BNXT_RX_PAGE_MODE(bp)) {
-		bp->rx_offset = NET_IP_ALIGN;
-		bp->rx_dma_offset = 0;
+		bp->rx_offset = NET_IP_ALIGN + XDP_PACKET_HEADROOM;
+		bp->rx_dma_offset = XDP_PACKET_HEADROOM;
 	} else {
 		bp->rx_offset = BNXT_RX_OFFSET;
 		bp->rx_dma_offset = BNXT_RX_DMA_OFFSET;
@@ -2557,7 +2575,7 @@ static int bnxt_calc_nr_ring_pages(u32 ring_size, int desc_per_pg)
 	return pages;
 }
 
-static void bnxt_set_tpa_flags(struct bnxt *bp)
+void bnxt_set_tpa_flags(struct bnxt *bp)
 {
 	bp->flags &= ~BNXT_FLAG_TPA;
 	if (bp->flags & BNXT_FLAG_NO_AGG_RINGS)
@@ -7104,6 +7122,9 @@ static void bnxt_udp_tunnel_del(struct net_device *dev,
 #endif
 	.ndo_udp_tunnel_add	= bnxt_udp_tunnel_add,
 	.ndo_udp_tunnel_del	= bnxt_udp_tunnel_del,
+#ifdef CONFIG_BNXT_XDP
+	.ndo_xdp		= bnxt_xdp,
+#endif
 };
 
 static void bnxt_remove_one(struct pci_dev *pdev)
@@ -7128,6 +7149,8 @@ static void bnxt_remove_one(struct pci_dev *pdev)
 	pci_iounmap(pdev, bp->bar0);
 	kfree(bp->edev);
 	bp->edev = NULL;
+	if (bp->xdp_prog)
+		bpf_prog_put(bp->xdp_prog);
 	free_netdev(dev);
 
 	pci_release_regions(pdev);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 6bf9444..db9d5d9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -418,7 +418,8 @@ struct rx_tpa_end_cmp_ext {
 
 #define BNXT_MAX_MTU		9500
 #define BNXT_MAX_PAGE_MODE_MTU	\
-	((unsigned int)PAGE_SIZE - VLAN_ETH_HLEN - NET_IP_ALIGN)
+	((unsigned int)PAGE_SIZE - VLAN_ETH_HLEN - NET_IP_ALIGN -	\
+	 XDP_PACKET_HEADROOM)
 
 #define BNXT_MIN_PKT_SIZE	52
 
@@ -621,6 +622,8 @@ struct bnxt_rx_ring_info {
 	void __iomem		*rx_doorbell;
 	void __iomem		*rx_agg_doorbell;
 
+	struct bpf_prog		*xdp_prog;
+
 	struct rx_bd		*rx_desc_ring[MAX_RX_PAGES];
 	struct bnxt_sw_rx_bd	*rx_buf_ring;
 
@@ -1170,6 +1173,8 @@ struct bnxt {
 
 	u8			num_leds;
 	struct bnxt_led_info	leds[BNXT_MAX_LED];
+
+	struct bpf_prog		*xdp_prog;
 };
 
 #define BNXT_RX_STATS_OFFSET(counter)			\
@@ -1189,6 +1194,8 @@ struct bnxt {
 #define SFF_MODULE_ID_QSFP28			0x11
 #define BNXT_MAX_PHY_I2C_RESP_SIZE		64
 
+void bnxt_reuse_rx_data(struct bnxt_rx_ring_info *rxr, u16 cons, void *data);
+void bnxt_set_tpa_flags(struct bnxt *bp);
 void bnxt_set_ring_params(struct bnxt *);
 int bnxt_set_rx_skb_mode(struct bnxt *bp, bool page_mode);
 void bnxt_hwrm_cmd_hdr_init(struct bnxt *, void *, u16, u16, u16);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
new file mode 100644
index 0000000..50315d7
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
@@ -0,0 +1,157 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2016-2017 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/pci.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
+#include <linux/bpf.h>
+#include <linux/bpf_trace.h>
+#include <linux/filter.h>
+#include "bnxt_hsi.h"
+#include "bnxt.h"
+#include "bnxt_xdp.h"
+
+#ifdef CONFIG_BNXT_XDP
+/* returns the following:
+ * true    - packet consumed by XDP and new buffer is allocated.
+ * false   - packet should be passed to the stack.
+ */
+bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons,
+		 struct page *page, unsigned int len, u8 *event)
+{
+	struct bpf_prog *xdp_prog = READ_ONCE(rxr->xdp_prog);
+	struct bnxt_sw_rx_bd *cons_rx_buf;
+	struct pci_dev *pdev = bp->pdev;
+	u32 offset = bp->rx_offset;
+	struct xdp_buff xdp;
+	dma_addr_t mapping;
+	void *orig_data;
+	u32 act;
+
+	if (!xdp_prog)
+		return false;
+
+	cons_rx_buf = &rxr->rx_buf_ring[cons];
+
+	xdp.data_hard_start = cons_rx_buf->data_ptr - bp->rx_offset;
+	xdp.data = cons_rx_buf->data_ptr;
+	xdp.data_end = xdp.data + len;
+	orig_data = xdp.data;
+	mapping = cons_rx_buf->mapping - bp->rx_dma_offset;
+
+	dma_sync_single_for_cpu(&pdev->dev, mapping + offset, len, bp->rx_dir);
+
+	rcu_read_lock();
+	act = bpf_prog_run_xdp(xdp_prog, &xdp);
+	rcu_read_unlock();
+
+	if (orig_data != xdp.data) {
+		offset = xdp.data - xdp.data_hard_start;
+		len = xdp.data_end - xdp.data;
+	}
+	switch (act) {
+	case XDP_PASS:
+		return false;
+
+	default:
+		bpf_warn_invalid_xdp_action(act);
+		/* Fall thru */
+	case XDP_ABORTED:
+		trace_xdp_exception(bp->dev, xdp_prog, act);
+		/* Fall thru */
+	case XDP_DROP:
+		bnxt_reuse_rx_data(rxr, cons, page);
+		break;
+	}
+	return true;
+}
+
+/* Under rtnl_lock */
+static int bnxt_xdp_set(struct bnxt *bp, struct bpf_prog *prog)
+{
+	struct net_device *dev = bp->dev;
+	int tx_xdp = 0, rc, tc;
+	struct bpf_prog *old;
+
+	if (prog && bp->dev->mtu > BNXT_MAX_PAGE_MODE_MTU) {
+		netdev_err(dev, "MTU %d larger than largest XDP supported MTU %d.\n",
+			   bp->dev->mtu, BNXT_MAX_PAGE_MODE_MTU);
+		return -EOPNOTSUPP;
+	}
+	if (!(bp->flags & BNXT_FLAG_SHARED_RINGS)) {
+		netdev_err(dev, "ethtool rx/tx channels must be combined to support XDP.\n");
+		return -EOPNOTSUPP;
+	}
+	if (prog)
+		tx_xdp = bp->rx_nr_rings;
+
+	tc = netdev_get_num_tc(dev);
+	if (!tc)
+		tc = 1;
+	rc = bnxt_reserve_rings(bp, bp->tx_nr_rings_per_tc, bp->rx_nr_rings,
+				tc, tx_xdp);
+	if (rc) {
+		netdev_err(dev, "Unable to reserve enough TX rings to support XDP.\n");
+		return rc;
+	}
+	if (netif_running(dev))
+		bnxt_close_nic(bp, true, false);
+
+	old = xchg(&bp->xdp_prog, prog);
+	if (old)
+		bpf_prog_put(old);
+
+	if (prog) {
+		bnxt_set_rx_skb_mode(bp, true);
+	} else {
+		bool sh = (bp->flags & BNXT_FLAG_SHARED_RINGS) ? true : false;
+		int rx, tx;
+
+		bnxt_set_rx_skb_mode(bp, false);
+		bnxt_get_max_rings(bp, &rx, &tx, sh);
+		if (rx > 1) {
+			bp->flags &= ~BNXT_FLAG_NO_AGG_RINGS;
+			bp->dev->hw_features |= NETIF_F_LRO;
+		}
+	}
+	bp->tx_nr_rings_xdp = tx_xdp;
+	bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tc + tx_xdp;
+	bp->cp_nr_rings = max_t(int, bp->tx_nr_rings, bp->rx_nr_rings);
+	bp->num_stat_ctxs = bp->cp_nr_rings;
+	bnxt_set_tpa_flags(bp);
+	bnxt_set_ring_params(bp);
+
+	if (netif_running(dev))
+		return bnxt_open_nic(bp, true, false);
+
+	return 0;
+}
+
+int bnxt_xdp(struct net_device *dev, struct netdev_xdp *xdp)
+{
+	struct bnxt *bp = netdev_priv(dev);
+	int rc;
+
+	switch (xdp->command) {
+	case XDP_SETUP_PROG:
+		rc = bnxt_xdp_set(bp, xdp->prog);
+		break;
+	case XDP_QUERY_PROG:
+		xdp->prog_attached = !!bp->xdp_prog;
+		rc = 0;
+		break;
+	default:
+		rc = -EINVAL;
+		break;
+	}
+	return rc;
+}
+#endif
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
new file mode 100644
index 0000000..85c01cc
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
@@ -0,0 +1,27 @@
+/* Broadcom NetXtreme-C/E network driver.
+ *
+ * Copyright (c) 2016-2017 Broadcom Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef BNXT_XDP_H
+#define BNXT_XDP_H
+
+#ifdef CONFIG_BNXT_XDP
+bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons,
+		 struct page *page, unsigned int len, u8 *event);
+
+int bnxt_xdp(struct net_device *dev, struct netdev_xdp *xdp);
+
+#else
+static inline bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
+			       u16 cons, struct page *page, unsigned int len,
+			       u8 *event)
+{
+	return false;
+}
+#endif
+#endif
-- 
1.8.3.1

[PATCH net-next v2 12/12] bnxt_en: Add support for XDP_TX action.

[Michael Chan]

Add dedicated transmit function and transmit completion handler for
XDP which are a lot simpler than the original functions for SKB.

v2: Add trace_xdp_exception().
    Add dma_sync.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Tested-by: Andy Gospodarek <gospo@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 35 ++++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     | 14 +++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 90 +++++++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h |  2 +
 4 files changed, 125 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4613702..6376c1f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -212,16 +212,7 @@ static bool bnxt_vf_pciid(enum board_idx idx)
 #define BNXT_CP_DB_IRQ_DIS(db)						\
 		writel(DB_CP_IRQ_DIS_FLAGS, db)
 
-static inline u32 bnxt_tx_avail(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
-{
-	/* Tell compiler to fetch tx indices from memory. */
-	barrier();
-
-	return bp->tx_ring_size -
-		((txr->tx_prod - txr->tx_cons) & bp->tx_ring_mask);
-}
-
-static const u16 bnxt_lhint_arr[] = {
+const u16 bnxt_lhint_arr[] = {
 	TX_BD_FLAGS_LHINT_512_AND_SMALLER,
 	TX_BD_FLAGS_LHINT_512_TO_1023,
 	TX_BD_FLAGS_LHINT_1024_TO_2047,
@@ -613,9 +604,8 @@ static inline u8 *__bnxt_alloc_rx_data(struct bnxt *bp, dma_addr_t *mapping,
 	return data;
 }
 
-static inline int bnxt_alloc_rx_data(struct bnxt *bp,
-				     struct bnxt_rx_ring_info *rxr,
-				     u16 prod, gfp_t gfp)
+int bnxt_alloc_rx_data(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
+		       u16 prod, gfp_t gfp)
 {
 	struct rx_bd *rxbd = &rxr->rx_desc_ring[RX_RING(prod)][RX_IDX(prod)];
 	struct bnxt_sw_rx_bd *rx_buf = &rxr->rx_buf_ring[prod];
@@ -1770,6 +1760,17 @@ static int bnxt_poll_work(struct bnxt *bp, struct bnxt_napi *bnapi, int budget)
 	if (tx_pkts)
 		bnapi->tx_int(bp, bnapi, tx_pkts);
 
+	if (event & BNXT_TX_EVENT) {
+		struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
+		void __iomem *db = txr->tx_doorbell;
+		u16 prod = txr->tx_prod;
+
+		/* Sync BD data before updating doorbell */
+		wmb();
+
+		writel(DB_KEY_TX | prod, db);
+		writel(DB_KEY_TX | prod, db);
+	}
 	if (event & BNXT_RX_EVENT) {
 		struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
 
@@ -3060,12 +3061,14 @@ static int bnxt_alloc_mem(struct bnxt *bp, bool irq_re_init)
 			bp->tx_ring[i].bnapi = bp->bnapi[j];
 			bp->bnapi[j]->tx_ring = &bp->tx_ring[i];
 			bp->tx_ring_map[i] = bp->tx_nr_rings_xdp + i;
-			if (i >= bp->tx_nr_rings_xdp)
+			if (i >= bp->tx_nr_rings_xdp) {
 				bp->tx_ring[i].txq_index = i -
 					bp->tx_nr_rings_xdp;
-			else
+				bp->bnapi[j]->tx_int = bnxt_tx_int;
+			} else {
 				bp->bnapi[j]->flags |= BNXT_NAPI_FLAG_XDP;
-			bp->bnapi[j]->tx_int = bnxt_tx_int;
+				bp->bnapi[j]->tx_int = bnxt_tx_int_xdp;
+			}
 		}
 
 		rc = bnxt_alloc_stats(bp);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index db9d5d9..8ce4c59 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -514,6 +514,7 @@ struct rx_tpa_end_cmp_ext {
 
 #define BNXT_RX_EVENT	1
 #define BNXT_AGG_EVENT	2
+#define BNXT_TX_EVENT	4
 
 struct bnxt_sw_tx_bd {
 	union {
@@ -1194,6 +1195,19 @@ struct bnxt {
 #define SFF_MODULE_ID_QSFP28			0x11
 #define BNXT_MAX_PHY_I2C_RESP_SIZE		64
 
+static inline u32 bnxt_tx_avail(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
+{
+	/* Tell compiler to fetch tx indices from memory. */
+	barrier();
+
+	return bp->tx_ring_size -
+		((txr->tx_prod - txr->tx_cons) & bp->tx_ring_mask);
+}
+
+extern const u16 bnxt_lhint_arr[];
+
+int bnxt_alloc_rx_data(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
+		       u16 prod, gfp_t gfp);
 void bnxt_reuse_rx_data(struct bnxt_rx_ring_info *rxr, u16 cons, void *data);
 void bnxt_set_tpa_flags(struct bnxt *bp);
 void bnxt_set_ring_params(struct bnxt *);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
index 50315d7..0a9a050 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
@@ -20,6 +20,68 @@
 #include "bnxt_xdp.h"
 
 #ifdef CONFIG_BNXT_XDP
+static int bnxt_xmit_xdp(struct bnxt *bp, struct bnxt_tx_ring_info *txr,
+			 struct page *page, dma_addr_t mapping, u32 offset,
+			 u32 len)
+{
+	struct bnxt_sw_tx_bd *tx_buf;
+	struct tx_bd_ext *txbd1;
+	struct tx_bd *txbd;
+	u32 flags;
+	u16 prod;
+
+	if (bnxt_tx_avail(bp, txr) < 2)
+		return -ENOSPC;
+
+	prod = txr->tx_prod;
+	txbd = &txr->tx_desc_ring[TX_RING(prod)][TX_IDX(prod)];
+
+	tx_buf = &txr->tx_buf_ring[prod];
+	tx_buf->page = page;
+	dma_unmap_addr_set(tx_buf, mapping, mapping);
+	flags = (len << TX_BD_LEN_SHIFT) | TX_BD_TYPE_LONG_TX_BD |
+		(2 << TX_BD_FLAGS_BD_CNT_SHIFT) | TX_BD_FLAGS_PACKET_END |
+		bnxt_lhint_arr[len >> 9];
+	txbd->tx_bd_len_flags_type = cpu_to_le32(flags);
+	txbd->tx_bd_opaque = prod;
+	txbd->tx_bd_haddr = cpu_to_le64(mapping + offset);
+
+	prod = NEXT_TX(prod);
+	txbd1 = (struct tx_bd_ext *)
+		&txr->tx_desc_ring[TX_RING(prod)][TX_IDX(prod)];
+
+	txbd1->tx_bd_hsize_lflags = cpu_to_le32(0);
+	txbd1->tx_bd_mss = cpu_to_le32(0);
+	txbd1->tx_bd_cfa_action = cpu_to_le32(0);
+	txbd1->tx_bd_cfa_meta = cpu_to_le32(0);
+
+	prod = NEXT_TX(prod);
+	txr->tx_prod = prod;
+	return 0;
+}
+
+void bnxt_tx_int_xdp(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
+{
+	struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
+	struct device *dev = &bp->pdev->dev;
+	u16 cons = txr->tx_cons;
+	int i;
+
+	for (i = 0; i < nr_pkts; i++) {
+		struct bnxt_sw_tx_bd *tx_buf;
+
+		tx_buf = &txr->tx_buf_ring[cons];
+		cons = NEXT_TX(cons);
+		cons = NEXT_TX(cons);
+
+		dma_unmap_page(dev, dma_unmap_addr(tx_buf, mapping), PAGE_SIZE,
+			       bp->rx_dir);
+		__free_page(tx_buf->page);
+		tx_buf->page = NULL;
+	}
+	txr->tx_cons = cons;
+}
+
 /* returns the following:
  * true    - packet consumed by XDP and new buffer is allocated.
  * false   - packet should be passed to the stack.
@@ -61,6 +123,28 @@ bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons,
 	case XDP_PASS:
 		return false;
 
+	case XDP_TX: {
+		struct bnxt_tx_ring_info *txr = rxr->bnapi->tx_ring;
+		int rc;
+
+		rc = bnxt_alloc_rx_data(bp, rxr, rxr->rx_prod, GFP_ATOMIC);
+		if (unlikely(rc)) {
+			trace_xdp_exception(bp->dev, xdp_prog, act);
+			bnxt_reuse_rx_data(rxr, cons, page);
+			return true;
+		}
+		dma_sync_single_for_device(&pdev->dev, mapping + offset, len,
+					   bp->rx_dir);
+		if (bnxt_xmit_xdp(bp, txr, page, mapping, offset, len)) {
+			trace_xdp_exception(bp->dev, xdp_prog, act);
+			dma_unmap_page(&bp->pdev->dev, mapping, PAGE_SIZE,
+				       bp->rx_dir);
+			__free_page(page);
+			return true;
+		}
+		*event |= BNXT_TX_EVENT;
+		return true;
+	}
 	default:
 		bpf_warn_invalid_xdp_action(act);
 		/* Fall thru */
@@ -154,4 +238,10 @@ int bnxt_xdp(struct net_device *dev, struct netdev_xdp *xdp)
 	}
 	return rc;
 }
+
+#else
+
+void bnxt_tx_int_xdp(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
+{
+}
 #endif
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
index 85c01cc..4589603 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
@@ -10,6 +10,8 @@
 #ifndef BNXT_XDP_H
 #define BNXT_XDP_H
 
+void bnxt_tx_int_xdp(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts);
+
 #ifdef CONFIG_BNXT_XDP
 bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons,
 		 struct page *page, unsigned int len, u8 *event);
-- 
1.8.3.1

RE: [PATCH net-next v2 11/12] bnxt_en: Add basic XDP support.

[Mintz, Yuval]

> +config BNXT_XDP
> +	bool "Xpress Data Path (XDP) driver support"
> +	default n
> +	depends on BNXT && BPF
> +	---help---
> +	  Say Y here if you want to enable XDP in the driver to support
> +	  eBPF programs in the fast path.
> +

Wasn't it recently discussed that per-feature option is preferable
to a per-feature per-device option?
Assuming BPF > XDP and thus shouldn't directly imply that XDP
should be supported, perhaps the right thing is to add a global
XDP config option?

> +	if (prog && bp->dev->mtu > BNXT_MAX_PAGE_MODE_MTU) {
> +		netdev_err(dev, "MTU %d larger than largest XDP supported
> MTU %d.\n",
> +			   bp->dev->mtu, BNXT_MAX_PAGE_MODE_MTU);
> +		return -EOPNOTSUPP;
> +	}

Is it O.k. to print with netdev_err() for a user-provided unsupported
configuration? Shouldn't that be limited?

> +		bool sh = (bp->flags & BNXT_FLAG_SHARED_RINGS) ? true :
> false;

Didn't you already check this flag is set?

Re: [PATCH net-next v2 11/12] bnxt_en: Add basic XDP support.

[Michael Chan]

On Thu, Feb 2, 2017 at 9:18 AM, Mintz, Yuval <Yuval.Mintz@cavium.com> wrote:
>> +config BNXT_XDP
>> +     bool "Xpress Data Path (XDP) driver support"
>> +     default n
>> +     depends on BNXT && BPF
>> +     ---help---
>> +       Say Y here if you want to enable XDP in the driver to support
>> +       eBPF programs in the fast path.
>> +
>
> Wasn't it recently discussed that per-feature option is preferable
> to a per-feature per-device option?
> Assuming BPF > XDP and thus shouldn't directly imply that XDP
> should be supported, perhaps the right thing is to add a global
> XDP config option?

Since the driver has other per-device options, I'm adding another
per-device option right now.  It can easily be converted to a more
tree-wide option if that's what we want to do.

>
>> +     if (prog && bp->dev->mtu > BNXT_MAX_PAGE_MODE_MTU) {
>> +             netdev_err(dev, "MTU %d larger than largest XDP supported
>> MTU %d.\n",
>> +                        bp->dev->mtu, BNXT_MAX_PAGE_MODE_MTU);
>> +             return -EOPNOTSUPP;
>> +     }
>
> Is it O.k. to print with netdev_err() for a user-provided unsupported
> configuration? Shouldn't that be limited?

You are suggesting to use lower level warning, such as netdev_warn(), right?

>
>> +             bool sh = (bp->flags & BNXT_FLAG_SHARED_RINGS) ? true :
>> false;
>
> Didn't you already check this flag is set?
>

You are right.  I checked already.

Re: [PATCH net-next v2 01/12] bnxt_en: Refactor rx SKB function.

[Jakub Kicinski]

On Thu,  2 Feb 2017 11:55:29 -0500, Michael Chan wrote:
> @@ -755,8 +757,8 @@ static void bnxt_reuse_rx_agg_bufs(struct bnxt_napi *bnapi, u16 cp_cons,
>  
>  static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
>  				   struct bnxt_rx_ring_info *rxr, u16 cons,
> -				   u16 prod, u8 *data, dma_addr_t dma_addr,
> -				   unsigned int len)
> +				   u16 prod, void *data, dma_addr_t dma_addr,
> +				   unsigned int offset_and_len)
>  {
>  	int err;
>  	struct sk_buff *skb;
> @@ -776,7 +778,7 @@ static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
>  	}
>  
>  	skb_reserve(skb, BNXT_RX_OFFSET);
> -	skb_put(skb, len);
> +	skb_put(skb, offset_and_len & 0xffff);
>  	return skb;
>  }
>  

Sorry to be a pain but I still don't understand (a) why you make this
change in the first patch if it's only needed from patch 5 on; (b) why
do you encode the two parameters in a single u32?  It's the seventh
parameter so it's going on the stack anyway, no?

Re: [PATCH net-next v2 11/12] bnxt_en: Add basic XDP support.

[Jakub Kicinski]

On Thu,  2 Feb 2017 11:55:39 -0500, Michael Chan wrote:
> +/* returns the following:
> + * true    - packet consumed by XDP and new buffer is allocated.
> + * false   - packet should be passed to the stack.
> + */
> +bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons,
> +		 struct page *page, unsigned int len, u8 *event)
> +{
> +	struct bpf_prog *xdp_prog = READ_ONCE(rxr->xdp_prog);
> +	struct bnxt_sw_rx_bd *cons_rx_buf;
> +	struct pci_dev *pdev = bp->pdev;
> +	u32 offset = bp->rx_offset;
> +	struct xdp_buff xdp;
> +	dma_addr_t mapping;
> +	void *orig_data;
> +	u32 act;
> +
> +	if (!xdp_prog)
> +		return false;
> +
> +	cons_rx_buf = &rxr->rx_buf_ring[cons];
> +
> +	xdp.data_hard_start = cons_rx_buf->data_ptr - bp->rx_offset;
> +	xdp.data = cons_rx_buf->data_ptr;
> +	xdp.data_end = xdp.data + len;
> +	orig_data = xdp.data;
> +	mapping = cons_rx_buf->mapping - bp->rx_dma_offset;
> +
> +	dma_sync_single_for_cpu(&pdev->dev, mapping + offset, len, bp->rx_dir);
> +
> +	rcu_read_lock();
> +	act = bpf_prog_run_xdp(xdp_prog, &xdp);
> +	rcu_read_unlock();
> +
> +	if (orig_data != xdp.data) {
> +		offset = xdp.data - xdp.data_hard_start;
> +		len = xdp.data_end - xdp.data;

If BPF changed the start of the packet and returned XDP_PASS you should
make sure stack will see the modified packet.  I.e. with adjusted
offset and len.

(CC: Alexei, Martin in case I'm wrong)

> +	}
> +	switch (act) {
> +	case XDP_PASS:
> +		return false;
> +
> +	default:
> +		bpf_warn_invalid_xdp_action(act);
> +		/* Fall thru */
> +	case XDP_ABORTED:
> +		trace_xdp_exception(bp->dev, xdp_prog, act);
> +		/* Fall thru */
> +	case XDP_DROP:
> +		bnxt_reuse_rx_data(rxr, cons, page);
> +		break;
> +	}
> +	return true;
> +}

Re: [PATCH net-next v2 01/12] bnxt_en: Refactor rx SKB function.

[Michael Chan]

On Thu, Feb 2, 2017 at 2:56 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Thu,  2 Feb 2017 11:55:29 -0500, Michael Chan wrote:
>> @@ -755,8 +757,8 @@ static void bnxt_reuse_rx_agg_bufs(struct bnxt_napi *bnapi, u16 cp_cons,
>>
>>  static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
>>                                  struct bnxt_rx_ring_info *rxr, u16 cons,
>> -                                u16 prod, u8 *data, dma_addr_t dma_addr,
>> -                                unsigned int len)
>> +                                u16 prod, void *data, dma_addr_t dma_addr,
>> +                                unsigned int offset_and_len)
>>  {
>>       int err;
>>       struct sk_buff *skb;
>> @@ -776,7 +778,7 @@ static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
>>       }
>>
>>       skb_reserve(skb, BNXT_RX_OFFSET);
>> -     skb_put(skb, len);
>> +     skb_put(skb, offset_and_len & 0xffff);
>>       return skb;
>>  }
>>
>
> Sorry to be a pain but I still don't understand (a) why you make this
> change in the first patch if it's only needed from patch 5 on;

Because I'm thinking ahead.  The function is now a function pointer
and I want to make the parameter change now.

> (b) why
> do you encode the two parameters in a single u32?  It's the seventh
> parameter so it's going on the stack anyway, no?

Both the length and the offset come from the hardware's rx completion
record.  Both are u16.  The offset happens to be in the upper 16-bit
in the hardware record.  So it is convenient to encode it like this
and I chose to do it like this. Of course, using a separate parameter
will also work.

Re: [PATCH net-next v2 11/12] bnxt_en: Add basic XDP support.

[Alexei Starovoitov]

On Thu, Feb 02, 2017 at 03:08:27PM -0800, Jakub Kicinski wrote:
> > +	rcu_read_lock();
> > +	act = bpf_prog_run_xdp(xdp_prog, &xdp);
> > +	rcu_read_unlock();
> > +
> > +	if (orig_data != xdp.data) {
> > +		offset = xdp.data - xdp.data_hard_start;
> > +		len = xdp.data_end - xdp.data;
> 
> If BPF changed the start of the packet and returned XDP_PASS you should
> make sure stack will see the modified packet.  I.e. with adjusted
> offset and len.

that is correct.
skb passed to the stack should see new packet start and len.
Though today it's only useful with tc+cls_bpf on top that can
take advantage of it. Certainly more work is needed in this area
for xdp overall. I don't think we should mix that discussion
with this patch set.

Re: [PATCH net-next v2 01/12] bnxt_en: Refactor rx SKB function.

[Jakub Kicinski]

On Thu, 2 Feb 2017 15:40:19 -0800, Michael Chan wrote:
> On Thu, Feb 2, 2017 at 2:56 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> > On Thu,  2 Feb 2017 11:55:29 -0500, Michael Chan wrote:  
> >> @@ -755,8 +757,8 @@ static void bnxt_reuse_rx_agg_bufs(struct bnxt_napi *bnapi, u16 cp_cons,
> >>
> >>  static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
> >>                                  struct bnxt_rx_ring_info *rxr, u16 cons,
> >> -                                u16 prod, u8 *data, dma_addr_t dma_addr,
> >> -                                unsigned int len)
> >> +                                u16 prod, void *data, dma_addr_t dma_addr,
> >> +                                unsigned int offset_and_len)
> >>  {
> >>       int err;
> >>       struct sk_buff *skb;
> >> @@ -776,7 +778,7 @@ static struct sk_buff *bnxt_rx_skb(struct bnxt *bp,
> >>       }
> >>
> >>       skb_reserve(skb, BNXT_RX_OFFSET);
> >> -     skb_put(skb, len);
> >> +     skb_put(skb, offset_and_len & 0xffff);
> >>       return skb;
> >>  }
> >>  
> >
> > Sorry to be a pain but I still don't understand (a) why you make this
> > change in the first patch if it's only needed from patch 5 on;  
> 
> Because I'm thinking ahead.  The function is now a function pointer
> and I want to make the parameter change now.

You're thinking ahead, reviewers have to guess ahead :)

> > (b) why
> > do you encode the two parameters in a single u32?  It's the seventh
> > parameter so it's going on the stack anyway, no?  
> 
> Both the length and the offset come from the hardware's rx completion
> record.  Both are u16.  The offset happens to be in the upper 16-bit
> in the hardware record.  So it is convenient to encode it like this
> and I chose to do it like this. Of course, using a separate parameter
> will also work.

Yes, I initially thought you read them out this way straight from the
descriptor but you actually combine them into this form:

+		unsigned int payload_len;
+
+		payload_len = (le32_to_cpu(rxcmp->rx_cmp_misc_v1) &
+			       RX_CMP_PAYLOAD_OFFSET) | len;
+		skb = bp->rx_skb_func(bp, rxr, cons, prod, data, dma_addr,

I don't mind though, I was just hoping this is some clever optimization
technique I could learn :)

Re: [PATCH net-next v2 01/12] bnxt_en: Refactor rx SKB function.

[Michael Chan]

On Thu, Feb 2, 2017 at 4:22 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Thu, 2 Feb 2017 15:40:19 -0800, Michael Chan wrote:
>> On Thu, Feb 2, 2017 at 2:56 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
>> > (b) why
>> > do you encode the two parameters in a single u32?  It's the seventh
>> > parameter so it's going on the stack anyway, no?
>>
>> Both the length and the offset come from the hardware's rx completion
>> record.  Both are u16.  The offset happens to be in the upper 16-bit
>> in the hardware record.  So it is convenient to encode it like this
>> and I chose to do it like this. Of course, using a separate parameter
>> will also work.
>
> Yes, I initially thought you read them out this way straight from the
> descriptor but you actually combine them into this form:
>
> +               unsigned int payload_len;
> +
> +               payload_len = (le32_to_cpu(rxcmp->rx_cmp_misc_v1) &
> +                              RX_CMP_PAYLOAD_OFFSET) | len;
> +               skb = bp->rx_skb_func(bp, rxr, cons, prod, data, dma_addr,
>

Yes they are combined because they come from 2 different words.  But I
don't have to do the shift for the offset because it is already in the
upper 16-bit.

> I don't mind though, I was just hoping this is some clever optimization
> technique I could learn :)

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[David Miller]

Please _DO NOT_ guard XDP support with an ifdef the user
can modify.

Treat it like any other common netdev feature a driver might
support such as checksum offloading or GRO.

Thanks.

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[Michael Chan]

On Fri, Feb 3, 2017 at 12:49 PM, David Miller <davem@davemloft.net> wrote:
>
> Please _DO NOT_ guard XDP support with an ifdef the user
> can modify.
>
> Treat it like any other common netdev feature a driver might
> support such as checksum offloading or GRO.
>

David, I want to make sure I understand completely.  Are you saying
don't use Kconfig option for XDP?  Have it always available?

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[David Miller]

From: Michael Chan <michael.chan@broadcom.com>
Date: Fri, 3 Feb 2017 13:13:47 -0800

> On Fri, Feb 3, 2017 at 12:49 PM, David Miller <davem@davemloft.net> wrote:
>>
>> Please _DO NOT_ guard XDP support with an ifdef the user
>> can modify.
>>
>> Treat it like any other common netdev feature a driver might
>> support such as checksum offloading or GRO.
>>
> 
> David, I want to make sure I understand completely.  Are you saying
> don't use Kconfig option for XDP?  Have it always available?

Yes.

I don't see a similar config option used in any other driver.

What's really driving me completely mad about driver XDP adoption
is that there is so much inconsistency.

If you do not see another XDP supporting driver do something, don't be
tempted to blaze your own trail and handle something in a unique way.

We don't set precedence by one driver saying "hey it's better to do
things this way, forget what all the other drivers are doing."  Rather
we have a "discussion" about what the appropriate thing is to do and
convert all the drivers only after a decision has been made.

Meanwhile we keep the status quo.

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[Tom Herbert]

On Fri, Feb 3, 2017 at 1:50 PM, David Miller <davem@davemloft.net> wrote:
> From: Michael Chan <michael.chan@broadcom.com>
> Date: Fri, 3 Feb 2017 13:13:47 -0800
>
>> On Fri, Feb 3, 2017 at 12:49 PM, David Miller <davem@davemloft.net> wrote:
>>>
>>> Please _DO NOT_ guard XDP support with an ifdef the user
>>> can modify.
>>>
>>> Treat it like any other common netdev feature a driver might
>>> support such as checksum offloading or GRO.
>>>
>>
>> David, I want to make sure I understand completely.  Are you saying
>> don't use Kconfig option for XDP?  Have it always available?
>
> Yes.
>
> I don't see a similar config option used in any other driver.
>
> What's really driving me completely mad about driver XDP adoption
> is that there is so much inconsistency.
>
> If you do not see another XDP supporting driver do something, don't be
> tempted to blaze your own trail and handle something in a unique way.
>
> We don't set precedence by one driver saying "hey it's better to do
> things this way, forget what all the other drivers are doing."  Rather
> we have a "discussion" about what the appropriate thing is to do and
> convert all the drivers only after a decision has been made.
>
> Meanwhile we keep the status quo.

I am working on some API changes that will hopefully get a little
consistency across these drivers (this includes feature flag
NETIF_F_XDP). This will reduce code some and should be good cleanup,
but XDP is currently very intertwined with the critical data path so
we might need to be looking at this for a while. There's now more
drivers with XDP support than when I started this work, so I don't
think bnxt should wait for this cleanup-- it's just one more driver
we'll have to retrofit.

Tom

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[David Miller]

From: Tom Herbert <tom@herbertland.com>
Date: Fri, 3 Feb 2017 13:58:56 -0800

> On Fri, Feb 3, 2017 at 1:50 PM, David Miller <davem@davemloft.net> wrote:
>> From: Michael Chan <michael.chan@broadcom.com>
>> Date: Fri, 3 Feb 2017 13:13:47 -0800
>>
>>> On Fri, Feb 3, 2017 at 12:49 PM, David Miller <davem@davemloft.net> wrote:
>>>>
>>>> Please _DO NOT_ guard XDP support with an ifdef the user
>>>> can modify.
>>>>
>>>> Treat it like any other common netdev feature a driver might
>>>> support such as checksum offloading or GRO.
>>>>
>>>
>>> David, I want to make sure I understand completely.  Are you saying
>>> don't use Kconfig option for XDP?  Have it always available?
>>
>> Yes.
>>
>> I don't see a similar config option used in any other driver.
>>
>> What's really driving me completely mad about driver XDP adoption
>> is that there is so much inconsistency.
>>
>> If you do not see another XDP supporting driver do something, don't be
>> tempted to blaze your own trail and handle something in a unique way.
>>
>> We don't set precedence by one driver saying "hey it's better to do
>> things this way, forget what all the other drivers are doing."  Rather
>> we have a "discussion" about what the appropriate thing is to do and
>> convert all the drivers only after a decision has been made.
>>
>> Meanwhile we keep the status quo.
> 
> I am working on some API changes that will hopefully get a little
> consistency across these drivers (this includes feature flag
> NETIF_F_XDP). This will reduce code some and should be good cleanup,
> but XDP is currently very intertwined with the critical data path so
> we might need to be looking at this for a while. There's now more
> drivers with XDP support than when I started this work, so I don't
> think bnxt should wait for this cleanup-- it's just one more driver
> we'll have to retrofit.

Of course.

Michael just respin with the Kconfig change and I'll apply your
series.  In fact I was about to until I noticed the XDP Kconfig knob
:)

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[Tom Herbert]

On Fri, Feb 3, 2017 at 2:02 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <tom@herbertland.com>
> Date: Fri, 3 Feb 2017 13:58:56 -0800
>
>> On Fri, Feb 3, 2017 at 1:50 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Michael Chan <michael.chan@broadcom.com>
>>> Date: Fri, 3 Feb 2017 13:13:47 -0800
>>>
>>>> On Fri, Feb 3, 2017 at 12:49 PM, David Miller <davem@davemloft.net> wrote:
>>>>>
>>>>> Please _DO NOT_ guard XDP support with an ifdef the user
>>>>> can modify.
>>>>>
>>>>> Treat it like any other common netdev feature a driver might
>>>>> support such as checksum offloading or GRO.
>>>>>
>>>>
>>>> David, I want to make sure I understand completely.  Are you saying
>>>> don't use Kconfig option for XDP?  Have it always available?
>>>
>>> Yes.
>>>
>>> I don't see a similar config option used in any other driver.
>>>
>>> What's really driving me completely mad about driver XDP adoption
>>> is that there is so much inconsistency.
>>>
>>> If you do not see another XDP supporting driver do something, don't be
>>> tempted to blaze your own trail and handle something in a unique way.
>>>
>>> We don't set precedence by one driver saying "hey it's better to do
>>> things this way, forget what all the other drivers are doing."  Rather
>>> we have a "discussion" about what the appropriate thing is to do and
>>> convert all the drivers only after a decision has been made.
>>>
>>> Meanwhile we keep the status quo.
>>
>> I am working on some API changes that will hopefully get a little
>> consistency across these drivers (this includes feature flag
>> NETIF_F_XDP). This will reduce code some and should be good cleanup,
>> but XDP is currently very intertwined with the critical data path so
>> we might need to be looking at this for a while. There's now more
>> drivers with XDP support than when I started this work, so I don't
>> think bnxt should wait for this cleanup-- it's just one more driver
>> we'll have to retrofit.
>
> Of course.
>
> Michael just respin with the Kconfig change and I'll apply your
> series.  In fact I was about to until I noticed the XDP Kconfig knob
> :)

Meaning no Kconfig and no features flag I assume...

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[David Miller]

From: Tom Herbert <tom@herbertland.com>
Date: Fri, 3 Feb 2017 14:25:00 -0800

> On Fri, Feb 3, 2017 at 2:02 PM, David Miller <davem@davemloft.net> wrote:
>> From: Tom Herbert <tom@herbertland.com>
>> Date: Fri, 3 Feb 2017 13:58:56 -0800
>>
>>> On Fri, Feb 3, 2017 at 1:50 PM, David Miller <davem@davemloft.net> wrote:
>>>> From: Michael Chan <michael.chan@broadcom.com>
>>>> Date: Fri, 3 Feb 2017 13:13:47 -0800
>>>>
>>>>> On Fri, Feb 3, 2017 at 12:49 PM, David Miller <davem@davemloft.net> wrote:
>>>>>>
>>>>>> Please _DO NOT_ guard XDP support with an ifdef the user
>>>>>> can modify.
>>>>>>
>>>>>> Treat it like any other common netdev feature a driver might
>>>>>> support such as checksum offloading or GRO.
>>>>>>
>>>>>
>>>>> David, I want to make sure I understand completely.  Are you saying
>>>>> don't use Kconfig option for XDP?  Have it always available?
>>>>
>>>> Yes.
>>>>
>>>> I don't see a similar config option used in any other driver.
>>>>
>>>> What's really driving me completely mad about driver XDP adoption
>>>> is that there is so much inconsistency.
>>>>
>>>> If you do not see another XDP supporting driver do something, don't be
>>>> tempted to blaze your own trail and handle something in a unique way.
>>>>
>>>> We don't set precedence by one driver saying "hey it's better to do
>>>> things this way, forget what all the other drivers are doing."  Rather
>>>> we have a "discussion" about what the appropriate thing is to do and
>>>> convert all the drivers only after a decision has been made.
>>>>
>>>> Meanwhile we keep the status quo.
>>>
>>> I am working on some API changes that will hopefully get a little
>>> consistency across these drivers (this includes feature flag
>>> NETIF_F_XDP). This will reduce code some and should be good cleanup,
>>> but XDP is currently very intertwined with the critical data path so
>>> we might need to be looking at this for a while. There's now more
>>> drivers with XDP support than when I started this work, so I don't
>>> think bnxt should wait for this cleanup-- it's just one more driver
>>> we'll have to retrofit.
>>
>> Of course.
>>
>> Michael just respin with the Kconfig change and I'll apply your
>> series.  In fact I was about to until I noticed the XDP Kconfig knob
>> :)
> 
> Meaning no Kconfig and no features flag I assume...

For now, yes.

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[Jakub Kicinski]

On Fri, 03 Feb 2017 16:50:54 -0500 (EST), David Miller wrote:
> We don't set precedence by one driver saying "hey it's better to do
> things this way, forget what all the other drivers are doing."  Rather
> we have a "discussion" about what the appropriate thing is to do and
> convert all the drivers only after a decision has been made.

Would making sure that if xdp_adjust_head() changes the starting offset
(and length) of frame and the program returns XDP_PASS - the stack will
see the changes made by xdp_adjust_head() fall under the same follow
the precedent rule?  That is what Martin did for mlx4 and mlx5, and what
John did in virtio, but not what this patch set does (see my comment on
patch 11).

I should have double checked mlx4/mlx5 and made it clearer that there is
a precedent here in my review comment...

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[Michael Chan]

On Fri, Feb 3, 2017 at 4:33 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Fri, 03 Feb 2017 16:50:54 -0500 (EST), David Miller wrote:
>> We don't set precedence by one driver saying "hey it's better to do
>> things this way, forget what all the other drivers are doing."  Rather
>> we have a "discussion" about what the appropriate thing is to do and
>> convert all the drivers only after a decision has been made.
>
> Would making sure that if xdp_adjust_head() changes the starting offset
> (and length) of frame and the program returns XDP_PASS - the stack will
> see the changes made by xdp_adjust_head() fall under the same follow
> the precedent rule?  That is what Martin did for mlx4 and mlx5, and what
> John did in virtio, but not what this patch set does (see my comment on
> patch 11).
>
> I should have double checked mlx4/mlx5 and made it clearer that there is
> a precedent here in my review comment...

Yes, I plan to also include this change (make modified offset and
length visible to the stack) in the next version of the patch set.

Re: [PATCH net-next v2 00/12] bnxt_en: Add XDP support.

[Jakub Kicinski]

On Fri, 3 Feb 2017 17:32:16 -0800, Michael Chan wrote:
> On Fri, Feb 3, 2017 at 4:33 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> > On Fri, 03 Feb 2017 16:50:54 -0500 (EST), David Miller wrote:  
> >> We don't set precedence by one driver saying "hey it's better to do
> >> things this way, forget what all the other drivers are doing."  Rather
> >> we have a "discussion" about what the appropriate thing is to do and
> >> convert all the drivers only after a decision has been made.  
> >
> > Would making sure that if xdp_adjust_head() changes the starting offset
> > (and length) of frame and the program returns XDP_PASS - the stack will
> > see the changes made by xdp_adjust_head() fall under the same follow
> > the precedent rule?  That is what Martin did for mlx4 and mlx5, and what
> > John did in virtio, but not what this patch set does (see my comment on
> > patch 11).
> >
> > I should have double checked mlx4/mlx5 and made it clearer that there is
> > a precedent here in my review comment...  
> 
> Yes, I plan to also include this change (make modified offset and
> length visible to the stack) in the next version of the patch set.

Awesome, thanks!