DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers
@ 2014-06-13 13:37 David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 1/7] ethdev: retrieve flow control configuration David Marchand
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: David Marchand @ 2014-06-13 13:37 UTC (permalink / raw)
  To: dev

This patchset introduces 3 new ethdev operations: flow control parameters
retrieval and mtu get/set operations.

Changes since v1:
- compute min rx buffer size at ethdev level (to simplify pmd mtu checks)
- introduce enable_scatter rx mode so that we can advise pmd to configure
  scatter mode
- rework mtu get/set operations (based on Konstantin comments)
- pass checkpatch.pl checks


-- 
David Marchand

David Marchand (3):
  ethdev: add autoneg parameter in flow ctrl accessors
  ethdev: store min rx buffer size
  ethdev: introduce enable_scatter rx mode

Ivan Boule (2):
  ixgbe: add set_mtu to ixgbevf
  app/testpmd: allow to configure mtu

Samuel Gauthier (1):
  ethdev: add mtu accessors

Zijie Pan (1):
  ethdev: retrieve flow control configuration

 app/test-pmd/cmdline.c              |   54 +++++++++++++
 app/test-pmd/config.c               |   13 ++++
 app/test-pmd/testpmd.h              |    2 +-
 lib/librte_ether/rte_ethdev.c       |   76 ++++++++++++++++--
 lib/librte_ether/rte_ethdev.h       |   65 +++++++++++++++-
 lib/librte_pmd_e1000/em_ethdev.c    |   89 ++++++++++++++++++++++
 lib/librte_pmd_e1000/em_rxtx.c      |    5 ++
 lib/librte_pmd_e1000/igb_ethdev.c   |  100 ++++++++++++++++++++++++
 lib/librte_pmd_e1000/igb_rxtx.c     |   10 +++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |  144 ++++++++++++++++++++++++++++++++++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   |   27 ++++++-
 11 files changed, 569 insertions(+), 16 deletions(-)

-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 1/7] ethdev: retrieve flow control configuration
  2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
