DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes
@ 2015-05-29  8:10 Chen Jing D(Mark)
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 1/6] fm10k: Fix improper RX buffer size assignment Chen Jing D(Mark)
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Chen Jing D(Mark) @ 2015-05-29  8:10 UTC (permalink / raw)
  To: dev; +Cc: shaopeng.he

From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>

This patch set include a few bug fixes and enhancements on fm10k driver.

Chen Jing D(Mark) (6):
  fm10k: Fix improper RX buffer size assignment
  fm10k: Fix jumbo frame issue
  fm10k: Fix data integrity issue with multi-segment frame
  fm10k: Fix issue that MAC addr can't be set to silicon
  fm10k: Do sanity check on mac address
  fm10k: Add default mac/vlan filter to SM

 drivers/net/fm10k/fm10k.h        |    5 +-
 drivers/net/fm10k/fm10k_ethdev.c |  100 ++++++++++++++++++++++++++-----------
 drivers/net/fm10k/fm10k_rxtx.c   |   15 +++++-
 3 files changed, 86 insertions(+), 34 deletions(-)

-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 1/6] fm10k: Fix improper RX buffer size assignment
  2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
@ 2015-05-29  8:10 ` Chen Jing D(Mark)
  2015-06-16 11:55   ` Qiu, Michael
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 2/6] fm10k: Fix jumbo frame issue Chen Jing D(Mark)
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Chen Jing D(Mark) @ 2015-05-29  8:10 UTC (permalink / raw)
  To: dev; +Cc: shaopeng.he

From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>

As RX buffer is aligned to 512B within mbuf, some bytes are reserved
for this purpose, and the worst case could be 511B. But SRR reg
assumes all buffers have the same size. In order to fill the gap,
we'll have to consider the worsst case and assume 512B is reserved.
If we don't do so, it's possible for HW to overwrite data to next
mbuf.

Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
 drivers/net/fm10k/fm10k.h        |    5 +++--
 drivers/net/fm10k/fm10k_ethdev.c |   12 ++++++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/fm10k/fm10k.h b/drivers/net/fm10k/fm10k.h
index 0e31796..ad7a7d1 100644
--- a/drivers/net/fm10k/fm10k.h
+++ b/drivers/net/fm10k/fm10k.h
@@ -191,7 +191,8 @@ struct fm10k_tx_queue {
 
 /* enforce 512B alignment on default Rx DMA addresses */
 #define MBUF_DMA_ADDR_DEFAULT(mb) \
-	((uint64_t) RTE_ALIGN(((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM), 512))
+	((uint64_t) RTE_ALIGN(((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM),\
+			FM10K_RX_DATABUF_ALIGN))
 
 static inline void fifo_reset(struct fifo *fifo, uint32_t len)
 {
@@ -263,7 +264,7 @@ fm10k_addr_alignment_valid(struct rte_mbuf *mb)
 	uint64_t boundary1, boundary2;
 
 	/* 512B aligned? */
-	if (RTE_ALIGN(addr, 512) == addr)
+	if (RTE_ALIGN(addr, FM10K_RX_DATABUF_ALIGN) == addr)
 		return 1;
 
 	/* 8B aligned, and max Ethernet frame would not cross a 4KB boundary? */
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 275c19c..a5e09a0 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -41,7 +41,6 @@
 #include "fm10k.h"
 #include "base/fm10k_api.h"
 
-#define FM10K_RX_BUFF_ALIGN 512
 /* Default delay to acquire mailbox lock */
 #define FM10K_MBXLOCK_DELAY_US 20
 #define UINT64_LOWER_32BITS_MASK 0x00000000ffffffffULL
@@ -426,6 +425,15 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
 		/* Configure the Rx buffer size for one buff without split */
 		buf_size = (uint16_t)(rte_pktmbuf_data_room_size(rxq->mp) -
 			RTE_PKTMBUF_HEADROOM);
+		/* As RX buffer is aligned to 512B within mbuf, some bytes are
+		 * reserved for this purpose, and the worst case could be 511B.
+		 * But SRR reg assumes all buffers have the same size. In order
+		 * to fill the gap, we'll have to consider the worst case and
+		 * assume 512B is reserved. If we don't do so, it's possible
+		 * for HW to overwrite data to next mbuf.
+		 */
+		buf_size -= FM10K_RX_DATABUF_ALIGN;
+
 		FM10K_WRITE_REG(hw, FM10K_SRRCTL(i),
 				buf_size >> FM10K_SRRCTL_BSIZEPKT_SHIFT);
 
@@ -909,7 +917,7 @@ mempool_element_size_valid(struct rte_mempool *mp)
 			RTE_PKTMBUF_HEADROOM;
 
 	/* account for up to 512B of alignment */
-	min_size -= FM10K_RX_BUFF_ALIGN;
+	min_size -= FM10K_RX_DATABUF_ALIGN;
 
 	/* sanity check for overflow */
 	if (min_size > mp->elt_size)
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 2/6] fm10k: Fix jumbo frame issue
  2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 1/6] fm10k: Fix improper RX buffer size assignment Chen Jing D(Mark)
@ 2015-05-29  8:10 ` Chen Jing D(Mark)
  2015-06-12  1:15   ` Qiu, Michael
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 3/6] fm10k: Fix data integrity issue with multi-segment frame Chen Jing D(Mark)
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Chen Jing D(Mark) @ 2015-05-29  8:10 UTC (permalink / raw)
  To: dev; +Cc: shaopeng.he

From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>

fm10k can't receive frame greater than 1536 and Scatter RX
function can't work correctly. The root cause is
SRRCTL.FM10K_SRRCTL_BUFFER_CHAINING_EN bit is not enabled.

Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index a5e09a0..19e718b 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -439,9 +439,14 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
 
 		/* It adds dual VLAN length for supporting dual VLAN */
 		if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
-				2 * FM10K_VLAN_TAG_SIZE) > buf_size){
+				2 * FM10K_VLAN_TAG_SIZE) > buf_size ||
+			dev->data->dev_conf.rxmode.enable_scatter) {
+			uint32_t reg;
 			dev->data->scattered_rx = 1;
 			dev->rx_pkt_burst = fm10k_recv_scattered_pkts;
+			reg = FM10K_READ_REG(hw, FM10K_SRRCTL(i));
+			reg |= FM10K_SRRCTL_BUFFER_CHAINING_EN;
+			FM10K_WRITE_REG(hw, FM10K_SRRCTL(i), reg);
 		}
 
 		/* Enable drop on empty, it's RO for VF */
