DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] net/i40e: set no drop for traffic class
@ 2016-12-04 13:54 Rory Sexton
  2016-12-05  8:44 ` Wu, Jingjing
  0 siblings, 1 reply; 9+ messages in thread
From: Rory Sexton @ 2016-12-04 13:54 UTC (permalink / raw)
  To: jingjing.wu; +Cc: dev, nemanja.marjanovic, John McNamara, Rory Sexton

From: John McNamara <john.mcnamara@intel.com>

The default traffic class in i40e is set to drop versus on ixgbe
it isset to no drop. This means when packets build up in the RX
SRAM on the NIC, they are dropped, and they do this when the SW
descriptor rings fill up.

This patch changes this behaviour and our testing shows there
are no drops as a result.

Signed-off-by: Rory Sexton <rory.sexton@intel.com>
Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c |  1 +
 drivers/net/i40e/i40e_rxtx.c   | 12 ++++++++++++
 drivers/net/i40e/i40e_rxtx.h   |  1 +
 lib/librte_ether/rte_ethdev.h  | 24 ++++++++++++++++++++++++
 4 files changed, 38 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..9702acb 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -553,6 +553,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.get_eeprom                   = i40e_get_eeprom,
 	.mac_addr_set                 = i40e_set_default_mac_addr,
 	.mtu_set                      = i40e_dev_mtu_set,
+	.set_no_drop		      = i40e_set_no_drop,
 };
 
 /* store statistics names and its offset in stats structure */
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 7ae7d9f..02aeff4 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -783,6 +783,18 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	return nb_rx;
 }
 
+uint32_t
+i40e_set_no_drop(struct rte_eth_dev *dev, uint16_t rx_queue_id)
+{
+	struct i40e_rx_queue *rxq = dev->data->rx_queues[rx_queue_id];
+	struct i40e_hw *hw = I40E_VSI_TO_HW(rxq->vsi);
+
+	/* Set No Drop Traffic Class. */
+	I40E_WRITE_REG(hw, 0x1c0980, 0xff);
+
+	return 1;
+}
+
 uint16_t
 i40e_recv_scattered_pkts(void *rx_queue,
 			 struct rte_mbuf **rx_pkts,
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index ecdb13c..47720e5 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -196,6 +196,7 @@ union i40e_tx_offload {
 	};
 };
 
+uint32_t i40e_set_no_drop(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int i40e_dev_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int i40e_dev_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id);
 int i40e_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..a862101 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1113,6 +1113,10 @@ typedef int (*eth_queue_start_t)(struct rte_eth_dev *dev,
 				    uint16_t queue_id);
 /**< @internal Start rx and tx of a queue of an Ethernet device. */
 
+typedef uint32_t (*set_no_drop)(struct rte_eth_dev *dev,
+					uint16_t queue_id);
+/**< @internal Ethernet device configuration. */
+
 typedef int (*eth_queue_stop_t)(struct rte_eth_dev *dev,
 				    uint16_t queue_id);
 /**< @internal Stop rx and tx of a queue of an Ethernet device. */
@@ -1547,6 +1551,8 @@ struct eth_dev_ops {
 	eth_l2_tunnel_eth_type_conf_t l2_tunnel_eth_type_conf;
 	/** Enable/disable l2 tunnel offload functions */
 	eth_l2_tunnel_offload_set_t l2_tunnel_offload_set;
+	/** Read NIC SRAM .*/
+	set_no_drop set_no_drop;
 };
 
 /**
@@ -2730,6 +2736,24 @@ rte_eth_rx_descriptor_done(uint8_t port_id, uint16_t queue_id, uint16_t offset)
 }
 
 /**
+ * Sets port to no drop.
+ *
+ * @param port_id
+ *  The port identifier of the Ethernet device.
+ *
+ * @return
+ *  Nic occupancy in kilobytes.
+ */
+static inline uint32_t
+set_no_drop_tc(uint8_t port_id)
+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	uint32_t limit = (*dev->dev_ops->set_no_drop)(dev, 0);
+
+	return limit;
+}
+
+/**
  * Send a burst of output packets on a transmit queue of an Ethernet device.
  *
  * The rte_eth_tx_burst() function is invoked to transmit output packets
-- 
2.7.4

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

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

* Re: [dpdk-dev] [PATCH v1] net/i40e: set no drop for traffic class
  2016-12-04 13:54 [dpdk-dev] [PATCH v1] net/i40e: set no drop for traffic class Rory Sexton
@ 2016-12-05  8:44 ` Wu, Jingjing
  2016-12-09 14:02   ` Sexton, Rory
  0 siblings, 1 reply; 9+ messages in thread
From: Wu, Jingjing @ 2016-12-05  8:44 UTC (permalink / raw)
  To: Sexton, Rory; +Cc: dev, Marjanovic, Nemanja, Mcnamara, John



-----Original Message-----
From: Sexton, Rory 
Sent: Sunday, December 4, 2016 9:55 PM
To: Wu, Jingjing <jingjing.wu@intel.com>
Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>; Mcnamara, John <john.mcnamara@intel.com>; Sexton, Rory <rory.sexton@intel.com>
Subject: [PATCH v1] net/i40e: set no drop for traffic class

From: John McNamara <john.mcnamara@intel.com>

The default traffic class in i40e is set to drop versus on ixgbe it isset to no drop. This means when packets build up in the RX SRAM on the NIC, they are dropped, and they do this when the SW descriptor rings fill up.

This patch changes this behaviour and our testing shows there are no drops as a result.

Signed-off-by: Rory Sexton <rory.sexton@intel.com>
Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c |  1 +
 drivers/net/i40e/i40e_rxtx.c   | 12 ++++++++++++
 drivers/net/i40e/i40e_rxtx.h   |  1 +
 lib/librte_ether/rte_ethdev.h  | 24 ++++++++++++++++++++++++
 4 files changed, 38 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 67778ba..9702acb 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -553,6 +553,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.get_eeprom                   = i40e_get_eeprom,
 	.mac_addr_set                 = i40e_set_default_mac_addr,
 	.mtu_set                      = i40e_dev_mtu_set,
+	.set_no_drop		      = i40e_set_no_drop,
 };
 
 /* store statistics names and its offset in stats structure */ diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index 7ae7d9f..02aeff4 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -783,6 +783,18 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	return nb_rx;
 }
 
+uint32_t
+i40e_set_no_drop(struct rte_eth_dev *dev, uint16_t rx_queue_id) {
+	struct i40e_rx_queue *rxq = dev->data->rx_queues[rx_queue_id];
+	struct i40e_hw *hw = I40E_VSI_TO_HW(rxq->vsi);
+
+	/* Set No Drop Traffic Class. */
+	I40E_WRITE_REG(hw, 0x1c0980, 0xff);
+
+	return 1;
+}

0x1c0980 is the register (PRTDCB_TC2PFC) which is used to control pfc for each TC.
We already have ETH_DCB_PFC_SUPPORT flag in rte_eth_conf.dcb_capability_en to
Control if PFC is enabled. And rte_eth_dcb_rx_conf.nb_tcs identified number of TCs
It supports.
"I40E_WRITE_REG(hw, 0x1c0980, 0xff);" can be achieved by enabling DCB and PFC
For all TCs.


Why do we need such a new API?


Thanks
Jingjing

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

* Re: [dpdk-dev] [PATCH v1] net/i40e: set no drop for traffic class
  2016-12-05  8:44 ` Wu, Jingjing