@ 2014-06-13 13:37 ` David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 2/7] ethdev: add autoneg parameter in flow ctrl accessors David Marchand
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: David Marchand @ 2014-06-13 13:37 UTC (permalink / raw)
  To: dev

From: Zijie Pan <zijie.pan@6wind.com>

This patch adds a new function in ethdev api to retrieve current flow control
configuration.
This operation has been implemented for rte_em_pmd, rte_igb_pmd and
rte_ixgbe_pmd.

Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.c       |   16 ++++++++++
 lib/librte_ether/rte_ethdev.h       |   24 +++++++++++++--
 lib/librte_pmd_e1000/em_ethdev.c    |   44 ++++++++++++++++++++++++++++
 lib/librte_pmd_e1000/igb_ethdev.c   |   44 ++++++++++++++++++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   55 +++++++++++++++++++++++++++++++++--
 5 files changed, 179 insertions(+), 4 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 8011b8b..e881ebe 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1617,6 +1617,22 @@ rte_eth_dev_fdir_set_masks(uint8_t port_id, struct rte_fdir_masks *fdir_mask)
 }
 
 int
+rte_eth_dev_flow_ctrl_get(uint8_t port_id, struct rte_eth_fc_conf *fc_conf)
+{
+	struct rte_eth_dev *dev;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return (-ENODEV);
+	}
+
+	dev = &rte_eth_devices[port_id];
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->flow_ctrl_get, -ENOTSUP);
+	memset(fc_conf, 0, sizeof(*fc_conf));
+	return (*dev->dev_ops->flow_ctrl_get)(dev, fc_conf);
+}
+
+int
 rte_eth_dev_flow_ctrl_set(uint8_t port_id, struct rte_eth_fc_conf *fc_conf)
 {
 	struct rte_eth_dev *dev;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 67eda50..71e9ae0 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -969,8 +969,12 @@ typedef int (*fdir_set_masks_t)(struct rte_eth_dev *dev,
 				struct rte_fdir_masks *fdir_masks);
 /**< @internal Setup flow director masks on an Ethernet device */
 
+typedef int (*flow_ctrl_get_t)(struct rte_eth_dev *dev,
+			       struct rte_eth_fc_conf *fc_conf);
+/**< @internal Get current flow control parameter on an Ethernet device */
+
 typedef int (*flow_ctrl_set_t)(struct rte_eth_dev *dev,
-				struct rte_eth_fc_conf *fc_conf);
+			       struct rte_eth_fc_conf *fc_conf);
 /**< @internal Setup flow control parameter on an Ethernet device */
 
 typedef int (*priority_flow_ctrl_set_t)(struct rte_eth_dev *dev,
@@ -1151,6 +1155,7 @@ struct eth_dev_ops {
 	eth_queue_release_t        tx_queue_release;/**< Release TX queue.*/
 	eth_dev_led_on_t           dev_led_on;    /**< Turn on LED. */
 	eth_dev_led_off_t          dev_led_off;   /**< Turn off LED. */
+	flow_ctrl_get_t            flow_ctrl_get; /**< Get flow control. */
 	flow_ctrl_set_t            flow_ctrl_set; /**< Setup flow control. */
 	priority_flow_ctrl_set_t   priority_flow_ctrl_set; /**< Setup priority flow control.*/
 	eth_mac_addr_remove_t      mac_addr_remove; /**< Remove MAC address */
@@ -2432,6 +2437,21 @@ int  rte_eth_led_on(uint8_t port_id);
 int  rte_eth_led_off(uint8_t port_id);
 
 /**
+ * Get current status of the Ethernet link flow control for Ethernet device
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param fc_conf
+ *   The pointer to the structure where to store the flow control parameters.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if hardware doesn't support flow control.
+ *   - (-ENODEV)  if *port_id* invalid.
+ */
+int rte_eth_dev_flow_ctrl_get(uint8_t port_id,
+			      struct rte_eth_fc_conf *fc_conf);
+
+/**
  * Configure the Ethernet link flow control for Ethernet device
  *
  * @param port_id
@@ -2446,7 +2466,7 @@ int  rte_eth_led_off(uint8_t port_id);
  *   - (-EIO)     if flow control setup failure
  */
 int rte_eth_dev_flow_ctrl_set(uint8_t port_id,
-				struct rte_eth_fc_conf *fc_conf);
+			      struct rte_eth_fc_conf *fc_conf);
 
 /**
  * Configure the Ethernet priority flow control under DCB environment
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 398838f..cb5eb4a 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -77,6 +77,8 @@ static void eth_em_stats_get(struct rte_eth_dev *dev,
 static void eth_em_stats_reset(struct rte_eth_dev *dev);
 static void eth_em_infos_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
+static int eth_em_flow_ctrl_get(struct rte_eth_dev *dev,
+				struct rte_eth_fc_conf *fc_conf);
 static int eth_em_flow_ctrl_set(struct rte_eth_dev *dev,
 				struct rte_eth_fc_conf *fc_conf);
 static int eth_em_interrupt_setup(struct rte_eth_dev *dev);
@@ -153,6 +155,7 @@ static struct eth_dev_ops eth_em_ops = {
 	.tx_queue_release     = eth_em_tx_queue_release,
 	.dev_led_on           = eth_em_led_on,
 	.dev_led_off          = eth_em_led_off,
+	.flow_ctrl_get        = eth_em_flow_ctrl_get,
 	.flow_ctrl_set        = eth_em_flow_ctrl_set,
 	.mac_addr_add         = eth_em_rar_set,
 	.mac_addr_remove      = eth_em_rar_clear,
@@ -1358,6 +1361,47 @@ eth_em_led_off(struct rte_eth_dev *dev)
 }
 
 static int
+eth_em_flow_ctrl_get(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
+{
+	struct e1000_hw *hw;
+	uint32_t ctrl;
+	int tx_pause;
+	int rx_pause;
+
+	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	fc_conf->pause_time = hw->fc.pause_time;
+	fc_conf->high_water = hw->fc.high_water;
+	fc_conf->low_water = hw->fc.low_water;
+	fc_conf->send_xon = hw->fc.send_xon;
+
+	/*
+	 * Return rx_pause and tx_pause status according to actual setting of
+	 * the TFCE and RFCE bits in the CTRL register.
+	 */
+	ctrl = E1000_READ_REG(hw, E1000_CTRL);
+	if (ctrl & E1000_CTRL_TFCE)
+		tx_pause = 1;
+	else
+		tx_pause = 0;
+
+	if (ctrl & E1000_CTRL_RFCE)
+		rx_pause = 1;
+	else
+		rx_pause = 0;
+
+	if (rx_pause && tx_pause)
+		fc_conf->mode = RTE_FC_FULL;
+	else if (rx_pause)
+		fc_conf->mode = RTE_FC_RX_PAUSE;
+	else if (tx_pause)
+		fc_conf->mode = RTE_FC_TX_PAUSE;
+	else
+		fc_conf->mode = RTE_FC_NONE;
+
+	return 0;
+}
+
+static int
 eth_em_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 {
 	struct e1000_hw *hw;
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 6e835c3..3fbbc1a 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -72,6 +72,8 @@ static void eth_igb_stats_get(struct rte_eth_dev *dev,
 static void eth_igb_stats_reset(struct rte_eth_dev *dev);
 static void eth_igb_infos_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
+static int  eth_igb_flow_ctrl_get(struct rte_eth_dev *dev,
+				struct rte_eth_fc_conf *fc_conf);
 static int  eth_igb_flow_ctrl_set(struct rte_eth_dev *dev,
 				struct rte_eth_fc_conf *fc_conf);
 static int eth_igb_lsc_interrupt_setup(struct rte_eth_dev *dev);
@@ -189,6 +191,7 @@ static struct eth_dev_ops eth_igb_ops = {
 	.tx_queue_release     = eth_igb_tx_queue_release,
 	.dev_led_on           = eth_igb_led_on,
 	.dev_led_off          = eth_igb_led_off,
+	.flow_ctrl_get        = eth_igb_flow_ctrl_get,
 	.flow_ctrl_set        = eth_igb_flow_ctrl_set,
 	.mac_addr_add         = eth_igb_rar_set,
 	.mac_addr_remove      = eth_igb_rar_clear,
@@ -1798,6 +1801,47 @@ eth_igb_led_off(struct rte_eth_dev *dev)
 }
 
 static int
+eth_igb_flow_ctrl_get(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
+{
+	struct e1000_hw *hw;
+	uint32_t ctrl;
+	int tx_pause;
+	int rx_pause;
+
+	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	fc_conf->pause_time = hw->fc.pause_time;
+	fc_conf->high_water = hw->fc.high_water;
+	fc_conf->low_water = hw->fc.low_water;
+	fc_conf->send_xon = hw->fc.send_xon;
+
+	/*
+	 * Return rx_pause and tx_pause status according to actual setting of
+	 * the TFCE and RFCE bits in the CTRL register.
+	 */
+	ctrl = E1000_READ_REG(hw, E1000_CTRL);
+	if (ctrl & E1000_CTRL_TFCE)
+		tx_pause = 1;
+	else
+		tx_pause = 0;
+
+	if (ctrl & E1000_CTRL_RFCE)
+		rx_pause = 1;
+	else
+		rx_pause = 0;
+
+	if (rx_pause && tx_pause)
+		fc_conf->mode = RTE_FC_FULL;
+	else if (rx_pause)
+		fc_conf->mode = RTE_FC_RX_PAUSE;
+	else if (tx_pause)
+		fc_conf->mode = RTE_FC_TX_PAUSE;
+	else
+		fc_conf->mode = RTE_FC_NONE;
+
+	return 0;
+}
+
+static int
 eth_igb_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 {
 	struct e1000_hw *hw;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 10e5633..34d6d8d 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -133,8 +133,10 @@ static void ixgbe_vlan_hw_extend_disable(struct rte_eth_dev *dev);
 
 static int ixgbe_dev_led_on(struct rte_eth_dev *dev);
 static int ixgbe_dev_led_off(struct rte_eth_dev *dev);
-static int  ixgbe_flow_ctrl_set(struct rte_eth_dev *dev,
-		struct rte_eth_fc_conf *fc_conf);
+static int ixgbe_flow_ctrl_get(struct rte_eth_dev *dev,
+			       struct rte_eth_fc_conf *fc_conf);
+static int ixgbe_flow_ctrl_set(struct rte_eth_dev *dev,
+			       struct rte_eth_fc_conf *fc_conf);
 static int ixgbe_priority_flow_ctrl_set(struct rte_eth_dev *dev,
 		struct rte_eth_pfc_conf *pfc_conf);
 static int ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
@@ -289,6 +291,7 @@ static struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.tx_queue_release     = ixgbe_dev_tx_queue_release,
 	.dev_led_on           = ixgbe_dev_led_on,
 	.dev_led_off          = ixgbe_dev_led_off,
+	.flow_ctrl_get        = ixgbe_flow_ctrl_get,
 	.flow_ctrl_set        = ixgbe_flow_ctrl_set,
 	.priority_flow_ctrl_set = ixgbe_priority_flow_ctrl_set,
 	.mac_addr_add         = ixgbe_add_rar,
@@ -2259,6 +2262,54 @@ ixgbe_dev_led_off(struct rte_eth_dev *dev)
 }
 
 static int
+ixgbe_flow_ctrl_get(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
+{
+	struct ixgbe_hw *hw;
+	uint32_t mflcn_reg;
+	uint32_t fccfg_reg;
+	int rx_pause;
+	int tx_pause;
+
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	fc_conf->pause_time = hw->fc.pause_time;
+	fc_conf->high_water = hw->fc.high_water[0];
+	fc_conf->low_water = hw->fc.low_water[0];
+	fc_conf->send_xon = hw->fc.send_xon;
+
+	/*
+	 * Return rx_pause status according to actual setting of
+	 * MFLCN register.
+	 */
+	mflcn_reg = IXGBE_READ_REG(hw, IXGBE_MFLCN);
+	if (mflcn_reg & (IXGBE_MFLCN_RPFCE | IXGBE_MFLCN_RFCE))
+		rx_pause = 1;
+	else
+		rx_pause = 0;
+
+	/*
+	 * Return tx_pause status according to actual setting of
+	 * FCCFG register.
+	 */
+	fccfg_reg = IXGBE_READ_REG(hw, IXGBE_FCCFG);
+	if (fccfg_reg & (IXGBE_FCCFG_TFCE_802_3X | IXGBE_FCCFG_TFCE_PRIORITY))
+		tx_pause = 1;
+	else
+		tx_pause = 0;
+
+	if (rx_pause && tx_pause)
+		fc_conf->mode = RTE_FC_FULL;
+	else if (rx_pause)
+		fc_conf->mode = RTE_FC_RX_PAUSE;
+	else if (tx_pause)
+		fc_conf->mode = RTE_FC_TX_PAUSE;
+	else
+		fc_conf->mode = RTE_FC_NONE;
+
+	return 0;
+}
+
+static int
 ixgbe_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 {
 	struct ixgbe_hw *hw;
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 2/7] ethdev: add autoneg parameter in flow ctrl accessors
  2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 1/7] ethdev: retrieve flow control configuration David Marchand
@ 2014-06-13 13:37 ` David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 3/7] ethdev: store min rx buffer size David Marchand
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: David Marchand @ 2014-06-13 13:37 UTC (permalink / raw)
  To: dev