@@ -452,11 +457,6 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
 		FM10K_WRITE_FLUSH(hw);
 	}
 
-	if (dev->data->dev_conf.rxmode.enable_scatter) {
-		dev->rx_pkt_burst = fm10k_recv_scattered_pkts;
-		dev->data->scattered_rx = 1;
-	}
-
 	/* Configure RSS if applicable */
 	fm10k_dev_mq_rx_configure(dev);
 	return 0;
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 3/6] fm10k: Fix data integrity issue with multi-segment frame
  2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 1/6] fm10k: Fix improper RX buffer size assignment Chen Jing D(Mark)
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 2/6] fm10k: Fix jumbo frame issue Chen Jing D(Mark)
@ 2015-05-29  8:10 ` Chen Jing D(Mark)
  2015-06-16 12:07   ` Qiu, Michael
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon Chen Jing D(Mark)
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Chen Jing D(Mark) @ 2015-05-29  8:10 UTC (permalink / raw)
  To: dev; +Cc: shaopeng.he

From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>

In TX side, bit FM10K_TXD_FLAG_LAST in TX descriptor only is set
in the last descriptor for multi-segment packets. But current
implementation didn't set all the fields of TX descriptor, which
will cause descriptors processed now to re-use fields set in last
scroll. If FM10K_TXD_FLAG_LAST bit was set in the last round and
it happened this is not the last descriptor of a multi-segnment
packet, HW will send out the incomplete packet out and leads to
data intergrity issue.

Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
 drivers/net/fm10k/fm10k_rxtx.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index 56df6cd..f5d1ad0 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -402,9 +402,9 @@ static inline void tx_xmit_pkt(struct fm10k_tx_queue *q, struct rte_mbuf *mb)
 		q->nb_used = q->nb_used + mb->nb_segs;
 	}
 
-	q->hw_ring[last_id].flags = flags;
 	q->nb_free -= mb->nb_segs;
 
+	q->hw_ring[q->next_free].flags = 0;
 	/* set checksum flags on first descriptor of packet. SCTP checksum
 	 * offload is not supported, but we do not explicitly check for this
 	 * case in favor of greatly simplified processing. */
@@ -415,16 +415,27 @@ static inline void tx_xmit_pkt(struct fm10k_tx_queue *q, struct rte_mbuf *mb)
 	if (mb->ol_flags & PKT_TX_VLAN_PKT)
 		q->hw_ring[q->next_free].vlan = mb->vlan_tci;
 
+	q->sw_ring[q->next_free] = mb;
+	q->hw_ring[q->next_free].buffer_addr =
+			rte_cpu_to_le_64(MBUF_DMA_ADDR(mb));
+	q->hw_ring[q->next_free].buflen =
+			rte_cpu_to_le_16(rte_pktmbuf_data_len(mb));
+	if (++q->next_free == q->nb_desc)
+		q->next_free = 0;
+
 	/* fill up the rings */
-	for (; mb != NULL; mb = mb->next) {
+	for (mb = mb->next; mb != NULL; mb = mb->next) {
 		q->sw_ring[q->next_free] = mb;
 		q->hw_ring[q->next_free].buffer_addr =
 				rte_cpu_to_le_64(MBUF_DMA_ADDR(mb));
 		q->hw_ring[q->next_free].buflen =
 				rte_cpu_to_le_16(rte_pktmbuf_data_len(mb));
+		q->hw_ring[q->next_free].flags = 0;
 		if (++q->next_free == q->nb_desc)
 			q->next_free = 0;
 	}
+
+	q->hw_ring[last_id].flags = flags;
 }
 
 uint16_t
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon
  2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
                   ` (2 preceding siblings ...)
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 3/6] fm10k: Fix data integrity issue with multi-segment frame Chen Jing D(Mark)
@ 2015-05-29  8:10 ` Chen Jing D(Mark)
  2015-06-09 16:23   ` Qiu, Michael
  2015-06-12  2:03   ` Qiu, Michael
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 5/6] fm10k: Do sanity check on mac address Chen Jing D(Mark)
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: Chen Jing D(Mark) @ 2015-05-29  8:10 UTC (permalink / raw)
  To: dev; +Cc: shaopeng.he

From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>

In fm10k, PF driver needs to communicate with switch through
mailbox if it needs to add/delete MAC address.
This fix will validate if switch is ready before going forward.
Then, it is necessary to acquire LPORT_MAP info after issuing
MAC addr request to switch.

Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c |   34 +++++++++++++++++++++++++++++++---
 1 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 19e718b..dedfbb4 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -45,6 +45,10 @@
 #define FM10K_MBXLOCK_DELAY_US 20
 #define UINT64_LOWER_32BITS_MASK 0x00000000ffffffffULL
 
+/* Max try times to aquire switch status */
+#define MAX_QUERY_SWITCH_STATE_TIMES 10
+/* Wait interval to get switch status */
+#define WAIT_SWITCH_MSG_US    100000
 /* Number of chars per uint32 type */
 #define CHARS_PER_UINT32 (sizeof(uint32_t))
 #define BIT_MASK_PER_UINT32 ((1 << CHARS_PER_UINT32) - 1)
@@ -1802,6 +1806,32 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 		fm10k_dev_enable_intr_vf(dev);
 	}
 