@ 2016-12-09 14:02   ` Sexton, Rory
  2016-12-26  8:45     ` Wu, Jingjing
  0 siblings, 1 reply; 9+ messages in thread
From: Sexton, Rory @ 2016-12-09 14:02 UTC (permalink / raw)
  To: Wu, Jingjing; +Cc: dev, Marjanovic, Nemanja, Mcnamara, John

Hi Jingjing,

Yes PRTDCB_TC2PFC is used to control pfc for each TC however we have noticed other advantages of using the register.
By setting the register explicitly by doing the "I40E_WRITE_REG(hw, 0x1c0980, 0xff);" it allows for packets to be temporarily stored on the NICs RX SRAM until there is space for them on SW descriptor ring versus dropping them when the SW ring becomes full. This also allows for larger burst handling. It also means SW doesn't have to be as quick to empty the DRAM based descriptor rings, allowing more processing on cores. 

I have tested using the ETH_DCB_PFC_SUPPORT flag in rte_eth_conf.dcb_capability_en and rte_eth_dcb_rx_conf.nb_tcs. 
This results in the NIC's RX SRAM not being used and if there is no space on SW descriptor ring for packet it is dropped. 
The advantages of using the PRTDCB_TC2PFC explicitly is that there will be no packet loss and descriptor rings do not need to be modified (can be left at 128 for rx and 512 for tx as default settings for apps).  Enabling via this register allows Burst handling to be within the NIC Rx Buffer and SW rings combined. 
At the moment for tests the rx and tx descriptor rings have to be increased to 2048 to eliminate packet loss.