Add autoneg field in flow control parameters.
This makes it easier to understand why changing some parameters does not always
have the expected result.

Changing autoneg is not supported at the moment.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.h       |    1 +
 lib/librte_pmd_e1000/em_ethdev.c    |    3 +++
 lib/librte_pmd_e1000/igb_ethdev.c   |    3 +++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |    3 +++
 4 files changed, 10 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 71e9ae0..5113b7a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -550,6 +550,7 @@ struct rte_eth_fc_conf {
 	uint16_t send_xon;    /**< Is XON frame need be sent */
 	enum rte_eth_fc_mode mode;  /**< Link flow control mode */
 	uint8_t mac_ctrl_frame_fwd; /**< Forward MAC control frames */
+	uint8_t autoneg;      /**< Use Pause autoneg */
 };
 
 /**
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index cb5eb4a..657f843 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -1373,6 +1373,7 @@ eth_em_flow_ctrl_get(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	fc_conf->high_water = hw->fc.high_water;
 	fc_conf->low_water = hw->fc.low_water;
 	fc_conf->send_xon = hw->fc.send_xon;
+	fc_conf->autoneg = hw->mac.autoneg;
 
 	/*
 	 * Return rx_pause and tx_pause status according to actual setting of
@@ -1417,6 +1418,8 @@ eth_em_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	uint32_t rctl;
 
 	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	if (fc_conf->autoneg != hw->mac.autoneg)
+		return -ENOTSUP;
 	rx_buf_size = em_get_rx_buffer_size(hw);
 	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n", rx_buf_size);
 
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 3fbbc1a..a2f1af9 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -1813,6 +1813,7 @@ eth_igb_flow_ctrl_get(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	fc_conf->high_water = hw->fc.high_water;
 	fc_conf->low_water = hw->fc.low_water;
 	fc_conf->send_xon = hw->fc.send_xon;
+	fc_conf->autoneg = hw->mac.autoneg;
 
 	/*
 	 * Return rx_pause and tx_pause status according to actual setting of
@@ -1857,6 +1858,8 @@ eth_igb_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	uint32_t rctl;
 
 	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	if (fc_conf->autoneg != hw->mac.autoneg)
+		return -ENOTSUP;
 	rx_buf_size = igb_get_rx_buffer_size(hw);
 	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n", rx_buf_size);
 
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 34d6d8d..5c846e5 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -2276,6 +2276,7 @@ ixgbe_flow_ctrl_get(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	fc_conf->high_water = hw->fc.high_water[0];
 	fc_conf->low_water = hw->fc.low_water[0];
 	fc_conf->send_xon = hw->fc.send_xon;
+	fc_conf->autoneg = !hw->fc.disable_fc_autoneg;
 
 	/*
 	 * Return rx_pause status according to actual setting of
@@ -2327,6 +2328,8 @@ ixgbe_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	PMD_INIT_FUNC_TRACE();
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	if (fc_conf->autoneg != !hw->fc.disable_fc_autoneg)
+		return -ENOTSUP;
 	rx_buf_size = IXGBE_READ_REG(hw, IXGBE_RXPBSIZE(0));
 	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n", rx_buf_size);
 
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 3/7] ethdev: store min rx buffer size
  2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 1/7] ethdev: retrieve flow control configuration David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 2/7] ethdev: add autoneg parameter in flow ctrl accessors David Marchand
@ 2014-06-13 13:37 ` David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 4/7] ethdev: introduce enable_scatter rx mode David Marchand
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: David Marchand @ 2014-06-13 13:37 UTC (permalink / raw)
  To: dev

This avoids code duplication in PMD when dealing with mtu changes.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.c |   19 ++++++++++++++-----
 lib/librte_ether/rte_ethdev.h |    3 +++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index e881ebe..df18b7b 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -879,6 +879,8 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
 		       const struct rte_eth_rxconf *rx_conf,
 		       struct rte_mempool *mp)
 {
+	int ret;
+	uint32_t mbp_buf_size;
 	struct rte_eth_dev *dev;
 	struct rte_pktmbuf_pool_private *mbp_priv;
 	struct rte_eth_dev_info dev_info;
@@ -919,13 +921,14 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
 		return (-ENOSPC);
 	}
 	mbp_priv = rte_mempool_get_priv(mp);
-	if ((uint32_t) (mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM) <
-	    dev_info.min_rx_bufsize) {
+	mbp_buf_size = mbp_priv->mbuf_data_room_size;
+
+	if ((mbp_buf_size - RTE_PKTMBUF_HEADROOM) < dev_info.min_rx_bufsize) {
 		PMD_DEBUG_TRACE("%s mbuf_data_room_size %d < %d "
 				"(RTE_PKTMBUF_HEADROOM=%d + min_rx_bufsize(dev)"
 				"=%d)\n",
 				mp->name,
-				(int)mbp_priv->mbuf_data_room_size,
+				(int)mbp_buf_size,
 				(int)(RTE_PKTMBUF_HEADROOM +
 				      dev_info.min_rx_bufsize),
 				(int)RTE_PKTMBUF_HEADROOM,
@@ -933,8 +936,14 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
 		return (-EINVAL);
 	}
 
-	return (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
-					       socket_id, rx_conf, mp);
+	ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
+					      socket_id, rx_conf, mp);
+	if (!ret) {
+		if (dev->data->min_rx_buf_size > mbp_buf_size)
+			dev->data->min_rx_buf_size = mbp_buf_size;
+	}
+
+	return ret;
 }
 
 int
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 5113b7a..3701023 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1262,6 +1262,9 @@ struct rte_eth_dev_data {
 	struct rte_eth_conf dev_conf;   /**< Configuration applied to device. */
 	uint16_t max_frame_size;        /**< Default is ETHER_MAX_LEN (1518). */
 
+	uint32_t min_rx_buf_size;
+	/**< Common rx buffer size handled by all queues */
+
 	uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
 	struct ether_addr* mac_addrs;/**< Device Ethernet Link address. */
 	uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 4/7] ethdev: introduce enable_scatter rx mode
  2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
                   ` (2 preceding siblings ...)
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 3/7] ethdev: store min rx buffer size David Marchand
@ 2014-06-13 13:37 ` David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 5/7] ethdev: add mtu accessors David Marchand
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: David Marchand @ 2014-06-13 13:37 UTC (permalink / raw)
  To: dev