+	/* Enable uio intr after callback registered */
+	rte_intr_enable(&(dev->pci_dev->intr_handle));
+
+	hw->mac.ops.update_int_moderator(hw);
+
+	/* Make sure Switch Manager is ready before going forward. */
+	if (hw->mac.type == fm10k_mac_pf) {
+		int switch_ready = 0;
+		int i;
+
+		for (i = 0; i < MAX_QUERY_SWITCH_STATE_TIMES; i++) {
+			fm10k_mbx_lock(hw);
+			hw->mac.ops.get_host_state(hw, &switch_ready);
+			fm10k_mbx_unlock(hw);
+			if (switch_ready)
+				break;
+			/* Delay some time to acquire async LPORT_MAP info. */
+			rte_delay_us(WAIT_SWITCH_MSG_US);
+		}
+
+		if (switch_ready == 0) {
+			PMD_INIT_LOG(ERR, "switch is not ready");
+			return -1;
+		}
+	}
+
 	/*
 	 * Below function will trigger operations on mailbox, acquire lock to
 	 * avoid race condition from interrupt handler. Operations on mailbox
@@ -1811,7 +1841,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	 */
 	fm10k_mbx_lock(hw);
 	/* Enable port first */
-	hw->mac.ops.update_lport_state(hw, 0, 0, 1);
+	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
 
 	/* Update default vlan */
 	hw->mac.ops.update_vlan(hw, hw->mac.default_vid, 0, true);
@@ -1831,8 +1861,6 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 
 	fm10k_mbx_unlock(hw);
 
-	/* enable uio intr after callback registered */
-	rte_intr_enable(&(dev->pci_dev->intr_handle));
 
 	return 0;
 }
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 5/6] fm10k: Do sanity check on mac address
  2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
                   ` (3 preceding siblings ...)
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon Chen Jing D(Mark)
@ 2015-05-29  8:10 ` Chen Jing D(Mark)
  2015-06-16 12:08   ` Qiu, Michael
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 6/6] fm10k: Add default mac/vlan filter to SM Chen Jing D(Mark)
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Chen Jing D(Mark) @ 2015-05-29  8:10 UTC (permalink / raw)
  To: dev; +Cc: shaopeng.he

From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>

After acquiring MAC address from HW, it's necessary to validate
MAC address before use.

Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c |   24 ++++++++++--------------
 1 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index dedfbb4..b6e82e3 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1756,24 +1756,20 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	}
 
 	diag = fm10k_read_mac_addr(hw);
-	if (diag != FM10K_SUCCESS) {
-		/*
-		 * TODO: remove special handling on VF. Need shared code to
-		 * fix first.
-		 */
-		if (hw->mac.type == fm10k_mac_pf) {
-			PMD_INIT_LOG(ERR, "Read MAC addr failed: %d", diag);
-			return -EIO;
-		} else {
-			/* Generate a random addr */
-			eth_random_addr(hw->mac.addr);
-			memcpy(hw->mac.perm_addr, hw->mac.addr, ETH_ALEN);
-		}
-	}
 
 	ether_addr_copy((const struct ether_addr *)hw->mac.addr,
 			&dev->data->mac_addrs[0]);
 
+	if (diag != FM10K_SUCCESS ||
+		!is_valid_assigned_ether_addr(dev->data->mac_addrs)) {
+
+		/* Generate a random addr */
+		eth_random_addr(hw->mac.addr);
+		memcpy(hw->mac.perm_addr, hw->mac.addr, ETH_ALEN);
+		ether_addr_copy((const struct ether_addr *)hw->mac.addr,
+		&dev->data->mac_addrs[0]);
+	}
+
 	/* Reset the hw statistics */
 	fm10k_stats_reset(dev);
 
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 6/6] fm10k: Add default mac/vlan filter to SM
  2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
                   ` (4 preceding siblings ...)
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 5/6] fm10k: Do sanity check on mac address Chen Jing D(Mark)
@ 2015-05-29  8:10 ` Chen Jing D(Mark)
  2015-06-08  2:39 ` [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes He, Shaopeng
  2015-06-09 15:21 ` Qiu, Michael
  7 siblings, 0 replies; 17+ messages in thread
From: Chen Jing D(Mark) @ 2015-05-29  8:10 UTC (permalink / raw)
  To: dev; +Cc: shaopeng.he

From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>

Since the communication between PF/Switch Manager, VF/PF is
asynchronous through mailbox, it's hard to determine when Switch
Manager/PF host will send the default vlan to PF/VF. So, it's
necessary to set default vlan until the device is started.

Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index b6e82e3..3a26480 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -667,6 +667,17 @@ fm10k_dev_start(struct rte_eth_dev *dev)
 		}
 	}
 
+	if (hw->mac.default_vid && hw->mac.default_vid <= ETHER_MAX_VLAN_ID) {
+		fm10k_mbx_lock(hw);
+		/* Update default vlan */
+		hw->mac.ops.update_vlan(hw, hw->mac.default_vid, 0, true);
+
+		/* Add default mac/vlan filter to PF/Switch manger */
+		hw->mac.ops.update_uc_addr(hw, hw->mac.dglort_map, hw->mac.addr,
+				hw->mac.default_vid, true, 0);
+		fm10k_mbx_unlock(hw);
+	}
+
 	return 0;
 }
 
@@ -1839,15 +1850,12 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	/* Enable port first */
 	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
 
-	/* Update default vlan */
-	hw->mac.ops.update_vlan(hw, hw->mac.default_vid, 0, true);
-
 	/*
-	 * Add default mac/vlan filter. glort is assigned by SM for PF, while is
+	 * Add default mac. glort is assigned by SM for PF, while is
 	 * unused for VF. PF will assign correct glort for VF.
 	 */
 	hw->mac.ops.update_uc_addr(hw, hw->mac.dglort_map, hw->mac.addr,
-			      hw->mac.default_vid, 1, 0);
+				0, 1, 0);
 
 	/* Set unicast mode by default. App can change to other mode in other
 	 * API func.
-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes
  2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
                   ` (5 preceding siblings ...)
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 6/6] fm10k: Add default mac/vlan filter to SM Chen Jing D(Mark)
@ 2015-06-08  2:39 ` He, Shaopeng
  2015-06-09 15:21 ` Qiu, Michael
  7 siblings, 0 replies; 17+ messages in thread