Ideally it would be an optional setting as using it may increase the max latency.


Regards,
Rory/Nemanja

-----Original Message-----
From: Wu, Jingjing 
Sent: Monday, December 5, 2016 8:45 AM
To: Sexton, Rory <rory.sexton@intel.com>
Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>; Mcnamara, John <john.mcnamara@intel.com>
Subject: RE: [PATCH v1] net/i40e: set no drop for traffic class



-----Original Message-----
From: Sexton, Rory
Sent: Sunday, December 4, 2016 9:55 PM
To: Wu, Jingjing <jingjing.wu@intel.com>
Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>; Mcnamara, John <john.mcnamara@intel.com>; Sexton, Rory <rory.sexton@intel.com>
Subject: [PATCH v1] net/i40e: set no drop for traffic class

From: John McNamara <john.mcnamara@intel.com>

The default traffic class in i40e is set to drop versus on ixgbe it isset to no drop. This means when packets build up in the RX SRAM on the NIC, they are dropped, and they do this when the SW descriptor rings fill up.

This patch changes this behaviour and our testing shows there are no drops as a result.

Signed-off-by: Rory Sexton <rory.sexton@intel.com>
Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c |  1 +
 drivers/net/i40e/i40e_rxtx.c   | 12 ++++++++++++
 drivers/net/i40e/i40e_rxtx.h   |  1 +
 lib/librte_ether/rte_ethdev.h  | 24 ++++++++++++++++++++++++
 4 files changed, 38 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 67778ba..9702acb 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -553,6 +553,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.get_eeprom                   = i40e_get_eeprom,
 	.mac_addr_set                 = i40e_set_default_mac_addr,
 	.mtu_set                      = i40e_dev_mtu_set,
+	.set_no_drop		      = i40e_set_no_drop,
 };
 
 /* store statistics names and its offset in stats structure */ diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index 7ae7d9f..02aeff4 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -783,6 +783,18 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	return nb_rx;
 }
 
+uint32_t
+i40e_set_no_drop(struct rte_eth_dev *dev, uint16_t rx_queue_id) {
+	struct i40e_rx_queue *rxq = dev->data->rx_queues[rx_queue_id];
+	struct i40e_hw *hw = I40E_VSI_TO_HW(rxq->vsi);
+
+	/* Set No Drop Traffic Class. */
+	I40E_WRITE_REG(hw, 0x1c0980, 0xff);
+
+	return 1;
+}

0x1c0980 is the register (PRTDCB_TC2PFC) which is used to control pfc for each TC.
We already have ETH_DCB_PFC_SUPPORT flag in rte_eth_conf.dcb_capability_en to Control if PFC is enabled. And rte_eth_dcb_rx_conf.nb_tcs identified number of TCs It supports.
"I40E_WRITE_REG(hw, 0x1c0980, 0xff);" can be achieved by enabling DCB and PFC For all TCs.