We might want to be sure the scatter packets reception handler is selected in a
pmd. This makes it possible to then change mtu later, without the need of
restarting a port.
It is then the pmd duty to tell it enabled the scatter reception handler by
setting dev->data->scattered_rx to 1.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.h     |    3 ++-
 lib/librte_pmd_e1000/em_rxtx.c    |    5 +++++
 lib/librte_pmd_e1000/igb_rxtx.c   |   10 ++++++++++
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c |   10 ++++++++++
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 3701023..54646bb 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -301,7 +301,8 @@ struct rte_eth_rxmode {
 		hw_vlan_strip    : 1, /**< VLAN strip enable. */
 		hw_vlan_extend   : 1, /**< Extended VLAN enable. */
 		jumbo_frame      : 1, /**< Jumbo Frame Receipt enable. */
-		hw_strip_crc     : 1; /**< Enable CRC stripping by hardware. */
+		hw_strip_crc     : 1, /**< Enable CRC stripping by hardware. */
+		enable_scatter   : 1; /**< Enable scatter packets rx handler */
 };
 
 /**
diff --git a/lib/librte_pmd_e1000/em_rxtx.c b/lib/librte_pmd_e1000/em_rxtx.c
index 1575e79..e5f1933 100644
--- a/lib/librte_pmd_e1000/em_rxtx.c
+++ b/lib/librte_pmd_e1000/em_rxtx.c
@@ -1714,6 +1714,11 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 		}
 	}
 
+	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		dev->rx_pkt_burst = eth_em_recv_scattered_pkts;
+		dev->data->scattered_rx = 1;
+	}
+
 	/*
 	 * Setup the Checksum Register.
 	 * Receive Full-Packet Checksum Offload is mutually exclusive with RSS.
diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index c11931f..1cee40c 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -1997,6 +1997,11 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 		E1000_WRITE_REG(hw, E1000_RXDCTL(rxq->reg_idx), rxdctl);
 	}
 
+	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		dev->rx_pkt_burst = eth_igb_recv_scattered_pkts;
+		dev->data->scattered_rx = 1;
+	}
+
 	/*
 	 * Setup BSIZE field of RCTL register, if needed.
 	 * Buffer sizes >= 1024 are not [supposed to be] setup in the RCTL
@@ -2266,6 +2271,11 @@ eth_igbvf_rx_init(struct rte_eth_dev *dev)
 		E1000_WRITE_REG(hw, E1000_RXDCTL(i), rxdctl);
 	}
 
+	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		dev->rx_pkt_burst = eth_igb_recv_scattered_pkts;
+		dev->data->scattered_rx = 1;
+	}
+
 	/*
 	 * Setup the HW Rx Head and Tail Descriptor Pointers.
 	 * This needs to be done after enable.
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 67afd5a..a2f6413 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -3478,6 +3478,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		}
 	}
 
+	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
+		dev->data->scattered_rx = 1;
+	}
+
 	/*
 	 * Device configured with multiple RX queues.
 	 */
@@ -3953,6 +3958,11 @@ ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
 		}
 	}
 
+	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
+		dev->data->scattered_rx = 1;
+	}
+
 	return 0;
 }
 
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 5/7] ethdev: add mtu accessors
  2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
                   ` (3 preceding siblings ...)
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 4/7] ethdev: introduce enable_scatter rx mode David Marchand
@ 2014-06-13 13:37 ` David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 6/7] ixgbe: add set_mtu to ixgbevf David Marchand
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: David Marchand @ 2014-06-13 13:37 UTC (permalink / raw)
  To: dev

From: Samuel Gauthier <samuel.gauthier@6wind.com>

This patch adds two new functions in ethdev api to retrieve current MTU and
change MTU of a port.
Only .mtu_set function is pmd specific.
pmd should update its max_rx_pkt_len if needed.

This operation has been implemented for rte_em_pmd, rte_igb_pmd and
rte_ixgbe_pmd.

Signed-off-by: Samuel Gauthier <samuel.gauthier@6wind.com>
Signed-off-by: Ivan Boule <ivan.boule@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_ether/rte_ethdev.c       |   41 +++++++++++++++++++++++++--
 lib/librte_ether/rte_ethdev.h       |   34 +++++++++++++++++++++-
 lib/librte_pmd_e1000/em_ethdev.c    |   42 +++++++++++++++++++++++++++
 lib/librte_pmd_e1000/igb_ethdev.c   |   53 +++++++++++++++++++++++++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   49 ++++++++++++++++++++++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   |    2 +-
 6 files changed, 217 insertions(+), 4 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index df18b7b..a411f99 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -200,9 +200,9 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv,
 	TAILQ_INIT(&(eth_dev->callbacks));
 
 	/*
-	 * Set the default maximum frame size.
+	 * Set the default MTU.
 	 */
-	eth_dev->data->max_frame_size = ETHER_MAX_LEN;
+	eth_dev->data->mtu = ETHER_MTU;
 
 	/* Invoke PMD device initialization function */
 	diag = (*eth_drv->eth_dev_init)(eth_drv, eth_dev);
@@ -1228,6 +1228,43 @@ rte_eth_macaddr_get(uint8_t port_id, struct ether_addr *mac_addr)
 	ether_addr_copy(&dev->data->mac_addrs[0], mac_addr);
 }
 
