[dpdk-dev] [PATCH 0/6] net/cxgbe: bug fixes

[dpdk-dev] [PATCH 0/6] net/cxgbe: bug fixes

[Rahul Lakkireddy]

Patch 1 adds a missing PCI un-initialization function for CXGBEVF to
fix resource leaks.

Patch 2 fixes VLAN strip offload always being enabled without looking
at Rx offload flags.

Patch 3 fixes issue with redefined match items in flows being offloaded
to hardware.

Patch 4 fixes flow offload timeouts under heavy load.

Patch 5 fixes wrong ingress port value being written in filter spec passed
to hardware while offloading flows.

Patch 6 fixes freeing illegal memory location while destroying MPS table.

Thanks,
Rahul

Rahul Lakkireddy (6):
  net/cxgbevf: add missing PCI uninitialization function for VF
  net/cxgbe: check Rx offload flags before doing VLAN strip offload
  net/cxgbe: fix check for redefined match items
  net/cxgbe: increase completion wait time for flow operations
  net/cxgbe: fix wrong ingress port value set in filter spec
  net/cxgbevf: fix illegal memory access when freeing MPS TCAM

 drivers/net/cxgbe/cxgbe_filter.c   |  7 ++-----
 drivers/net/cxgbe/cxgbe_flow.c     | 16 +++++++---------
 drivers/net/cxgbe/cxgbe_flow.h     |  5 +++--
 drivers/net/cxgbe/cxgbe_main.c     | 18 +++++++++++-------
 drivers/net/cxgbe/cxgbevf_ethdev.c | 12 +++++++++++-
 drivers/net/cxgbe/cxgbevf_main.c   |  6 ++++++
 drivers/net/cxgbe/mps_tcam.c       |  4 +---
 7 files changed, 41 insertions(+), 27 deletions(-)

-- 
2.18.0

[dpdk-dev] [PATCH 1/6] net/cxgbevf: add missing PCI uninitialization function for VF

[Rahul Lakkireddy]

Fixes: 2195df6d11bd ("net/cxgbe: rework ethdev device allocation")
Cc: stable@dpdk.org

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbevf_ethdev.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/cxgbe/cxgbevf_ethdev.c b/drivers/net/cxgbe/cxgbevf_ethdev.c
index 3b32ca9d4..a6458d533 100644
--- a/drivers/net/cxgbe/cxgbevf_ethdev.c
+++ b/drivers/net/cxgbe/cxgbevf_ethdev.c
@@ -177,6 +177,16 @@ static int eth_cxgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	return err;
 }
 