From: He, Shaopeng @ 2015-06-08  2:39 UTC (permalink / raw)
  To: dev


> -----Original Message-----
> From: Chen, Jing D
> Sent: Friday, May 29, 2015 4:11 PM
> To: dev@dpdk.org
> Cc: He, Shaopeng; Qiu, Michael; Chen, Jing D
> Subject: [PATCH 0/6] fm10k: A series of bug fixes
> 
> From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
> 
> This patch set include a few bug fixes and enhancements on fm10k driver.
> 
> Chen Jing D(Mark) (6):
>   fm10k: Fix improper RX buffer size assignment
>   fm10k: Fix jumbo frame issue
>   fm10k: Fix data integrity issue with multi-segment frame
>   fm10k: Fix issue that MAC addr can't be set to silicon
>   fm10k: Do sanity check on mac address
>   fm10k: Add default mac/vlan filter to SM
> 
>  drivers/net/fm10k/fm10k.h        |    5 +-
>  drivers/net/fm10k/fm10k_ethdev.c |  100
> ++++++++++++++++++++++++++-----------
>  drivers/net/fm10k/fm10k_rxtx.c   |   15 +++++-
>  3 files changed, 86 insertions(+), 34 deletions(-)
> 
> --
> 1.7.7.6

Acked-by: Shaopeng He <Shaopeng.he@intel.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes
  2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
                   ` (6 preceding siblings ...)
  2015-06-08  2:39 ` [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes He, Shaopeng
@ 2015-06-09 15:21 ` Qiu, Michael
  2015-06-22 13:55   ` Thomas Monjalon
  7 siblings, 1 reply; 17+ messages in thread
From: Qiu, Michael @ 2015-06-09 15:21 UTC (permalink / raw)
  To: Chen, Jing D, dev; +Cc: He, Shaopeng

On 2015/5/29 16:11, Chen, Jing D wrote:
> From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
>
> This patch set include a few bug fixes and enhancements on fm10k driver.
>
> Chen Jing D(Mark) (6):
>   fm10k: Fix improper RX buffer size assignment
>   fm10k: Fix jumbo frame issue
>   fm10k: Fix data integrity issue with multi-segment frame
>   fm10k: Fix issue that MAC addr can't be set to silicon
>   fm10k: Do sanity check on mac address
>   fm10k: Add default mac/vlan filter to SM
>
>  drivers/net/fm10k/fm10k.h        |    5 +-
>  drivers/net/fm10k/fm10k_ethdev.c |  100 ++++++++++++++++++++++++++-----------
>  drivers/net/fm10k/fm10k_rxtx.c   |   15 +++++-
>  3 files changed, 86 insertions(+), 34 deletions(-)
>
Acked-by: Michael Qiu <michael.qiu@intel.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon Chen Jing D(Mark)
@ 2015-06-09 16:23   ` Qiu, Michael
  2015-06-10  5:34     ` Chen, Jing D
  2015-06-12  2:03   ` Qiu, Michael
  1 sibling, 1 reply; 17+ messages in thread
From: Qiu, Michael @ 2015-06-09 16:23 UTC (permalink / raw)
  To: Chen, Jing D, dev; +Cc: He, Shaopeng

On 2015/5/29 16:11, Chen, Jing D wrote:
> From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
>
> In fm10k, PF driver needs to communicate with switch through
> mailbox if it needs to add/delete MAC address.
> This fix will validate if switch is ready before going forward.
> Then, it is necessary to acquire LPORT_MAP info after issuing
> MAC addr request to switch.
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c |   34 +++++++++++++++++++++++++++++++---
>  1 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index 19e718b..dedfbb4 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -45,6 +45,10 @@
>  #define FM10K_MBXLOCK_DELAY_US 20
>  #define UINT64_LOWER_32BITS_MASK 0x00000000ffffffffULL
>  
> +/* Max try times to aquire switch status */
> +#define MAX_QUERY_SWITCH_STATE_TIMES 10
> +/* Wait interval to get switch status */
> +#define WAIT_SWITCH_MSG_US    100000
>  /* Number of chars per uint32 type */
>  #define CHARS_PER_UINT32 (sizeof(uint32_t))
>  #define BIT_MASK_PER_UINT32 ((1 << CHARS_PER_UINT32) - 1)
> @@ -1802,6 +1806,32 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  		fm10k_dev_enable_intr_vf(dev);
>  	}
>  
> +	/* Enable uio intr after callback registered */
> +	rte_intr_enable(&(dev->pci_dev->intr_handle));
> +
> +	hw->mac.ops.update_int_moderator(hw);
> +
> +	/* Make sure Switch Manager is ready before going forward. */
> +	if (hw->mac.type == fm10k_mac_pf) {
> +		int switch_ready = 0;
> +		int i;
> +
> +		for (i = 0; i < MAX_QUERY_SWITCH_STATE_TIMES; i++) {
> +			fm10k_mbx_lock(hw);
> +			hw->mac.ops.get_host_state(hw, &switch_ready);
> +			fm10k_mbx_unlock(hw);
> +			if (switch_ready)
> +				break;
> +			/* Delay some time to acquire async LPORT_MAP info. */
> +			rte_delay_us(WAIT_SWITCH_MSG_US);
> +		}
> +
> +		if (switch_ready == 0) {
> +			PMD_INIT_LOG(ERR, "switch is not ready");
> +			return -1;

Here better to return  -EIO or other error code? "-1" seems not keep the
same style with other return routine in this function.

Thanks,
Michael


> +		}
> +	}
> +
>  	/*
>  	 * Below function will trigger operations on mailbox, acquire lock to
>  	 * avoid race condition from interrupt handler. Operations on mailbox
> @@ -1811,7 +1841,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  	 */
>  	fm10k_mbx_lock(hw);
>  	/* Enable port first */
> -	hw->mac.ops.update_lport_state(hw, 0, 0, 1);
> +	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
>  
>  	/* Update default vlan */
>  	hw->mac.ops.update_vlan(hw, hw->mac.default_vid, 0, true);
> @@ -1831,8 +1861,6 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  
>  	fm10k_mbx_unlock(hw);
>  
> -	/* enable uio intr after callback registered */
> -	rte_intr_enable(&(dev->pci_dev->intr_handle));
>  
>  	return 0;
>  }


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon
  2015-06-09 16:23   ` Qiu, Michael
@ 2015-06-10  5:34     ` Chen, Jing D
  0 siblings, 0 replies; 17+ messages in thread
From: Chen, Jing D @ 2015-06-10  5:34 UTC (permalink / raw)
  To: Qiu, Michael, dev; +Cc: He, Shaopeng

Hi, Michael,

> -----Original Message-----
> From: Qiu, Michael
> Sent: Wednesday, June 10, 2015 12:24 AM
> To: Chen, Jing D; dev@dpdk.org
> Cc: He, Shaopeng
> Subject: Re: [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon
> 
> On 2015/5/29 16:11, Chen, Jing D wrote:
> > From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
> >
> > In fm10k, PF driver needs to communicate with switch through
> > mailbox if it needs to add/delete MAC address.
> > This fix will validate if switch is ready before going forward.
> > Then, it is necessary to acquire LPORT_MAP info after issuing
> > MAC addr request to switch.
> >
> > Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> > ---
> >  drivers/net/fm10k/fm10k_ethdev.c |   34
> +++++++++++++++++++++++++++++++---
> >  1 files changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> > index 19e718b..dedfbb4 100644
> > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > @@ -45,6 +45,10 @@
> >  #define FM10K_MBXLOCK_DELAY_US 20
> >  #define UINT64_LOWER_32BITS_MASK 0x00000000ffffffffULL
> >
> > +/* Max try times to aquire switch status */
> > +#define MAX_QUERY_SWITCH_STATE_TIMES 10
> > +/* Wait interval to get switch status */
> > +#define WAIT_SWITCH_MSG_US    100000
> >  /* Number of chars per uint32 type */
> >  #define CHARS_PER_UINT32 (sizeof(uint32_t))
> >  #define BIT_MASK_PER_UINT32 ((1 << CHARS_PER_UINT32) - 1)
> > @@ -1802,6 +1806,32 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
> >  		fm10k_dev_enable_intr_vf(dev);
> >  	}
> >
> > +	/* Enable uio intr after callback registered */
> > +	rte_intr_enable(&(dev->pci_dev->intr_handle));
> > +
> > +	hw->mac.ops.update_int_moderator(hw);
> > +
> > +	/* Make sure Switch Manager is ready before going forward. */
> > +	if (hw->mac.type == fm10k_mac_pf) {
> > +		int switch_ready = 0;
> > +		int i;
> > +
> > +		for (i = 0; i < MAX_QUERY_SWITCH_STATE_TIMES; i++) {
> > +			fm10k_mbx_lock(hw);
> > +			hw->mac.ops.get_host_state(hw, &switch_ready);
> > +			fm10k_mbx_unlock(hw);
> > +			if (switch_ready)
> > +				break;
> > +			/* Delay some time to acquire async LPORT_MAP
> info. */
> > +			rte_delay_us(WAIT_SWITCH_MSG_US);
> > +		}
> > +
> > +		if (switch_ready == 0) {
> > +			PMD_INIT_LOG(ERR, "switch is not ready");
> > +			return -1;
> 
> Here better to return  -EIO or other error code? "-1" seems not keep the
> same style with other return routine in this function.

Thanks for your comments!
I can't find a good error number to describe what's happening. Even with EIO, application
can't tell what kind of error is returned. So, I'd like to keep it unchanged.

> 
> Thanks,
> Michael
> 
> 
> > +		}
> > +	}
> > +
> >  	/*
> >  	 * Below function will trigger operations on mailbox, acquire lock to
> >  	 * avoid race condition from interrupt handler. Operations on mailbox
> > @@ -1811,7 +1841,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
> >  	 */
> >  	fm10k_mbx_lock(hw);
> >  	/* Enable port first */
> > -	hw->mac.ops.update_lport_state(hw, 0, 0, 1);
> > +	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
> >
> >  	/* Update default vlan */
> >  	hw->mac.ops.update_vlan(hw, hw->mac.default_vid, 0, true);
> > @@ -1831,8 +1861,6 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
> >
> >  	fm10k_mbx_unlock(hw);
> >
> > -	/* enable uio intr after callback registered */
> > -	rte_intr_enable(&(dev->pci_dev->intr_handle));
> >
> >  	return 0;
> >  }

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 2/6] fm10k: Fix jumbo frame issue
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 2/6] fm10k: Fix jumbo frame issue Chen Jing D(Mark)
@ 2015-06-12  1:15   ` Qiu, Michael
  0 siblings, 0 replies; 17+ messages in thread
From: Qiu, Michael @ 2015-06-12  1:15 UTC (permalink / raw)
  To: Chen, Jing D, dev; +Cc: He, Shaopeng

Tested-by: Michael Qiu <michael.qiu@intel.com>

- OS: Fedora20  3.11.10-301
- GCC: gcc version 4.8.3 2014911
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Ethernet controller: Intel Corporation Device 15a4 (rev 01)
- Default x86_64-native-linuxapp-gcc configuration
- Total 5 cases, 5 passed, 0 failed

- Case: Normal frames with no jumbo frame support
- Case: Jumbo frames with no jumbo frame support
- Case: Normal frames with jumbo frame support
- Case: Jumbo frames with jumbo frame support
- Case: Frames bigger than jumbo frames, wwith jumbo frame support


On 5/29/2015 4:11 PM, Chen, Jing D wrote:
> From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
>
> fm10k can't receive frame greater than 1536 and Scatter RX
> function can't work correctly. The root cause is
> SRRCTL.FM10K_SRRCTL_BUFFER_CHAINING_EN bit is not enabled.
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index a5e09a0..19e718b 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -439,9 +439,14 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
>  
>  		/* It adds dual VLAN length for supporting dual VLAN */
>  		if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
> -				2 * FM10K_VLAN_TAG_SIZE) > buf_size){
> +				2 * FM10K_VLAN_TAG_SIZE) > buf_size ||
> +			dev->data->dev_conf.rxmode.enable_scatter) {
> +			uint32_t reg;
>  			dev->data->scattered_rx = 1;
>  			dev->rx_pkt_burst = fm10k_recv_scattered_pkts;
> +			reg = FM10K_READ_REG(hw, FM10K_SRRCTL(i));
> +			reg |= FM10K_SRRCTL_BUFFER_CHAINING_EN;
> +			FM10K_WRITE_REG(hw, FM10K_SRRCTL(i), reg);
>  		}
>  
>  		/* Enable drop on empty, it's RO for VF */
> @@ -452,11 +457,6 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
>  		FM10K_WRITE_FLUSH(hw);
>  	}
>  
> -	if (dev->data->dev_conf.rxmode.enable_scatter) {
> -		dev->rx_pkt_burst = fm10k_recv_scattered_pkts;
> -		dev->data->scattered_rx = 1;
> -	}
> -
>  	/* Configure RSS if applicable */
>  	fm10k_dev_mq_rx_configure(dev);
>  	return 0;


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon Chen Jing D(Mark)
  2015-06-09 16:23   ` Qiu, Michael
@ 2015-06-12  2:03   ` Qiu, Michael
  1 sibling, 0 replies; 17+ messages in thread