+
+int
+rte_eth_dev_get_mtu(uint8_t port_id, uint16_t *mtu)
+{
+	struct rte_eth_dev *dev;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return (-ENODEV);
+	}
+
+	dev = &rte_eth_devices[port_id];
+	*mtu = dev->data->mtu;
+	return 0;
+}
+
+int
+rte_eth_dev_set_mtu(uint8_t port_id, uint16_t mtu)
+{
+	int ret;
+	struct rte_eth_dev *dev;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return (-ENODEV);
+	}
+
+	dev = &rte_eth_devices[port_id];
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mtu_set, -ENOTSUP);
+
+	ret = (*dev->dev_ops->mtu_set)(dev, mtu);
+	if (!ret)
+		dev->data->mtu = mtu;
+
+	return ret;
+}
+
 int
 rte_eth_dev_vlan_filter(uint8_t port_id, uint16_t vlan_id, int on)
 {
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 54646bb..7e490d0 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -905,6 +905,9 @@ typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
 /**< @Check DD bit of specific RX descriptor */
 
+typedef int (*mtu_set_t)(struct rte_eth_dev *dev, uint16_t mtu);
+/**< @internal Set MTU. */
+
 typedef int (*vlan_filter_set_t)(struct rte_eth_dev *dev,
 				  uint16_t vlan_id,
 				  int on);
@@ -1141,6 +1144,7 @@ struct eth_dev_ops {
 	eth_queue_stats_mapping_set_t queue_stats_mapping_set;
 	/**< Configure per queue stat counter mapping. */
 	eth_dev_infos_get_t        dev_infos_get; /**< Get device info. */
+	mtu_set_t                  mtu_set; /**< Set MTU. */
 	vlan_filter_set_t          vlan_filter_set;  /**< Filter VLAN Setup. */
 	vlan_tpid_set_t            vlan_tpid_set;      /**< Outer VLAN TPID Setup. */
 	vlan_strip_queue_set_t     vlan_strip_queue_set; /**< VLAN Stripping on queue. */
@@ -1261,7 +1265,7 @@ struct rte_eth_dev_data {
 	/**< Link-level information & status */
 
 	struct rte_eth_conf dev_conf;   /**< Configuration applied to device. */
-	uint16_t max_frame_size;        /**< Default is ETHER_MAX_LEN (1518). */
+	uint16_t mtu;                   /**< Maximum Transmission Unit. */
 
 	uint32_t min_rx_buf_size;
 	/**< Common rx buffer size handled by all queues */
@@ -1808,6 +1812,34 @@ extern void rte_eth_dev_info_get(uint8_t port_id,
 				 struct rte_eth_dev_info *dev_info);
 
 /**
+ * Retrieve the MTU of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param mtu
+ *   A pointer to a uint16_t where the retrieved MTU is to be stored.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ */
+extern int rte_eth_dev_get_mtu(uint8_t port_id, uint16_t *mtu);
+
+/**
+ * Change the MTU of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param mtu
+ *   A uint16_t for the MTU to be applied.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if operation is not supported.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if *mtu* invalid.
+ */
+extern int rte_eth_dev_set_mtu(uint8_t port_id, uint16_t mtu);
+
+/**
  * Enable/Disable hardware filtering by an Ethernet device of received
  * VLAN packets tagged with a given VLAN Tag Identifier.
  *
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 657f843..328a2e7 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -94,6 +94,8 @@ static void em_hw_control_release(struct e1000_hw *hw);
 static void em_init_manageability(struct e1000_hw *hw);
 static void em_release_manageability(struct e1000_hw *hw);
 
+static int eth_em_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
+
 static int eth_em_vlan_filter_set(struct rte_eth_dev *dev,
 		uint16_t vlan_id, int on);
 static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask);
@@ -145,6 +147,7 @@ static struct eth_dev_ops eth_em_ops = {
 	.stats_get            = eth_em_stats_get,
 	.stats_reset          = eth_em_stats_reset,
 	.dev_infos_get        = eth_em_infos_get,
+	.mtu_set              = eth_em_mtu_set,
 	.vlan_filter_set      = eth_em_vlan_filter_set,
 	.vlan_offload_set     = eth_em_vlan_offload_set,
 	.rx_queue_setup       = eth_em_rx_queue_setup,
@@ -1482,6 +1485,45 @@ eth_em_rar_clear(struct rte_eth_dev *dev, uint32_t index)
 	e1000_rar_set(hw, addr, index);
 }
 
+static int
+eth_em_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	struct rte_eth_dev_info dev_info;
+	struct e1000_hw *hw;
+	uint32_t frame_size;
+	uint32_t rctl;
+
+	eth_em_infos_get(dev, &dev_info);
+	frame_size = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE;
+
+	/* check that mtu is within the allowed range */
+	if ((mtu < 68) || (frame_size > dev_info.max_rx_pktlen))
+		return -EINVAL;
+
+	/* refuse mtu that requires the support of scattered packets when this
+	 * feature has not been enabled before. */
+	if (!dev->data->scattered_rx &&
+	    frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
+		return -EINVAL;
+
+	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	rctl = E1000_READ_REG(hw, E1000_RCTL);
+
+	/* switch to jumbo mode if needed */
+	if (frame_size > ETHER_MAX_LEN) {
+		dev->data->dev_conf.rxmode.jumbo_frame = 1;
+		rctl |= E1000_RCTL_LPE;
+	} else {
+		dev->data->dev_conf.rxmode.jumbo_frame = 0;
+		rctl &= ~E1000_RCTL_LPE;
+	}
+	E1000_WRITE_REG(hw, E1000_RCTL, rctl);
+
+	/* update max frame size */
+	dev->data->dev_conf.rxmode.max_rx_pkt_len = frame_size;
+	return 0;
+}
+
 struct rte_driver em_pmd_drv = {
 	.type = PMD_PDEV,
 	.init = rte_em_pmd_init,
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index a2f1af9..402361c 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -87,6 +87,8 @@ static void igb_hw_control_release(struct e1000_hw *hw);
 static void igb_init_manageability(struct e1000_hw *hw);
 static void igb_release_manageability(struct e1000_hw *hw);
 
+static int  eth_igb_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
+
 static int eth_igb_vlan_filter_set(struct rte_eth_dev *dev,
 		uint16_t vlan_id, int on);
 static void eth_igb_vlan_tpid_set(struct rte_eth_dev *dev, uint16_t tpid_id);
@@ -180,6 +182,7 @@ static struct eth_dev_ops eth_igb_ops = {
 	.stats_get            = eth_igb_stats_get,
 	.stats_reset          = eth_igb_stats_reset,
 	.dev_infos_get        = eth_igb_infos_get,
+	.mtu_set              = eth_igb_mtu_set,
 	.vlan_filter_set      = eth_igb_vlan_filter_set,
 	.vlan_tpid_set        = eth_igb_vlan_tpid_set,
 	.vlan_offload_set     = eth_igb_vlan_offload_set,
@@ -2233,6 +2236,56 @@ eth_igb_rss_reta_query(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+eth_igb_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	uint32_t rctl;
+	struct e1000_hw *hw;
+	struct rte_eth_dev_info dev_info;
+	uint32_t frame_size = mtu + (ETHER_HDR_LEN + ETHER_CRC_LEN +
+				     VLAN_TAG_SIZE);
+
+	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+#ifdef RTE_LIBRTE_82571_SUPPORT
+	/* XXX: not bigger than max_rx_pktlen */
+	if (hw->mac.type == e1000_82571)
+		return -ENOTSUP;
+#endif
+	eth_igb_infos_get(dev, &dev_info);
+
+	/* check that mtu is within the allowed range */
+	if ((mtu < 68) ||
+	    (frame_size > dev_info.max_rx_pktlen))
+		return -EINVAL;
+
+	/* refuse mtu that requires the support of scattered packets when this
+	 * feature has not been enabled before. */
+	if (!dev->data->scattered_rx &&
+	    frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
+		return -EINVAL;
+
+	rctl = E1000_READ_REG(hw, E1000_RCTL);
+
+	/* switch to jumbo mode if needed */
+	if (frame_size > ETHER_MAX_LEN) {
+		dev->data->dev_conf.rxmode.jumbo_frame = 1;
+		rctl |= E1000_RCTL_LPE;
+	} else {
+		dev->data->dev_conf.rxmode.jumbo_frame = 0;
+		rctl &= ~E1000_RCTL_LPE;
+	}
+	E1000_WRITE_REG(hw, E1000_RCTL, rctl);
+
+	/* update max frame size */
+	dev->data->dev_conf.rxmode.max_rx_pkt_len = frame_size;
+
+	E1000_WRITE_REG(hw, E1000_RLPML,
+			dev->data->dev_conf.rxmode.max_rx_pkt_len);
+
+	return 0;
+}
+
 static struct rte_driver pmd_igb_drv = {
 	.type = PMD_PDEV,
 	.init = rte_igb_pmd_init,
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 5c846e5..43d0683 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -118,6 +118,9 @@ static int ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 					     uint8_t is_rx);
 static void ixgbe_dev_info_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
+
+static int ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
+
 static int ixgbe_vlan_filter_set(struct rte_eth_dev *dev,
 		uint16_t vlan_id, int on);
 static void ixgbe_vlan_tpid_set(struct rte_eth_dev *dev, uint16_t tpid_id);
@@ -275,6 +278,7 @@ static struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.stats_reset          = ixgbe_dev_stats_reset,
 	.queue_stats_mapping_set = ixgbe_dev_queue_stats_mapping_set,
 	.dev_infos_get        = ixgbe_dev_info_get,
+	.mtu_set              = ixgbe_dev_mtu_set,
 	.vlan_filter_set      = ixgbe_vlan_filter_set,
 	.vlan_tpid_set        = ixgbe_vlan_tpid_set,
 	.vlan_offload_set     = ixgbe_vlan_offload_set,
@@ -2673,6 +2677,51 @@ ixgbe_remove_rar(struct rte_eth_dev *dev, uint32_t index)
 	ixgbe_clear_rar(hw, index);
 }
 
+static int
+ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	uint32_t hlreg0;
+	uint32_t maxfrs;
+	struct ixgbe_hw *hw;
+	struct rte_eth_dev_info dev_info;
+	uint32_t frame_size = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+
+	ixgbe_dev_info_get(dev, &dev_info);
+
+	/* check that mtu is within the allowed range */
+	if ((mtu < 68) || (frame_size > dev_info.max_rx_pktlen))
+		return -EINVAL;
+
+	/* refuse mtu that requires the support of scattered packets when this
+	 * feature has not been enabled before. */
+	if (!dev->data->scattered_rx &&
+	    frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
+		return -EINVAL;
+
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0);
+
+	/* switch to jumbo mode if needed */
+	if (frame_size > ETHER_MAX_LEN) {
+		dev->data->dev_conf.rxmode.jumbo_frame = 1;
+		hlreg0 |= IXGBE_HLREG0_JUMBOEN;
+	} else {
+		dev->data->dev_conf.rxmode.jumbo_frame = 0;
+		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
+	}
+	IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0);
+
+	/* update max frame size */
+	dev->data->dev_conf.rxmode.max_rx_pkt_len = frame_size;
+
+	maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS);
+	maxfrs &= 0x0000FFFF;
+	maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16);
+	IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs);
+
+	return 0;
+}
+
 /*
  * Virtual Function operations
  */
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index a2f6413..43b7d0b 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -2878,7 +2878,7 @@ ixgbe_dcb_hw_configure(struct rte_eth_dev *dev,
 	uint16_t max[IXGBE_DCB_MAX_TRAFFIC_CLASS] = {0};
 	uint8_t map[IXGBE_DCB_MAX_TRAFFIC_CLASS] = {0};
 	struct ixgbe_dcb_tc_config *tc;
-	uint32_t max_frame = dev->data->max_frame_size;
+	uint32_t max_frame = dev->data->mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
 	struct ixgbe_hw *hw =
 			IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 6/7] ixgbe: add set_mtu to ixgbevf
  2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
                   ` (4 preceding siblings ...)
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 5/7] ethdev: add mtu accessors David Marchand
@ 2014-06-13 13:37 ` David Marchand
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 7/7] app/testpmd: allow to configure mtu David Marchand
  2014-06-16 17:07 ` [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers Ananyev, Konstantin
  7 siblings, 0 replies; 15+ messages in thread
From: David Marchand @ 2014-06-13 13:37 UTC (permalink / raw)
  To: dev

From: Ivan Boule <ivan.boule@6wind.com>

The support of jumbo frames in the ixgbevf Poll Mode Driver of 10GbE
82599 VF functions consists in the following enhancements:

- Implement the mtu_set function in the ixgbevf PMD, using the IXGBE_VF_SET_LPE
  request of the version 1.0 of the VF/PF mailbox API for this purpose.

- Add a detailed explanation on the VF/PF rx max frame len negotiation.

Signed-off-by: Ivan Boule <ivan.boule@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   37 +++++++++++++++++++++++++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   |   15 +++++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 43d0683..da6dc0a 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -202,6 +202,8 @@ static void ixgbevf_add_mac_addr(struct rte_eth_dev *dev,
 				 uint32_t index, uint32_t pool);
 static void ixgbevf_remove_mac_addr(struct rte_eth_dev *dev, uint32_t index);
 
+static int ixgbevf_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu);
+
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
  */
@@ -349,6 +351,7 @@ static struct eth_dev_ops ixgbevf_eth_dev_ops = {
 	.stats_reset          = ixgbevf_dev_stats_reset,
 	.dev_close            = ixgbevf_dev_close,
 	.dev_infos_get        = ixgbe_dev_info_get,
+	.mtu_set              = ixgbevf_dev_set_mtu,
 	.vlan_filter_set      = ixgbevf_vlan_filter_set,
 	.vlan_strip_queue_set = ixgbevf_vlan_strip_queue_set,
 	.vlan_offload_set     = ixgbevf_vlan_offload_set,
@@ -3491,6 +3494,40 @@ ixgbevf_remove_mac_addr(struct rte_eth_dev *dev, uint32_t index)
 	}
 }
 
+static int
+ixgbevf_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
+{
+	struct ixgbe_hw *hw;
+	uint16_t max_frame = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN;
+
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* MTU < 68 is an error and causes problems on some kernels */
+	if ((mtu < 68) || (max_frame > ETHER_MAX_JUMBO_FRAME_LEN))
+		return -EINVAL;
+
+	/* refuse mtu that requires the support of scattered packets when this
+	 * feature has not been enabled before. */
+	if (!dev->data->scattered_rx &&
+	    max_frame > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
+		return -EINVAL;
+
+	/*
+	 * When supported by the underlying PF driver, use the IXGBE_VF_SET_MTU
+	 * request of the version 2.0 of the mailbox API.
+	 * For now, use the IXGBE_VF_SET_LPE request of the version 1.0
+	 * of the mailbox API.
+	 * This call to IXGBE_SET_LPE action won't work with ixgbe pf drivers
+	 * prior to 3.11.33 which contains the following change:
+	 * "ixgbe: Enable jumbo frames support w/ SR-IOV"
+	 */
+	ixgbevf_rlpml_set_vf(hw, max_frame);
+
+	/* update max frame size */
+	dev->data->dev_conf.rxmode.max_rx_pkt_len = max_frame;
+	return 0;
+}
+
 static struct rte_driver rte_ixgbe_driver = {
 	.type = PMD_PDEV,
 	.init = rte_ixgbe_pmd_init,
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 43b7d0b..2f50640 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -3875,7 +3875,20 @@ ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	/* setup MTU */
+	/*
+	 * When the VF driver issues a IXGBE_VF_RESET request, the PF driver
+	 * disables the VF receipt of packets if the PF MTU is > 1500.
+	 * This is done to deal with 82599 limitations that imposes
+	 * the PF and all VFs to share the same MTU.
+	 * Then, the PF driver enables again the VF receipt of packet when
+	 * the VF driver issues a IXGBE_VF_SET_LPE request.
+	 * In the meantime, the VF device cannot be used, even if the VF driver
+	 * and the Guest VM network stack are ready to accept packets with a
+	 * size up to the PF MTU.
+	 * As a work-around to this PF behaviour, force the call to
+	 * ixgbevf_rlpml_set_vf even if jumbo frames are not used. This way,
+	 * VF packets received can work in all cases.
+	 */
 	ixgbevf_rlpml_set_vf(hw,
 		(uint16_t)dev->data->dev_conf.rxmode.max_rx_pkt_len);
 
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 7/7] app/testpmd: allow to configure mtu
  2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
                   ` (5 preceding siblings ...)
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 6/7] ixgbe: add set_mtu to ixgbevf David Marchand
@ 2014-06-13 13:37 ` David Marchand
  2014-06-16 17:07 ` [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers Ananyev, Konstantin
  7 siblings, 0 replies; 15+ messages in thread
From: David Marchand @ 2014-06-13 13:37 UTC (permalink / raw)
  To: dev

From: Ivan Boule <ivan.boule@6wind.com>

Take avantage of the .set_mtu ethdev function and make it possible to configure
MTU on devices using testpmd.

Signed-off-by: Ivan Boule <ivan.boule@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 app/test-pmd/cmdline.c |   54 ++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/config.c  |   13 ++++++++++++
 app/test-pmd/testpmd.h |    2 +-
 3 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 4678977..6b150c1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -523,6 +523,8 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port config all (txfreet|txrst|rxfreet) (value)\n"
 			"    Set free threshold for rx/tx, or set"
 			" tx rs bit threshold.\n\n"
+			"port config mtu X value\n"
+			"    Set the MTU of port X to a given value\n\n"
 		);
 	}
 