+static int eth_cxgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
+{
+	struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private);
+	struct adapter *adap = pi->adapter;
+
+	/* Free up other ports and all resources */
+	cxgbe_close(adap);
+	return 0;
+}
+
 static int eth_cxgbevf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 				 struct rte_pci_device *pci_dev)
 {
@@ -186,7 +196,7 @@ static int eth_cxgbevf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 
 static int eth_cxgbevf_pci_remove(struct rte_pci_device *pci_dev)
 {
-	return rte_eth_dev_pci_generic_remove(pci_dev, NULL);
+	return rte_eth_dev_pci_generic_remove(pci_dev, eth_cxgbevf_dev_uninit);
 }
 
 static struct rte_pci_driver rte_cxgbevf_pmd = {
-- 
2.18.0

[dpdk-dev] [PATCH 2/6] net/cxgbe: check Rx offload flags before doing VLAN strip offload

[Rahul Lakkireddy]

Fixes: 436125e64174 ("net/cxgbe: update to Rx/Tx offloada API")
Cc: stable@dpdk.org

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_main.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_main.c b/drivers/net/cxgbe/cxgbe_main.c
index 88dc851f8..409c9f821 100644
--- a/drivers/net/cxgbe/cxgbe_main.c
+++ b/drivers/net/cxgbe/cxgbe_main.c
@@ -1339,18 +1339,22 @@ inline bool force_linkup(struct adapter *adap)
 int link_start(struct port_info *pi)
 {
 	struct adapter *adapter = pi->adapter;
-	int ret;
+	u64 conf_offloads;
 	unsigned int mtu;
+	int ret;
 
 	mtu = pi->eth_dev->data->dev_conf.rxmode.max_rx_pkt_len -
 	      (ETHER_HDR_LEN + ETHER_CRC_LEN);
 
+	conf_offloads = pi->eth_dev->data->dev_conf.rxmode.offloads;
+
 	/*
 	 * We do not set address filters and promiscuity here, the stack does
 	 * that step explicitly.
 	 */
-	ret = t4_set_rxmode(adapter, adapter->mbox, pi->viid, mtu, -1, -1,
-			    -1, 1, true);
+	ret = t4_set_rxmode(adapter, adapter->mbox, pi->viid, mtu, -1, -1, -1,
+			    !!(conf_offloads & DEV_RX_OFFLOAD_VLAN_STRIP),
+			    true);
 	if (ret == 0) {
 		ret = cxgbe_mpstcam_modify(pi, (int)pi->xact_addr_filt,
 				(u8 *)&pi->eth_dev->data->mac_addrs[0]);
-- 
2.18.0

[dpdk-dev] [PATCH 3/6] net/cxgbe: fix check for redefined match items

[Rahul Lakkireddy]

Fix issue where ethertype is set to 0x800 for IPv4 by ether match
item and IPv4 match item also inherently sets ethertype to 0x800.

Current logic returns error when IPv4 match item is parsed. So, fix
by only bailing if the redefined match items have different values.

Fixes: ee61f5113b17 ("net/cxgbe: parse and validate flows")
Cc: stable@dpdk.org

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_flow.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_flow.c b/drivers/net/cxgbe/cxgbe_flow.c
index 54ec7e595..a8f076e6c 100644
--- a/drivers/net/cxgbe/cxgbe_flow.c
+++ b/drivers/net/cxgbe/cxgbe_flow.c
@@ -7,14 +7,12 @@
 
 #define __CXGBE_FILL_FS(__v, __m, fs, elem, e) \
 do { \
-	if (!((fs)->val.elem || (fs)->mask.elem)) { \
-		(fs)->val.elem = (__v); \
-		(fs)->mask.elem = (__m); \
-	} else { \
+	if ((fs)->mask.elem && ((fs)->val.elem != (__v))) \
 		return rte_flow_error_set(e, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, \
-					  NULL, "a filter can be specified" \
-					  " only once"); \
-	} \
+					  NULL, "Redefined match item with" \
+					  " different values found"); \
+	(fs)->val.elem = (__v); \
+	(fs)->mask.elem = (__m); \
 } while (0)
 
 #define __CXGBE_FILL_FS_MEMCPY(__v, __m, fs, elem) \
-- 
2.18.0

[dpdk-dev] [PATCH 4/6] net/cxgbe: increase completion wait time for flow operations

[Rahul Lakkireddy]

Under heavy load, flow related operations can take more time to
complete. Increase max completion wait time to 10 seconds. Also
increase max receive budget to read more replies from firmware
in every cycle.

Fixes: 9eb2c9a48072 ("net/cxgbe: implement flow create operation")
Cc: stable@dpdk.org

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_flow.c | 4 ++--
 drivers/net/cxgbe/cxgbe_flow.h | 5 +++--
 drivers/net/cxgbe/cxgbe_main.c | 8 ++++----
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_flow.c b/drivers/net/cxgbe/cxgbe_flow.c
index a8f076e6c..4deaff8f2 100644
--- a/drivers/net/cxgbe/cxgbe_flow.c
+++ b/drivers/net/cxgbe/cxgbe_flow.c
@@ -797,7 +797,7 @@ static int __cxgbe_flow_create(struct rte_eth_dev *dev, struct rte_flow *flow)
 
 	/* Poll the FW for reply */
 	err = cxgbe_poll_for_completion(&adap->sge.fw_evtq,
-					CXGBE_FLOW_POLL_US,
+					CXGBE_FLOW_POLL_MS,
 					CXGBE_FLOW_POLL_CNT,
 					&ctx.completion);
 	if (err) {
@@ -883,7 +883,7 @@ static int __cxgbe_flow_destroy(struct rte_eth_dev *dev, struct rte_flow *flow)
 
 	/* Poll the FW for reply */
 	err = cxgbe_poll_for_completion(&adap->sge.fw_evtq,
-					CXGBE_FLOW_POLL_US,
+					CXGBE_FLOW_POLL_MS,
 					CXGBE_FLOW_POLL_CNT,
 					&ctx.completion);
 	if (err) {
diff --git a/drivers/net/cxgbe/cxgbe_flow.h b/drivers/net/cxgbe/cxgbe_flow.h
index 718bf3d05..ec8e47aeb 100644
--- a/drivers/net/cxgbe/cxgbe_flow.h
+++ b/drivers/net/cxgbe/cxgbe_flow.h
@@ -10,8 +10,9 @@
 #include "mps_tcam.h"
 #include "cxgbe.h"
 
-#define CXGBE_FLOW_POLL_US  10
-#define CXGBE_FLOW_POLL_CNT 10
+/* Max poll time is 100 * 100msec = 10 sec */
+#define CXGBE_FLOW_POLL_MS  100 /* 100 milliseconds */
+#define CXGBE_FLOW_POLL_CNT 100 /* Max number of times to poll */
 
 struct chrte_fparse {
 	int (*fptr)(const void *mask, /* currently supported mask */
diff --git a/drivers/net/cxgbe/cxgbe_main.c b/drivers/net/cxgbe/cxgbe_main.c
index 409c9f821..ec080e5d3 100644
--- a/drivers/net/cxgbe/cxgbe_main.c
+++ b/drivers/net/cxgbe/cxgbe_main.c
@@ -157,18 +157,18 @@ int setup_sge_ctrl_txq(struct adapter *adapter)
 /**
  * cxgbe_poll_for_completion: Poll rxq for completion
  * @q: rxq to poll
- * @us: microseconds to delay
+ * @ms: milliseconds to delay
  * @cnt: number of times to poll
  * @c: completion to check for 'done' status
  *
  * Polls the rxq for reples until completion is done or the count
  * expires.
  */
-int cxgbe_poll_for_completion(struct sge_rspq *q, unsigned int us,
+int cxgbe_poll_for_completion(struct sge_rspq *q, unsigned int ms,
 			      unsigned int cnt, struct t4_completion *c)
 {
 	unsigned int i;
-	unsigned int work_done, budget = 4;
+	unsigned int work_done, budget = 32;
 
 	if (!c)
 		return -EINVAL;
@@ -181,7 +181,7 @@ int cxgbe_poll_for_completion(struct sge_rspq *q, unsigned int us,
 			return 0;
 		}
 		t4_os_unlock(&c->lock);
-		udelay(us);
+		rte_delay_ms(ms);
 	}
 	return -ETIMEDOUT;
 }
-- 
2.18.0

[dpdk-dev] [PATCH 5/6] net/cxgbe: fix wrong ingress port value set in filter spec

[Rahul Lakkireddy]

Set the "value" in the filter spec and not the "mask". Also, remove
duplicate comparison for hardware supported match items already done
earlier.

Fixes: af44a577988b ("net/cxgbe: support to offload flows to HASH region")
Cc: stable@dpdk.org

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbe_filter.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbe_filter.c b/drivers/net/cxgbe/cxgbe_filter.c
index ef1102be3..3a7912e48 100644
--- a/drivers/net/cxgbe/cxgbe_filter.c
+++ b/drivers/net/cxgbe/cxgbe_filter.c
@@ -263,8 +263,8 @@ static u64 hash_filter_ntuple(const struct filter_entry *f)
 	u64 ntuple = 0;
 	u16 tcp_proto = IPPROTO_TCP; /* TCP Protocol Number */
 
-	if (tp->port_shift >= 0)
-		ntuple |= (u64)f->fs.mask.iport << tp->port_shift;
+	if (tp->port_shift >= 0 && f->fs.mask.iport)
+		ntuple |= (u64)f->fs.val.iport << tp->port_shift;
 
 	if (tp->protocol_shift >= 0) {
 		if (!f->fs.val.proto)
@@ -278,9 +278,6 @@ static u64 hash_filter_ntuple(const struct filter_entry *f)
 	if (tp->macmatch_shift >= 0 && f->fs.mask.macidx)
 		ntuple |= (u64)(f->fs.val.macidx) << tp->macmatch_shift;
 
-	if (ntuple != tp->hash_filter_mask)
-		return 0;
-
 	return ntuple;
 }
 
-- 
2.18.0

[dpdk-dev] [PATCH 6/6] net/cxgbevf: fix illegal memory access when freeing MPS TCAM

[Rahul Lakkireddy]

Individual MPS TCAM entries are not allocated as separate entities.
All entries are allocated once as an array. So, fix bug with attempting
to free illegal memory location.

Also add missing MPS TCAM initialization for CXGBEVF.

Fixes: 6fda3f0ddda9 ("net/cxgbe: add API to program hardware MPS table")

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/cxgbevf_main.c | 6 ++++++
 drivers/net/cxgbe/mps_tcam.c     | 4 +---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/cxgbe/cxgbevf_main.c b/drivers/net/cxgbe/cxgbevf_main.c
index 6223e1250..61bd85193 100644
--- a/drivers/net/cxgbe/cxgbevf_main.c
+++ b/drivers/net/cxgbe/cxgbevf_main.c
@@ -11,6 +11,7 @@
 #include "t4_regs.h"
 #include "t4_msg.h"
 #include "cxgbe.h"
+#include "mps_tcam.h"
 
 /*
  * Figure out how many Ports and Queue Sets we can support.  This depends on
@@ -271,6 +272,11 @@ int cxgbevf_probe(struct adapter *adapter)
 	print_adapter_info(adapter);
 	print_port_info(adapter);
 
+	adapter->mpstcam = t4_init_mpstcam(adapter);
+	if (!adapter->mpstcam)
+		dev_warn(adapter,
+			 "VF could not allocate mps tcam table. Continuing\n");
+
 	err = init_rss(adapter);
 	if (err)
 		goto out_free;
diff --git a/drivers/net/cxgbe/mps_tcam.c b/drivers/net/cxgbe/mps_tcam.c
index 02ec69a92..71c8070b3 100644
--- a/drivers/net/cxgbe/mps_tcam.c
+++ b/drivers/net/cxgbe/mps_tcam.c
@@ -236,8 +236,6 @@ struct mpstcam_table *t4_init_mpstcam(struct adapter *adap)
 
 void t4_cleanup_mpstcam(struct adapter *adap)
 {
-	if (adap->mpstcam) {
-		t4_os_free(adap->mpstcam->entry);
+	if (adap->mpstcam)
 		t4_os_free(adap->mpstcam);
-	}
 }
-- 
2.18.0

Re: [dpdk-dev] [PATCH 0/6] net/cxgbe: bug fixes

[Ferruh Yigit]

On 11/9/2018 7:56 AM, Rahul Lakkireddy wrote:
> Patch 1 adds a missing PCI un-initialization function for CXGBEVF to
> fix resource leaks.
> 
> Patch 2 fixes VLAN strip offload always being enabled without looking
> at Rx offload flags.
> 
> Patch 3 fixes issue with redefined match items in flows being offloaded
> to hardware.
> 
> Patch 4 fixes flow offload timeouts under heavy load.
> 
> Patch 5 fixes wrong ingress port value being written in filter spec passed
> to hardware while offloading flows.
> 
> Patch 6 fixes freeing illegal memory location while destroying MPS table.
> 
> Thanks,
> Rahul
> 
> Rahul Lakkireddy (6):
>   net/cxgbevf: add missing PCI uninitialization function for VF
>   net/cxgbe: check Rx offload flags before doing VLAN strip offload
>   net/cxgbe: fix check for redefined match items
>   net/cxgbe: increase completion wait time for flow operations
>   net/cxgbe: fix wrong ingress port value set in filter spec
>   net/cxgbevf: fix illegal memory access when freeing MPS TCAM

Series applied to dpdk-next-net/master, thanks.