Why do we need such a new API?


Thanks
Jingjing

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

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

* Re: [dpdk-dev] [PATCH v1] net/i40e: set no drop for traffic class
  2016-12-09 14:02   ` Sexton, Rory
@ 2016-12-26  8:45     ` Wu, Jingjing
  2017-01-16 15:52       ` [dpdk-dev] [PATCH v2] " rory.sexton
  0 siblings, 1 reply; 9+ messages in thread
From: Wu, Jingjing @ 2016-12-26  8:45 UTC (permalink / raw)
  To: Sexton, Rory; +Cc: dev, Marjanovic, Nemanja, Mcnamara, John



> -----Original Message-----
> From: Sexton, Rory
> Sent: Friday, December 9, 2016 10:03 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>;
> Mcnamara, John <john.mcnamara@intel.com>
> Subject: RE: [PATCH v1] net/i40e: set no drop for traffic class
> 
> Hi Jingjing,
> 
> Yes PRTDCB_TC2PFC is used to control pfc for each TC however we have
> noticed other advantages of using the register.
> By setting the register explicitly by doing the "I40E_WRITE_REG(hw, 0x1c0980,
> 0xff);" it allows for packets to be temporarily stored on the NICs RX SRAM
> until there is space for them on SW descriptor ring versus dropping them
> when the SW ring becomes full. This also allows for larger burst handling. It
> also means SW doesn't have to be as quick to empty the DRAM based
> descriptor rings, allowing more processing on cores.
> 
> I have tested using the ETH_DCB_PFC_SUPPORT flag in
> rte_eth_conf.dcb_capability_en and rte_eth_dcb_rx_conf.nb_tcs.
> This results in the NIC's RX SRAM not being used and if there is no space on
> SW descriptor ring for packet it is dropped.

Besides ETH_DCB_PFC_SUPPORT, ETH_MQ_RX_DCB_FLAG is also required in
dev_conf.rxmode.mq_mode. After doing that, you will find register PRTDCB_TC2PFC
is also changed.
 
If you don't want to enable DCB, why not just implement that function "i40e_priority_flow_ctrl_set"?
You can change the register in this function without define a new API.


> The advantages of using the PRTDCB_TC2PFC explicitly is that there will be no
> packet loss and descriptor rings do not need to be modified (can be left at
> 128 for rx and 512 for tx as default settings for apps).  Enabling via this
> register allows Burst handling to be within the NIC Rx Buffer and SW rings
> combined.
> At the moment for tests the rx and tx descriptor rings have to be increased
> to 2048 to eliminate packet loss.
> 
> Ideally it would be an optional setting as using it may increase the max
> latency.
> 


Thanks for the clarification.

Thanks
Jingjing

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

* [dpdk-dev] [PATCH v2] net/i40e: set no drop for traffic class
  2016-12-26  8:45     ` Wu, Jingjing
@ 2017-01-16 15:52       ` rory.sexton
  2017-01-17 15:09         ` Wu, Jingjing
  0 siblings, 1 reply; 9+ messages in thread
From: rory.sexton @ 2017-01-16 15:52 UTC (permalink / raw)
  To: jingjing.wu; +Cc: dev, Rory Sexton, Nemanja Marjanovic

From: Rory Sexton <rory.sexton@intel.com>

The default traffic class in i40e is set to drop versus on ixgbe
it isset to no drop. This means when packets build up in the RX
SRAM on the NIC, they are dropped, and they do this when the SW
descriptor rings fill up.

This patch changes this behaviour and our testing shows there
are no drops as a result.

Signed-off-by: Rory Sexton <rory.sexton@intel.com>
Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic@intel.com>
---
v2:
* Changed to use existing api to set priority register directly.

 drivers/net/i40e/i40e_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..97339b5 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2985,8 +2985,11 @@ static int
 i40e_priority_flow_ctrl_set(__rte_unused struct rte_eth_dev *dev,
 			    __rte_unused struct rte_eth_pfc_conf *pfc_conf)
 {
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
 	PMD_INIT_FUNC_TRACE();
 
+	I40E_WRITE_REG(hw, I40E_PRTDCB_TC2PFC, 0xff);
 	return -ENOSYS;
 }
 