@@ -1021,6 +1023,57 @@ cmdline_parse_inst_t cmd_config_max_pkt_len = {
 	},
 };
 
+/* *** configure port MTU *** */
+struct cmd_config_mtu_result {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t mtu;
+	uint8_t port_id;
+	uint16_t value;
+};
+
+static void
+cmd_config_mtu_parsed(void *parsed_result,
+		      __attribute__((unused)) struct cmdline *cl,
+		      __attribute__((unused)) void *data)
+{
+	struct cmd_config_mtu_result *res = parsed_result;
+
+	if (res->value < ETHER_MIN_LEN) {
+		printf("mtu cannot be less than %d\n", ETHER_MIN_LEN);
+		return;
+	}
+	port_mtu_set(res->port_id, res->value);
+}
+
+cmdline_parse_token_string_t cmd_config_mtu_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_mtu_result, port,
+				 "port");
+cmdline_parse_token_string_t cmd_config_mtu_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_mtu_result, keyword,
+				 "config");
+cmdline_parse_token_string_t cmd_config_mtu_mtu =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_mtu_result, keyword,
+				 "mtu");
+cmdline_parse_token_num_t cmd_config_mtu_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_mtu_result, port_id, UINT8);
+cmdline_parse_token_num_t cmd_config_mtu_value =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_mtu_result, value, UINT16);
+
+cmdline_parse_inst_t cmd_config_mtu = {
+	.f = cmd_config_mtu_parsed,
+	.data = NULL,
+	.help_str = "port config mtu value",
+	.tokens = {
+		(void *)&cmd_config_mtu_port,
+		(void *)&cmd_config_mtu_keyword,
+		(void *)&cmd_config_mtu_mtu,
+		(void *)&cmd_config_mtu_port_id,
+		(void *)&cmd_config_mtu_value,
+		NULL,
+	},
+};
+
 /* *** configure rx mode *** */
 struct cmd_config_rx_mode_flag {
 	cmdline_fixed_string_t port;
@@ -5600,6 +5653,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_config_speed_all,
 	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
 	(cmdline_parse_inst_t *)&cmd_config_rx_tx,
+	(cmdline_parse_inst_t *)&cmd_config_mtu,
 	(cmdline_parse_inst_t *)&cmd_config_max_pkt_len,
 	(cmdline_parse_inst_t *)&cmd_config_rx_mode_flag,
 	(cmdline_parse_inst_t *)&cmd_config_rss,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 52ad01a..4cebe00 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -493,6 +493,19 @@ port_reg_set(portid_t port_id, uint32_t reg_off, uint32_t reg_v)
 	display_port_reg_value(port_id, reg_off, reg_v);
 }
 