From: Qiu, Michael @ 2015-06-12  2:03 UTC (permalink / raw)
  To: Chen, Jing D, dev; +Cc: He, Shaopeng

Tested-by: Michael Qiu <michael.qiu@intel.com
<mailto:michael.qiu@intel.com>>



On 5/29/2015 4:11 PM, Chen, Jing D wrote:
> From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
>
> In fm10k, PF driver needs to communicate with switch through
> mailbox if it needs to add/delete MAC address.
> This fix will validate if switch is ready before going forward.
> Then, it is necessary to acquire LPORT_MAP info after issuing
> MAC addr request to switch.
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c |   34 +++++++++++++++++++++++++++++++---
>  1 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index 19e718b..dedfbb4 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -45,6 +45,10 @@
>  #define FM10K_MBXLOCK_DELAY_US 20
>  #define UINT64_LOWER_32BITS_MASK 0x00000000ffffffffULL
>  
> +/* Max try times to aquire switch status */
> +#define MAX_QUERY_SWITCH_STATE_TIMES 10
> +/* Wait interval to get switch status */
> +#define WAIT_SWITCH_MSG_US    100000
>  /* Number of chars per uint32 type */
>  #define CHARS_PER_UINT32 (sizeof(uint32_t))
>  #define BIT_MASK_PER_UINT32 ((1 << CHARS_PER_UINT32) - 1)
> @@ -1802,6 +1806,32 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  		fm10k_dev_enable_intr_vf(dev);
>  	}
>  
> +	/* Enable uio intr after callback registered */
> +	rte_intr_enable(&(dev->pci_dev->intr_handle));
> +
> +	hw->mac.ops.update_int_moderator(hw);
> +
> +	/* Make sure Switch Manager is ready before going forward. */
> +	if (hw->mac.type == fm10k_mac_pf) {
> +		int switch_ready = 0;
> +		int i;
> +
> +		for (i = 0; i < MAX_QUERY_SWITCH_STATE_TIMES; i++) {
> +			fm10k_mbx_lock(hw);
> +			hw->mac.ops.get_host_state(hw, &switch_ready);
> +			fm10k_mbx_unlock(hw);
> +			if (switch_ready)
> +				break;
> +			/* Delay some time to acquire async LPORT_MAP info. */
> +			rte_delay_us(WAIT_SWITCH_MSG_US);
> +		}
> +
> +		if (switch_ready == 0) {
> +			PMD_INIT_LOG(ERR, "switch is not ready");
> +			return -1;
> +		}
> +	}
> +
>  	/*
>  	 * Below function will trigger operations on mailbox, acquire lock to
>  	 * avoid race condition from interrupt handler. Operations on mailbox
> @@ -1811,7 +1841,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  	 */
>  	fm10k_mbx_lock(hw);
>  	/* Enable port first */
> -	hw->mac.ops.update_lport_state(hw, 0, 0, 1);
> +	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
>  
>  	/* Update default vlan */
>  	hw->mac.ops.update_vlan(hw, hw->mac.default_vid, 0, true);
> @@ -1831,8 +1861,6 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  
>  	fm10k_mbx_unlock(hw);
>  
> -	/* enable uio intr after callback registered */
> -	rte_intr_enable(&(dev->pci_dev->intr_handle));
>  
>  	return 0;
>  }


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 1/6] fm10k: Fix improper RX buffer size assignment
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 1/6] fm10k: Fix improper RX buffer size assignment Chen Jing D(Mark)
@ 2015-06-16 11:55   ` Qiu, Michael
  0 siblings, 0 replies; 17+ messages in thread
From: Qiu, Michael @ 2015-06-16 11:55 UTC (permalink / raw)
  To: Chen, Jing D, dev; +Cc: He, Shaopeng

Tested-by: Michael Qiu <michael.qiu@intel.com>

- OS: Fedora20  3.11.10-301
- GCC: gcc version 4.8.3 2014911
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Ethernet controller: Intel Corporation Device 15a4 (rev 01)
- Default x86_64-native-linuxapp-gcc configuration


On 5/29/2015 4:11 PM, Chen, Jing D wrote:
> From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
>
> As RX buffer is aligned to 512B within mbuf, some bytes are reserved
> for this purpose, and the worst case could be 511B. But SRR reg
> assumes all buffers have the same size. In order to fill the gap,
> we'll have to consider the worsst case and assume 512B is reserved.
> If we don't do so, it's possible for HW to overwrite data to next
> mbuf.
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
>  drivers/net/fm10k/fm10k.h        |    5 +++--
>  drivers/net/fm10k/fm10k_ethdev.c |   12 ++++++++++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k.h b/drivers/net/fm10k/fm10k.h
> index 0e31796..ad7a7d1 100644
> --- a/drivers/net/fm10k/fm10k.h
> +++ b/drivers/net/fm10k/fm10k.h
> @@ -191,7 +191,8 @@ struct fm10k_tx_queue {
>  
>  /* enforce 512B alignment on default Rx DMA addresses */
>  #define MBUF_DMA_ADDR_DEFAULT(mb) \
> -	((uint64_t) RTE_ALIGN(((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM), 512))
> +	((uint64_t) RTE_ALIGN(((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM),\
> +			FM10K_RX_DATABUF_ALIGN))
>  
>  static inline void fifo_reset(struct fifo *fifo, uint32_t len)
>  {
> @@ -263,7 +264,7 @@ fm10k_addr_alignment_valid(struct rte_mbuf *mb)
>  	uint64_t boundary1, boundary2;
>  
>  	/* 512B aligned? */
> -	if (RTE_ALIGN(addr, 512) == addr)
> +	if (RTE_ALIGN(addr, FM10K_RX_DATABUF_ALIGN) == addr)
>  		return 1;
>  
>  	/* 8B aligned, and max Ethernet frame would not cross a 4KB boundary? */
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index 275c19c..a5e09a0 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -41,7 +41,6 @@
>  #include "fm10k.h"
>  #include "base/fm10k_api.h"
>  
> -#define FM10K_RX_BUFF_ALIGN 512
>  /* Default delay to acquire mailbox lock */
>  #define FM10K_MBXLOCK_DELAY_US 20
>  #define UINT64_LOWER_32BITS_MASK 0x00000000ffffffffULL
> @@ -426,6 +425,15 @@ fm10k_dev_rx_init(struct rte_eth_dev *dev)
>  		/* Configure the Rx buffer size for one buff without split */
>  		buf_size = (uint16_t)(rte_pktmbuf_data_room_size(rxq->mp) -
>  			RTE_PKTMBUF_HEADROOM);
> +		/* As RX buffer is aligned to 512B within mbuf, some bytes are
> +		 * reserved for this purpose, and the worst case could be 511B.
> +		 * But SRR reg assumes all buffers have the same size. In order
> +		 * to fill the gap, we'll have to consider the worst case and
> +		 * assume 512B is reserved. If we don't do so, it's possible
> +		 * for HW to overwrite data to next mbuf.
> +		 */
> +		buf_size -= FM10K_RX_DATABUF_ALIGN;
> +
>  		FM10K_WRITE_REG(hw, FM10K_SRRCTL(i),
>  				buf_size >> FM10K_SRRCTL_BSIZEPKT_SHIFT);
>  
> @@ -909,7 +917,7 @@ mempool_element_size_valid(struct rte_mempool *mp)
>  			RTE_PKTMBUF_HEADROOM;
>  
>  	/* account for up to 512B of alignment */
> -	min_size -= FM10K_RX_BUFF_ALIGN;
> +	min_size -= FM10K_RX_DATABUF_ALIGN;
>  
>  	/* sanity check for overflow */
>  	if (min_size > mp->elt_size)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 3/6] fm10k: Fix data integrity issue with multi-segment frame
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 3/6] fm10k: Fix data integrity issue with multi-segment frame Chen Jing D(Mark)
@ 2015-06-16 12:07   ` Qiu, Michael
  0 siblings, 0 replies; 17+ messages in thread