-- 
2.4.3

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

* Re: [dpdk-dev] [PATCH v2] net/i40e: set no drop for traffic class
  2017-01-16 15:52       ` [dpdk-dev] [PATCH v2] " rory.sexton
@ 2017-01-17 15:09         ` Wu, Jingjing
  2017-01-19 10:38           ` Sexton, Rory
  0 siblings, 1 reply; 9+ messages in thread
From: Wu, Jingjing @ 2017-01-17 15:09 UTC (permalink / raw)
  To: Sexton, Rory; +Cc: dev, Marjanovic, Nemanja



> -----Original Message-----
> From: Sexton, Rory
> Sent: Monday, January 16, 2017 11:52 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Sexton, Rory <rory.sexton@intel.com>; Marjanovic,
> Nemanja <nemanja.marjanovic@intel.com>
> Subject: [PATCH v2] net/i40e: set no drop for traffic class
> 
> From: Rory Sexton <rory.sexton@intel.com>
> 
> The default traffic class in i40e is set to drop versus on ixgbe it isset to no
> drop. This means when packets build up in the RX SRAM on the NIC, they are
> dropped, and they do this when the SW descriptor rings fill up.
> 
> This patch changes this behaviour and our testing shows there are no drops
> as a result.
> 
> Signed-off-by: Rory Sexton <rory.sexton@intel.com>
> Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic@intel.com>
> ---
> v2:
> * Changed to use existing api to set priority register directly.
> 
>  drivers/net/i40e/i40e_ethdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 67778ba..97339b5 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2985,8 +2985,11 @@ static int
>  i40e_priority_flow_ctrl_set(__rte_unused struct rte_eth_dev *dev,
>  			    __rte_unused struct rte_eth_pfc_conf *pfc_conf)
> {
> +	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
>  	PMD_INIT_FUNC_TRACE();
> 
> +	I40E_WRITE_REG(hw, I40E_PRTDCB_TC2PFC, 0xff);

PRTDCB_TC2PFC  is the Bitmap who controls the use of Priority Flow Control (PFC) per each
TC. Bit n set to 1b indicates TC n uses PFC in Rx and Tx. The TC is referred as a no-drop TC.

And if look the rte_eth_pfc_conf, there is a field called priority, which would map to a TC.
Currently, the TC and priority is 1:1 map when dcb is enabled.
So how about change it like:
Check dcb info, and map the priority to tc, then
val = 0x1 << tc;
I40E_WRITE_REG(hw, I40E_PRTDCB_TC2PFC, val);

Thanks
Jingjing



>  	return -ENOSYS;
>  }
> 
> --
> 2.4.3

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

* Re: [dpdk-dev] [PATCH v2] net/i40e: set no drop for traffic class
  2017-01-17 15:09         ` Wu, Jingjing
@ 2017-01-19 10:38           ` Sexton, Rory
  2017-02-07 15:25             ` Wu, Jingjing
  0 siblings, 1 reply; 9+ messages in thread
From: Sexton, Rory @ 2017-01-19 10:38 UTC (permalink / raw)
  To: Wu, Jingjing; +Cc: dev, Marjanovic, Nemanja

Perhaps the best solution is as suggested to set
rte_eth_conf.dcb_capability_en = ETH_DCB_PFC_SUPPORT
rte_eth_conf.rxmode.mq_mode = ETH_MQ_RX_DCB_FLAG
and set rte_eth_dcb_rx_conf.nb_tcs to the number of tc's to apply
Using this port configuration will give the same behavior of the patch and it removes the need for an API change.

Rory

-----Original Message-----
From: Wu, Jingjing 
Sent: Tuesday, January 17, 2017 3:09 PM
To: Sexton, Rory <rory.sexton@intel.com>
Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>
Subject: RE: [PATCH v2] net/i40e: set no drop for traffic class