+void
+port_mtu_set(portid_t port_id, uint16_t mtu)
+{
+	int diag;
+
+	if (port_id_is_invalid(port_id))
+		return;
+	diag = rte_eth_dev_set_mtu(port_id, mtu);
+	if (diag == 0)
+		return;
+	printf("Set MTU failed. diag=%d\n", diag);
+}
+
 /*
  * RX/TX ring descriptors display functions.
  */
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 4fabf1c..4c1ed03 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -454,7 +454,7 @@ void fwd_config_setup(void);
 void set_def_fwd_config(void);
 int init_fwd_streams(void);
 
-
+void port_mtu_set(portid_t port_id, uint16_t mtu);
 void port_reg_bit_display(portid_t port_id, uint32_t reg_off, uint8_t bit_pos);
 void port_reg_bit_set(portid_t port_id, uint32_t reg_off, uint8_t bit_pos,
 		      uint8_t bit_v);
-- 
1.7.10.4

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

* Re: [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers
  2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
                   ` (6 preceding siblings ...)
  2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 7/7] app/testpmd: allow to configure mtu David Marchand
@ 2014-06-16 17:07 ` Ananyev, Konstantin
  2014-06-17  8:42   ` David Marchand
  7 siblings, 1 reply; 15+ messages in thread
From: Ananyev, Konstantin @ 2014-06-16 17:07 UTC (permalink / raw)
  To: David Marchand, dev

Hi David,

> This patchset introduces 3 new ethdev operations: flow control parameters
> retrieval and mtu get/set operations.

> Changes since v1:
> - compute min rx buffer size at ethdev level (to simplify pmd mtu checks)
> - introduce enable_scatter rx mode so that we can advise pmd to configure
>  scatter mode
> - rework mtu get/set operations (based on Konstantin comments)
> - pass checkpatch.pl checks

1)  [PATCH v2 3/7] ethdev: store min rx buffer size
@@ -879,6 +879,8 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
 		       const struct rte_eth_rxconf *rx_conf,
 		       struct rte_mempool *mp)
 {
...
+	if (!ret) {
+		if (dev->data->min_rx_buf_size > mbp_buf_size)
+			dev->data->min_rx_buf_size = mbp_buf_size;
+	}
+
+	return ret;
 
Where do you set the initial value of min_rx_buf_size?
Can't find it by some reason.

2)  [PATCH v2 5/7] ethdev: add mtu accessors
+static int
+ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
+{
...
+	if (!dev->data->scattered_rx &&
+	    frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
+		return -EINVAL;

Reading 82599 spec, 8.2.3.22.13 Max Frame Size - MAXFRS (0x04268; RW):
" The MFS does not include the 4 bytes of the VLAN header. Packets with VLAN header
can be as large as MFS + 4. When double VLAN is enabled, the device adds 8 to the
MFS for any packets."

So, I suppose it should be:
if (!dev->data->scattered_rx &&
frame_size + 2 * IXGBE_VLAN_TAG_SIZE > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)

Like in ixgbe_dev_rx_init().


3)  if ((mtu < 68) || (frame_size > dev_info.max_rx_pktlen))
Can we add a new define for min allowable MTU (68) as it used in few places.

Thanks
Konstantin

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

* Re: [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers
  2014-06-16 17:07 ` [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers Ananyev, Konstantin
@ 2014-06-17  8:42   ` David Marchand
  2014-06-17  8:57     ` Ananyev, Konstantin
  0 siblings, 1 reply; 15+ messages in thread
From: David Marchand @ 2014-06-17  8:42 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev

Hello Konstantin,


On 06/16/2014 07:07 PM, Ananyev, Konstantin wrote:
>
> 1)  [PATCH v2 3/7] ethdev: store min rx buffer size
> @@ -879,6 +879,8 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>   		       const struct rte_eth_rxconf *rx_conf,
>   		       struct rte_mempool *mp)
>   {
> ...
> +	if (!ret) {
> +		if (dev->data->min_rx_buf_size > mbp_buf_size)
> +			dev->data->min_rx_buf_size = mbp_buf_size;
> +	}
> +
> +	return ret;
>
> Where do you set the initial value of min_rx_buf_size?
> Can't find it by some reason.

Hum, actually, dev->data structure is supposed to be set to 0 at init 
time or I missed something.

I would say this happens once for the whole rte_eth_dev_data array in 
rte_eth_dev_data_alloc() in primary process (first call to 
rte_eth_dev_allocate()).


>
> 2)  [PATCH v2 5/7] ethdev: add mtu accessors
> +static int
> +ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
> +{
> ...
> +	if (!dev->data->scattered_rx &&
> +	    frame_size > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
> +		return -EINVAL;
>
> Reading 82599 spec, 8.2.3.22.13 Max Frame Size - MAXFRS (0x04268; RW):
> " The MFS does not include the 4 bytes of the VLAN header. Packets with VLAN header
> can be as large as MFS + 4. When double VLAN is enabled, the device adds 8 to the
> MFS for any packets."
>
> So, I suppose it should be:
> if (!dev->data->scattered_rx &&
> frame_size + 2 * IXGBE_VLAN_TAG_SIZE > dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
>
> Like in ixgbe_dev_rx_init().

Ok, I forgot to take this part you mentioned earlier.
I will send an update later (depending on the points 1) and 3)).


>
> 3)  if ((mtu < 68) || (frame_size > dev_info.max_rx_pktlen))
> Can we add a new define for min allowable MTU (68) as it used in few places.

RTE_IPV4_MIN_MTU then ?
I am not sure where this belongs, it could go in rte_ethdev.h.



-- 
David Marchand

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