From: Qiu, Michael @ 2015-06-16 12:07 UTC (permalink / raw)
  To: Chen, Jing D, dev; +Cc: He, Shaopeng

Tested-by: Michael Qiu <michael.qiu@intel.com>

- OS: Fedora20  3.11.10-301
- GCC: gcc version 4.8.3 2014911
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Ethernet controller: Intel Corporation Device 15a4 (rev 01)
- Default x86_64-native-linuxapp-gcc configuration

- Total 5 cases, 5 passed, 0 failed

- Case: Normal frames with no jumbo frame support
- Case: Jumbo frames with no jumbo frame support
- Case: Normal frames with jumbo frame support
- Case: Jumbo frames with jumbo frame support
- Case: Frames bigger than jumbo frames, wwith jumbo frame support


On 5/29/2015 4:11 PM, Chen, Jing D wrote:
> From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
>
> In TX side, bit FM10K_TXD_FLAG_LAST in TX descriptor only is set
> in the last descriptor for multi-segment packets. But current
> implementation didn't set all the fields of TX descriptor, which
> will cause descriptors processed now to re-use fields set in last
> scroll. If FM10K_TXD_FLAG_LAST bit was set in the last round and
> it happened this is not the last descriptor of a multi-segnment
> packet, HW will send out the incomplete packet out and leads to
> data intergrity issue.
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
>  drivers/net/fm10k/fm10k_rxtx.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
> index 56df6cd..f5d1ad0 100644
> --- a/drivers/net/fm10k/fm10k_rxtx.c
> +++ b/drivers/net/fm10k/fm10k_rxtx.c
> @@ -402,9 +402,9 @@ static inline void tx_xmit_pkt(struct fm10k_tx_queue *q, struct rte_mbuf *mb)
>  		q->nb_used = q->nb_used + mb->nb_segs;
>  	}
>  
> -	q->hw_ring[last_id].flags = flags;
>  	q->nb_free -= mb->nb_segs;
>  
> +	q->hw_ring[q->next_free].flags = 0;
>  	/* set checksum flags on first descriptor of packet. SCTP checksum
>  	 * offload is not supported, but we do not explicitly check for this
>  	 * case in favor of greatly simplified processing. */
> @@ -415,16 +415,27 @@ static inline void tx_xmit_pkt(struct fm10k_tx_queue *q, struct rte_mbuf *mb)
>  	if (mb->ol_flags & PKT_TX_VLAN_PKT)
>  		q->hw_ring[q->next_free].vlan = mb->vlan_tci;
>  
> +	q->sw_ring[q->next_free] = mb;
> +	q->hw_ring[q->next_free].buffer_addr =
> +			rte_cpu_to_le_64(MBUF_DMA_ADDR(mb));
> +	q->hw_ring[q->next_free].buflen =
> +			rte_cpu_to_le_16(rte_pktmbuf_data_len(mb));
> +	if (++q->next_free == q->nb_desc)
> +		q->next_free = 0;
> +
>  	/* fill up the rings */
> -	for (; mb != NULL; mb = mb->next) {
> +	for (mb = mb->next; mb != NULL; mb = mb->next) {
>  		q->sw_ring[q->next_free] = mb;
>  		q->hw_ring[q->next_free].buffer_addr =
>  				rte_cpu_to_le_64(MBUF_DMA_ADDR(mb));
>  		q->hw_ring[q->next_free].buflen =
>  				rte_cpu_to_le_16(rte_pktmbuf_data_len(mb));
> +		q->hw_ring[q->next_free].flags = 0;
>  		if (++q->next_free == q->nb_desc)
>  			q->next_free = 0;
>  	}
> +
> +	q->hw_ring[last_id].flags = flags;
>  }
>  
>  uint16_t


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 5/6] fm10k: Do sanity check on mac address
  2015-05-29  8:10 ` [dpdk-dev] [PATCH 5/6] fm10k: Do sanity check on mac address Chen Jing D(Mark)
@ 2015-06-16 12:08   ` Qiu, Michael
  0 siblings, 0 replies; 17+ messages in thread