> -----Original Message-----
> From: Sexton, Rory
> Sent: Monday, January 16, 2017 11:52 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Sexton, Rory <rory.sexton@intel.com>; Marjanovic, 
> Nemanja <nemanja.marjanovic@intel.com>
> Subject: [PATCH v2] net/i40e: set no drop for traffic class
> 
> From: Rory Sexton <rory.sexton@intel.com>
> 
> The default traffic class in i40e is set to drop versus on ixgbe it 
> isset to no drop. This means when packets build up in the RX SRAM on 
> the NIC, they are dropped, and they do this when the SW descriptor rings fill up.
> 
> This patch changes this behaviour and our testing shows there are no 
> drops as a result.
> 
> Signed-off-by: Rory Sexton <rory.sexton@intel.com>
> Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic@intel.com>
> ---
> v2:
> * Changed to use existing api to set priority register directly.
> 
>  drivers/net/i40e/i40e_ethdev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c 
> b/drivers/net/i40e/i40e_ethdev.c index 67778ba..97339b5 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2985,8 +2985,11 @@ static int
>  i40e_priority_flow_ctrl_set(__rte_unused struct rte_eth_dev *dev,
>  			    __rte_unused struct rte_eth_pfc_conf *pfc_conf) {
> +	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
>  	PMD_INIT_FUNC_TRACE();
> 
> +	I40E_WRITE_REG(hw, I40E_PRTDCB_TC2PFC, 0xff);

PRTDCB_TC2PFC  is the Bitmap who controls the use of Priority Flow Control (PFC) per each TC. Bit n set to 1b indicates TC n uses PFC in Rx and Tx. The TC is referred as a no-drop TC.

And if look the rte_eth_pfc_conf, there is a field called priority, which would map to a TC.
Currently, the TC and priority is 1:1 map when dcb is enabled.
So how about change it like:
Check dcb info, and map the priority to tc, then val = 0x1 << tc; I40E_WRITE_REG(hw, I40E_PRTDCB_TC2PFC, val);

Thanks
Jingjing



>  	return -ENOSYS;
>  }
> 
> --
> 2.4.3

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

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