* Re: [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers
  2014-06-17  8:42   ` David Marchand
@ 2014-06-17  8:57     ` Ananyev, Konstantin
  2014-06-17  9:25       ` Thomas Monjalon
  2014-06-17 13:39       ` David Marchand
  0 siblings, 2 replies; 15+ messages in thread
From: Ananyev, Konstantin @ 2014-06-17  8:57 UTC (permalink / raw)
  To: David Marchand, dev

Hi David,

>>
>> 1)  [PATCH v2 3/7] ethdev: store min rx buffer size
>> @@ -879,6 +879,8 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>>   		       const struct rte_eth_rxconf *rx_conf,
>>   		       struct rte_mempool *mp)
>>   {
>> ...
>> +	if (!ret) {
>> +		if (dev->data->min_rx_buf_size > mbp_buf_size)
>> +			dev->data->min_rx_buf_size = mbp_buf_size;
>> +	}
>> +
>> +	return ret;
>>
>> Where do you set the initial value of min_rx_buf_size?
>> Can't find it by some reason.

>Hum, actually, dev->data structure is supposed to be set to 0 at init 
>time or I missed something.

>I would say this happens once for the whole rte_eth_dev_data array in 
>rte_eth_dev_data_alloc() in primary process (first call to 
>rte_eth_dev_allocate()).

Yes, I understand that it will be initialised to 0 together with whole dev->data.
But then, the condition:
if (dev->data->min_rx_buf_size > mbp_buf_size)
would never be true, and  min_rx_buf_size would always remain 0?
I thought you need to initialise it with UINT32_MAX(or UINT16_MAX).
BTW, not big deal, but I think uint16_t is enough for min_rx_buf_size.

>>
>> 3)  if ((mtu < 68) || (frame_size > dev_info.max_rx_pktlen))
>> Can we add a new define for min allowable MTU (68) as it used in few places.

>RTE_IPV4_MIN_MTU then ?

Sounds good to me.

>I am not sure where this belongs, it could go in rte_ethdev.h.

Probably rte_ether.h?

Konstantin

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

* Re: [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers
  2014-06-17  8:57     ` Ananyev, Konstantin
@ 2014-06-17  9:25       ` Thomas Monjalon
  2014-06-17 11:42         ` Ananyev, Konstantin
  2014-06-17 13:39       ` David Marchand
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas Monjalon @ 2014-06-17  9:25 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: dev

2014-06-17 08:57, Ananyev, Konstantin:
> >> 3)  if ((mtu < 68) || (frame_size > dev_info.max_rx_pktlen))
> >> Can we add a new define for min allowable MTU (68) as it used in few places.
> 
> >RTE_IPV4_MIN_MTU then ?
> 
> Sounds good to me.
> 
> >I am not sure where this belongs, it could go in rte_ethdev.h.
> 
> Probably rte_ether.h?

As you konw, rte_ether.h is for ethernet definition
(and should be located in librte_net).
For RTE_IPV4_MIN_MTU, I think librte_net/rte_ip.h is more appropriate.

-- 
Thomas

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

* Re: [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers
  2014-06-17  9:25       ` Thomas Monjalon
@ 2014-06-17 11:42         ` Ananyev, Konstantin
  0 siblings, 0 replies; 15+ messages in thread
From: Ananyev, Konstantin @ 2014-06-17 11:42 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

>As you konw, rte_ether.h is for ethernet definition
>(and should be located in librte_net).
>For RTE_IPV4_MIN_MTU, I think librte_net/rte_ip.h is more appropriate.

Yes, it is.
Konstantin

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

* Re: [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers
  2014-06-17  8:57     ` Ananyev, Konstantin
  2014-06-17  9:25       ` Thomas Monjalon
@ 2014-06-17 13:39       ` David Marchand
  2014-06-17 15:26         ` Ananyev, Konstantin
  1 sibling, 1 reply; 15+ messages in thread
From: David Marchand @ 2014-06-17 13:39 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev

On 06/17/2014 10:57 AM, Ananyev, Konstantin wrote:
> Yes, I understand that it will be initialised to 0 together with whole dev->data.
> But then, the condition:
> if (dev->data->min_rx_buf_size > mbp_buf_size)
> would never be true, and  min_rx_buf_size would always remain 0?
> I thought you need to initialise it with UINT32_MAX(or UINT16_MAX).
> BTW, not big deal, but I think uint16_t is enough for min_rx_buf_size.

- Oh, right...
We need a check on this :
if (!dev->data->min_rx_buf_size ||
     dev->data->min_rx_buf_size > mbp_buf_size)


- Yep, uint16_t should be enough for min_rx_buf_size, but then, we might 
want to update other places where bufsizes are compared to uin32_t as well.


- Actually, looking at dev->data structure, there is something 
suspicious to me.
 From what I understood, secondary processes are not supposed to touch 
dev->data, at it is shared between processes.
So I don't understand why rte_eth_dev_allocate() writes 
dev->data->port_id, without looking at process type.

Idem, later in rte_eth_dev_init(), where 
eth_dev->data->rx_mbuf_alloc_failed is set to 0 (which should already be 
set to 0 anyway).

I think a cleanup is required here but it can wait until 1.7 is out.
Plus, I am not sure we should let secondary processes use fdir calls, 
change vlan offloads etc...


>
>>>
>>> 3)  if ((mtu < 68) || (frame_size > dev_info.max_rx_pktlen))
>>> Can we add a new define for min allowable MTU (68) as it used in few places.
>
>> RTE_IPV4_MIN_MTU then ?
>
> Sounds good to me.
>
>> I am not sure where this belongs, it could go in rte_ethdev.h.
>
> Probably rte_ether.h?

Ok, I spoke to Ivan and Thomas off-list.
I propose to add the following definition in rte_ether.h :

#define ETHER_MIN_MTU 68
/**< Minimum MTU for IPv4 packets, see RFC 791. */

What do you think of this ?


-- 
David Marchand

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

* Re: [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers
  2014-06-17 13:39       ` David Marchand
@ 2014-06-17 15:26         ` Ananyev, Konstantin
  0 siblings, 0 replies; 15+ messages in thread
From: Ananyev, Konstantin @ 2014-06-17 15:26 UTC (permalink / raw)
  To: David Marchand, dev

>- Actually, looking at dev->data structure, there is something 
>suspicious to me.
>From what I understood, secondary processes are not supposed to touch 
>dev->data, at it is shared between processes.
>So I don't understand why rte_eth_dev_allocate() writes 
>dev->data->port_id, without looking at process type.

It was a while since I looked at that part...
But yes, it doesn't look right to me either.
As I remember, primary and secondary processes supposed to have exactly the same device list.
Probably that's why it was ok so far.

>Idem, later in rte_eth_dev_init(), where 
>eth_dev->data->rx_mbuf_alloc_failed is set to 0 (which should already be 
>set to 0 anyway).

>I think a cleanup is required here but it can wait until 1.7 is out.

Yes, agree.

>Plus, I am not sure we should let secondary processes use fdir calls, 
>change vlan offloads etc...


>Ok, I spoke to Ivan and Thomas off-list.
>I propose to add the following definition in rte_ether.h :

>#define ETHER_MIN_MTU 68
>/**< Minimum MTU for IPv4 packets, see RFC 791. */

>What do you think of this ?

That's fine too.

Konstantin

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

end of thread, other threads:[~2014-06-17 15:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-13 13:37 [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers David Marchand
2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 1/7] ethdev: retrieve flow control configuration David Marchand
2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 2/7] ethdev: add autoneg parameter in flow ctrl accessors David Marchand
2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 3/7] ethdev: store min rx buffer size David Marchand
2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 4/7] ethdev: introduce enable_scatter rx mode David Marchand
2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 5/7] ethdev: add mtu accessors David Marchand
2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 6/7] ixgbe: add set_mtu to ixgbevf David Marchand
2014-06-13 13:37 ` [dpdk-dev] [PATCH v2 7/7] app/testpmd: allow to configure mtu David Marchand
2014-06-16 17:07 ` [dpdk-dev] [PATCH v2 0/7] add mtu and flow control handlers Ananyev, Konstantin
2014-06-17  8:42   ` David Marchand
2014-06-17  8:57     ` Ananyev, Konstantin
2014-06-17  9:25       ` Thomas Monjalon
2014-06-17 11:42         ` Ananyev, Konstantin
2014-06-17 13:39       ` David Marchand
2014-06-17 15:26         ` Ananyev, Konstantin

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