From: Qiu, Michael @ 2015-06-16 12:08 UTC (permalink / raw)
  To: Chen, Jing D, dev; +Cc: He, Shaopeng

Tested-by: Michael Qiu <michael.qiu@intel.com>

- OS: Fedora20  3.11.10-301
- GCC: gcc version 4.8.3 2014911
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Ethernet controller: Intel Corporation Device 15a4 (rev 01)
- Default x86_64-native-linuxapp-gcc configuration


On 5/29/2015 4:11 PM, Chen, Jing D wrote:
> From: "Chen Jing D(Mark)" <jing.d.chen@intel.com>
>
> After acquiring MAC address from HW, it's necessary to validate
> MAC address before use.
>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c |   24 ++++++++++--------------
>  1 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index dedfbb4..b6e82e3 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -1756,24 +1756,20 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  	}
>  
>  	diag = fm10k_read_mac_addr(hw);
> -	if (diag != FM10K_SUCCESS) {
> -		/*
> -		 * TODO: remove special handling on VF. Need shared code to
> -		 * fix first.
> -		 */
> -		if (hw->mac.type == fm10k_mac_pf) {
> -			PMD_INIT_LOG(ERR, "Read MAC addr failed: %d", diag);
> -			return -EIO;
> -		} else {
> -			/* Generate a random addr */
> -			eth_random_addr(hw->mac.addr);
> -			memcpy(hw->mac.perm_addr, hw->mac.addr, ETH_ALEN);
> -		}
> -	}
>  
>  	ether_addr_copy((const struct ether_addr *)hw->mac.addr,
>  			&dev->data->mac_addrs[0]);
>  
> +	if (diag != FM10K_SUCCESS ||
> +		!is_valid_assigned_ether_addr(dev->data->mac_addrs)) {
> +
> +		/* Generate a random addr */
> +		eth_random_addr(hw->mac.addr);
> +		memcpy(hw->mac.perm_addr, hw->mac.addr, ETH_ALEN);
> +		ether_addr_copy((const struct ether_addr *)hw->mac.addr,
> +		&dev->data->mac_addrs[0]);
> +	}
> +
>  	/* Reset the hw statistics */
>  	fm10k_stats_reset(dev);
>  


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes
  2015-06-09 15:21 ` Qiu, Michael
@ 2015-06-22 13:55   ` Thomas Monjalon
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Monjalon @ 2015-06-22 13:55 UTC (permalink / raw)
  To: Chen, Jing D; +Cc: dev, He, Shaopeng