* Re: [dpdk-dev] [PATCH v2] net/i40e: set no drop for traffic class
  2017-01-19 10:38           ` Sexton, Rory
@ 2017-02-07 15:25             ` Wu, Jingjing
  2017-02-09 15:34               ` Ferruh Yigit
  0 siblings, 1 reply; 9+ messages in thread
From: Wu, Jingjing @ 2017-02-07 15:25 UTC (permalink / raw)
  To: Sexton, Rory; +Cc: dev, Marjanovic, Nemanja



> -----Original Message-----
> From: Sexton, Rory
> Sent: Thursday, January 19, 2017 6:38 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>
> Subject: RE: [PATCH v2] net/i40e: set no drop for traffic class
> 
> Perhaps the best solution is as suggested to set rte_eth_conf.dcb_capability_en
> = ETH_DCB_PFC_SUPPORT rte_eth_conf.rxmode.mq_mode =
> ETH_MQ_RX_DCB_FLAG and set rte_eth_dcb_rx_conf.nb_tcs to the number of
> tc's to apply Using this port configuration will give the same behavior of the
> patch and it removes the need for an API change.
> 
> Rory
> 
Yes, That's what I thought when the v1 patch. So do we still need this patch now?

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Tuesday, January 17, 2017 3:09 PM
> To: Sexton, Rory <rory.sexton@intel.com>
> Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>
> Subject: RE: [PATCH v2] net/i40e: set no drop for traffic class
> 
> 
> 
> > -----Original Message-----
> > From: Sexton, Rory
> > Sent: Monday, January 16, 2017 11:52 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; Sexton, Rory <rory.sexton@intel.com>; Marjanovic,
> > Nemanja <nemanja.marjanovic@intel.com>
> > Subject: [PATCH v2] net/i40e: set no drop for traffic class
> >
> > From: Rory Sexton <rory.sexton@intel.com>
> >
> > The default traffic class in i40e is set to drop versus on ixgbe it
> > isset to no drop. This means when packets build up in the RX SRAM on
> > the NIC, they are dropped, and they do this when the SW descriptor rings fill up.
> >
> > This patch changes this behaviour and our testing shows there are no
> > drops as a result.
> >
> > Signed-off-by: Rory Sexton <rory.sexton@intel.com>
> > Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic@intel.com>
> > ---
> > v2:
> > * Changed to use existing api to set priority register directly.
> >
> >  drivers/net/i40e/i40e_ethdev.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 67778ba..97339b5 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2985,8 +2985,11 @@ static int
> >  i40e_priority_flow_ctrl_set(__rte_unused struct rte_eth_dev *dev,
> >  			    __rte_unused struct rte_eth_pfc_conf *pfc_conf) {
> > +	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > +
> >  	PMD_INIT_FUNC_TRACE();
> >
> > +	I40E_WRITE_REG(hw, I40E_PRTDCB_TC2PFC, 0xff);
> 
> PRTDCB_TC2PFC  is the Bitmap who controls the use of Priority Flow Control
> (PFC) per each TC. Bit n set to 1b indicates TC n uses PFC in Rx and Tx. The TC is
> referred as a no-drop TC.
> 
> And if look the rte_eth_pfc_conf, there is a field called priority, which would
> map to a TC.
> Currently, the TC and priority is 1:1 map when dcb is enabled.
> So how about change it like:
> Check dcb info, and map the priority to tc, then val = 0x1 << tc;
> I40E_WRITE_REG(hw, I40E_PRTDCB_TC2PFC, val);
> 
> Thanks
> Jingjing
> 
> 
> 
> >  	return -ENOSYS;
> >  }
> >
> > --
> > 2.4.3

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

* Re: [dpdk-dev] [PATCH v2] net/i40e: set no drop for traffic class
  2017-02-07 15:25             ` Wu, Jingjing
@ 2017-02-09 15:34               ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2017-02-09 15:34 UTC (permalink / raw)
  To: Wu, Jingjing, Sexton, Rory; +Cc: dev, Marjanovic, Nemanja

On 2/7/2017 3:25 PM, Wu, Jingjing wrote:
> 
> 
>> -----Original Message-----
>> From: Sexton, Rory
>> Sent: Thursday, January 19, 2017 6:38 PM
>> To: Wu, Jingjing <jingjing.wu@intel.com>
>> Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>
>> Subject: RE: [PATCH v2] net/i40e: set no drop for traffic class
>>
>> Perhaps the best solution is as suggested to set rte_eth_conf.dcb_capability_en
>> = ETH_DCB_PFC_SUPPORT rte_eth_conf.rxmode.mq_mode =
>> ETH_MQ_RX_DCB_FLAG and set rte_eth_dcb_rx_conf.nb_tcs to the number of
>> tc's to apply Using this port configuration will give the same behavior of the
>> patch and it removes the need for an API change.
>>
>> Rory
>>
> Yes, That's what I thought when the v1 patch. So do we still need this patch now?

I guess answer is No.
The patch is marked as "Rejected" in patchwork, please shout if that is
not the case.

Thanks,
ferruh

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

end of thread, other threads:[~2017-02-09 15:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-04 13:54 [dpdk-dev] [PATCH v1] net/i40e: set no drop for traffic class Rory Sexton
2016-12-05  8:44 ` Wu, Jingjing
2016-12-09 14:02   ` Sexton, Rory
2016-12-26  8:45     ` Wu, Jingjing
2017-01-16 15:52       ` [dpdk-dev] [PATCH v2] " rory.sexton
2017-01-17 15:09         ` Wu, Jingjing
2017-01-19 10:38           ` Sexton, Rory
2017-02-07 15:25             ` Wu, Jingjing
2017-02-09 15:34               ` Ferruh Yigit

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).