> > This patch set include a few bug fixes and enhancements on fm10k driver.
> >
> > Chen Jing D(Mark) (6):
> >   fm10k: Fix improper RX buffer size assignment
> >   fm10k: Fix jumbo frame issue
> >   fm10k: Fix data integrity issue with multi-segment frame
> >   fm10k: Fix issue that MAC addr can't be set to silicon
> >   fm10k: Do sanity check on mac address
> >   fm10k: Add default mac/vlan filter to SM
> 
> Acked-by: Michael Qiu <michael.qiu@intel.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-06-22 13:56 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-29  8:10 [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes Chen Jing D(Mark)
2015-05-29  8:10 ` [dpdk-dev] [PATCH 1/6] fm10k: Fix improper RX buffer size assignment Chen Jing D(Mark)
2015-06-16 11:55   ` Qiu, Michael
2015-05-29  8:10 ` [dpdk-dev] [PATCH 2/6] fm10k: Fix jumbo frame issue Chen Jing D(Mark)
2015-06-12  1:15   ` Qiu, Michael
2015-05-29  8:10 ` [dpdk-dev] [PATCH 3/6] fm10k: Fix data integrity issue with multi-segment frame Chen Jing D(Mark)
2015-06-16 12:07   ` Qiu, Michael
2015-05-29  8:10 ` [dpdk-dev] [PATCH 4/6] fm10k: Fix issue that MAC addr can't be set to silicon Chen Jing D(Mark)
2015-06-09 16:23   ` Qiu, Michael
2015-06-10  5:34     ` Chen, Jing D
2015-06-12  2:03   ` Qiu, Michael
2015-05-29  8:10 ` [dpdk-dev] [PATCH 5/6] fm10k: Do sanity check on mac address Chen Jing D(Mark)
2015-06-16 12:08   ` Qiu, Michael
2015-05-29  8:10 ` [dpdk-dev] [PATCH 6/6] fm10k: Add default mac/vlan filter to SM Chen Jing D(Mark)
2015-06-08  2:39 ` [dpdk-dev] [PATCH 0/6] fm10k: A series of bug fixes He, Shaopeng
2015-06-09 15:21 ` Qiu, Michael
2015-06-22 13:55   ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).