DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] update ixgbe shared code to version 2017-01-05
From: Wei Dai @ 2017-01-10 15:45 UTC (permalink / raw)
  To: dev; +Cc: helin.zhang, konstantin.ananyev, Wei Dai

This patch set updates ixgbe shared code to cid-ixgbe.2017.01.05.tar.gz .
The shared codes are in drivers/net/ixgbe/base/ and are developed by 
another Intel team.

This patch add more supports to some Intel Broadwell-DE platforms.
A new device id and two PHY types are introduced and also 
add some debug traces to help debugging.

Wei Dai (7):
  net/ixgbe/base: support Broadwell-DE XFI backplane
  net/ixgbe/base: remove a compiler warning
  net/ixgbe/base: configure speeds for KR/KX backplane
  net/ixgbe/base: support Marvell 1000BASE-T PHYs
  net/ixgbe/base: use clause 22 MDIO functions for Marvell PHYs
  net/ixgbe/base: add some debug traces
  net/ixgbe/base: update version of shared code

 drivers/net/ixgbe/base/README         |  2 +-
 drivers/net/ixgbe/base/ixgbe_api.c    |  1 +
 drivers/net/ixgbe/base/ixgbe_common.c |  9 +++-
 drivers/net/ixgbe/base/ixgbe_phy.c    | 20 +++++++--
 drivers/net/ixgbe/base/ixgbe_type.h   |  3 ++
 drivers/net/ixgbe/base/ixgbe_x540.c   | 11 ++++-
 drivers/net/ixgbe/base/ixgbe_x550.c   | 77 ++++++++++++++++++++++++++++++-----
 7 files changed, 104 insertions(+), 19 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH 2/7] net/ixgbe/base: remove a compiler warning
From: Wei Dai @ 2017-01-10 15:45 UTC (permalink / raw)
  To: dev; +Cc: helin.zhang, konstantin.ananyev, Wei Dai
In-Reply-To: <1484063146-15796-1-git-send-email-wei.dai@intel.com>

This patch fixes a compiler warning.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c
index 18bb18c..0dd33b8 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.c
+++ b/drivers/net/ixgbe/base/ixgbe_common.c
@@ -1150,7 +1150,7 @@ s32 ixgbe_init_led_link_act_generic(struct ixgbe_hw *hw)
 {
 	struct ixgbe_mac_info *mac = &hw->mac;
 	u32 led_reg, led_mode;
-	u16 i;
+	u8 i;
 
 	led_reg = IXGBE_READ_REG(hw, IXGBE_LEDCTL);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/7] net/ixgbe/base: configure speeds for KR/KX backplane
From: Wei Dai @ 2017-01-10 15:45 UTC (permalink / raw)
  To: dev; +Cc: helin.zhang, konstantin.ananyev, Wei Dai
In-Reply-To: <1484063146-15796-1-git-send-email-wei.dai@intel.com>

This patch ensures that the advertised link speeds are configured
for KR/KX backplane on some new platform. Without this patch the
link remains at 1G when resuming from low power after being
downshifted by LPLU(Low Power Link Up).
This patch ensures that the advertise speeds are not changed
for 2.5G configurations.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_x550.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 2f7f446..b23d479 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -2627,12 +2627,11 @@ s32 ixgbe_init_ext_t_x550em(struct ixgbe_hw *hw)
 /**
  *  ixgbe_setup_kr_x550em - Configure the KR PHY.
  *  @hw: pointer to hardware structure
- *
- *  Configures the integrated KR PHY for X550EM_x.
  **/
 s32 ixgbe_setup_kr_x550em(struct ixgbe_hw *hw)
 {
-	if (hw->mac.type != ixgbe_mac_X550EM_x)
+	/* leave link alone for 2.5G */
+	if (hw->phy.autoneg_advertised & IXGBE_LINK_SPEED_2_5GB_FULL)
 		return IXGBE_SUCCESS;
 
 	return ixgbe_setup_kr_speed_x550em(hw, hw->phy.autoneg_advertised);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 5/7] net/ixgbe/base: use clause 22 MDIO functions for Marvell PHYs
From: Wei Dai @ 2017-01-10 15:45 UTC (permalink / raw)
  To: dev; +Cc: helin.zhang, konstantin.ananyev, Wei Dai
In-Reply-To: <1484063146-15796-1-git-send-email-wei.dai@intel.com>

This patch sets the MDIO(Management Data Input/Output Interface)
read/write function ponters for Broadwell+Marvell PHYs to use
the clause 22 functions. Marvell PHYs do not support clause 45.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_x550.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 1fd7ffb..e15054b 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -2373,6 +2373,10 @@ s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
 		/* set up for CS4227 usage */
 		hw->phy.phy_semaphore_mask = IXGBE_GSSR_SHARED_I2C_SM;
 		break;
+	case IXGBE_DEV_ID_X550EM_X_1G_T:
+		phy->ops.read_reg_mdi = ixgbe_read_phy_reg_mdi_22;
+		phy->ops.write_reg_mdi = ixgbe_write_phy_reg_mdi_22;
+		break;
 	default:
 		break;
 	}
-- 
2.7.4

^ permalink raw reply related

* [PATCH 4/7] net/ixgbe/base: support Marvell 1000BASE-T PHYs
From: Wei Dai @ 2017-01-10 15:45 UTC (permalink / raw)
  To: dev; +Cc: helin.zhang, konstantin.ananyev, Wei Dai
In-Reply-To: <1484063146-15796-1-git-send-email-wei.dai@intel.com>

This patch adds initial support for Marvell 1000BASE-T PHYs
on Broadwell-DE.
Firmware owns the link config for Marvell PHYs on Broadwell-DE,
software should not touch it.
Also Broadwell-DE is not capable of speeds lower than 1Gb.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_phy.c  | 4 ++++
 drivers/net/ixgbe/base/ixgbe_type.h | 1 +
 drivers/net/ixgbe/base/ixgbe_x550.c | 8 ++++++++
 3 files changed, 13 insertions(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index 5ad4dfe..e0d7125 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -472,6 +472,10 @@ enum ixgbe_phy_type ixgbe_get_phy_type_from_id(u32 phy_id)
 	case X557_PHY_ID2:
 		phy_type = ixgbe_phy_x550em_ext_t;
 		break;
+	case IXGBE_M88E1500_E_PHY_ID:
+	case IXGBE_M88E1543_E_PHY_ID:
+		phy_type = ixgbe_phy_ext_1g_t;
+		break;
 	default:
 		phy_type = ixgbe_phy_unknown;
 		break;
diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index dea210e..bb1f85b 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -3647,6 +3647,7 @@ enum ixgbe_phy_type {
 	ixgbe_phy_x550em_kx4,
 	ixgbe_phy_x550em_xfi,
 	ixgbe_phy_x550em_ext_t,
+	ixgbe_phy_ext_1g_t,
 	ixgbe_phy_cu_unknown,
 	ixgbe_phy_qt,
 	ixgbe_phy_xaui,
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index b23d479..1fd7ffb 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1975,6 +1975,7 @@ s32 ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
 			*speed = IXGBE_LINK_SPEED_10GB_FULL;
 	} else {
 		switch (hw->phy.type) {
+		case ixgbe_phy_ext_1g_t:
 		case ixgbe_phy_sgmii:
 			*speed = IXGBE_LINK_SPEED_1GB_FULL;
 			break;
@@ -2399,6 +2400,10 @@ s32 ixgbe_init_phy_ops_X550em(struct ixgbe_hw *hw)
 		phy->ops.read_reg = ixgbe_read_phy_reg_x550em;
 		phy->ops.write_reg = ixgbe_write_phy_reg_x550em;
 		break;
+	case ixgbe_phy_ext_1g_t:
+		/* link is managed by FW */
+		phy->ops.setup_link = NULL;
+		break;
 	case ixgbe_phy_x550em_xfi:
 		/* link is managed by HW */
 		phy->ops.setup_link = NULL;
@@ -3706,6 +3711,9 @@ u32 ixgbe_get_supported_physical_layer_X550em(struct ixgbe_hw *hw)
 	case ixgbe_phy_sgmii:
 		physical_layer = IXGBE_PHYSICAL_LAYER_1000BASE_KX;
 		break;
+	case ixgbe_phy_ext_1g_t:
+		physical_layer |= IXGBE_PHYSICAL_LAYER_1000BASE_T;
+		break;
 	default:
 		break;
 	}
-- 
2.7.4

^ permalink raw reply related

* [PATCH 7/7] net/ixgbe/base: update version of shared code
From: Wei Dai @ 2017-01-10 15:45 UTC (permalink / raw)
  To: dev; +Cc: helin.zhang, konstantin.ananyev, Wei Dai
In-Reply-To: <1484063146-15796-1-git-send-email-wei.dai@intel.com>

update version of shared code to 2017-01-05 in README.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/base/README b/drivers/net/ixgbe/base/README
index 1936191..0a6054f 100644
--- a/drivers/net/ixgbe/base/README
+++ b/drivers/net/ixgbe/base/README
@@ -34,7 +34,7 @@ Intel® IXGBE driver
 ===================
 
 This directory contains source code of FreeBSD ixgbe driver of version
-cid-10g-shared-code.2016.11.21 released by the team which develop
+cid-10g-shared-code.2017.01.05 released by the team which develop
 basic drivers for any ixgbe NIC. The sub-directory of base/
 contains the original source package.
 This driver is valid for the product(s) listed below
-- 
2.7.4

^ permalink raw reply related

* [PATCH 6/7] net/ixgbe/base: add some debug traces
From: Wei Dai @ 2017-01-10 15:45 UTC (permalink / raw)
  To: dev; +Cc: helin.zhang, konstantin.ananyev, Wei Dai
In-Reply-To: <1484063146-15796-1-git-send-email-wei.dai@intel.com>

This patch adds some traces in the reset_hw logic
and semaphore acquisition logic to help debugging.

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_common.c |  7 ++++-
 drivers/net/ixgbe/base/ixgbe_phy.c    | 16 +++++++++---
 drivers/net/ixgbe/base/ixgbe_x540.c   | 11 ++++++--
 drivers/net/ixgbe/base/ixgbe_x550.c   | 49 +++++++++++++++++++++++++++++------
 4 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c
index 0dd33b8..9645667 100644
--- a/drivers/net/ixgbe/base/ixgbe_common.c
+++ b/drivers/net/ixgbe/base/ixgbe_common.c
@@ -410,8 +410,10 @@ s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw)
 
 	/* Setup flow control */
 	ret_val = ixgbe_setup_fc(hw);
-	if (ret_val != IXGBE_SUCCESS && ret_val != IXGBE_NOT_IMPLEMENTED)
+	if (ret_val != IXGBE_SUCCESS && ret_val != IXGBE_NOT_IMPLEMENTED) {
+		DEBUGOUT1("Flow control setup failed, returning %d\n", ret_val);
 		return ret_val;
+	}
 
 	/* Cache bit indicating need for crosstalk fix */
 	switch (hw->mac.type) {
@@ -501,6 +503,9 @@ s32 ixgbe_init_hw_generic(struct ixgbe_hw *hw)
 	/* Initialize the LED link active for LED blink support */
 	hw->mac.ops.init_led_link_act(hw);
 
+	if (status != IXGBE_SUCCESS)
+		DEBUGOUT1("Failed to initialize HW, STATUS = %d\n", status);
+
 	return status;
 }
 
diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index e0d7125..c953805 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -291,8 +291,11 @@ static bool ixgbe_probe_phy(struct ixgbe_hw *hw, u16 phy_addr)
 {
 	u16 ext_ability = 0;
 
-	if (!ixgbe_validate_phy_addr(hw, phy_addr))
+	if (!ixgbe_validate_phy_addr(hw, phy_addr)) {
+		DEBUGOUT1("Unable to validate PHY address 0x%04X\n",
+			phy_addr);
 		return false;
+	}
 
 	if (ixgbe_get_phy_id(hw))
 		return false;
@@ -411,6 +414,8 @@ bool ixgbe_validate_phy_addr(struct ixgbe_hw *hw, u32 phy_addr)
 	if (phy_id != 0xFFFF && phy_id != 0x0)
 		valid = true;
 
+	DEBUGOUT1("PHY ID HIGH is 0x%04X\n", phy_id);
+
 	return valid;
 }
 
@@ -439,6 +444,9 @@ s32 ixgbe_get_phy_id(struct ixgbe_hw *hw)
 		hw->phy.id |= (u32)(phy_id_low & IXGBE_PHY_REVISION_MASK);
 		hw->phy.revision = (u32)(phy_id_low & ~IXGBE_PHY_REVISION_MASK);
 	}
+	DEBUGOUT2("PHY_ID_HIGH 0x%04X, PHY_ID_LOW 0x%04X\n",
+		  phy_id_high, phy_id_low);
+
 	return status;
 }
 
@@ -570,7 +578,7 @@ s32 ixgbe_reset_phy_generic(struct ixgbe_hw *hw)
  *  @phy_data: Pointer to read data from PHY register
  **/
 s32 ixgbe_read_phy_reg_mdi(struct ixgbe_hw *hw, u32 reg_addr, u32 device_type,
-		       u16 *phy_data)
+			   u16 *phy_data)
 {
 	u32 i, data, command;
 
@@ -592,12 +600,13 @@ s32 ixgbe_read_phy_reg_mdi(struct ixgbe_hw *hw, u32 reg_addr, u32 device_type,
 
 		command = IXGBE_READ_REG(hw, IXGBE_MSCA);
 		if ((command & IXGBE_MSCA_MDI_COMMAND) == 0)
-				break;
+			break;
 	}
 
 
 	if ((command & IXGBE_MSCA_MDI_COMMAND) != 0) {
 		ERROR_REPORT1(IXGBE_ERROR_POLLING, "PHY address command did not complete.\n");
+		DEBUGOUT("PHY address command did not complete, returning IXGBE_ERR_PHY\n");
 		return IXGBE_ERR_PHY;
 	}
 
@@ -627,6 +636,7 @@ s32 ixgbe_read_phy_reg_mdi(struct ixgbe_hw *hw, u32 reg_addr, u32 device_type,
 
 	if ((command & IXGBE_MSCA_MDI_COMMAND) != 0) {
 		ERROR_REPORT1(IXGBE_ERROR_POLLING, "PHY read command didn't complete\n");
+		DEBUGOUT("PHY read command didn't complete, returning IXGBE_ERR_PHY\n");
 		return IXGBE_ERR_PHY;
 	}
 
diff --git a/drivers/net/ixgbe/base/ixgbe_x540.c b/drivers/net/ixgbe/base/ixgbe_x540.c
index 49bf154..499b1fa 100644
--- a/drivers/net/ixgbe/base/ixgbe_x540.c
+++ b/drivers/net/ixgbe/base/ixgbe_x540.c
@@ -775,8 +775,10 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
 		/* SW NVM semaphore bit is used for access to all
 		 * SW_FW_SYNC bits (not just NVM)
 		 */
-		if (ixgbe_get_swfw_sync_semaphore(hw))
+		if (ixgbe_get_swfw_sync_semaphore(hw)) {
+			DEBUGOUT("Failed to get NVM access and register semaphore, returning IXGBE_ERR_SWFW_SYNC\n");
 			return IXGBE_ERR_SWFW_SYNC;
+		}
 
 		swfw_sync = IXGBE_READ_REG(hw, IXGBE_SWFW_SYNC_BY_MAC(hw));
 		if (!(swfw_sync & (fwmask | swmask | hwmask))) {
@@ -798,6 +800,7 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
 	if (swmask == IXGBE_GSSR_SW_MNG_SM) {
 		ERROR_REPORT1(IXGBE_ERROR_POLLING,
 			     "Failed to get SW only semaphore");
+		DEBUGOUT("Failed to get SW only semaphore, returning IXGBE_ERR_SWFW_SYNC\n");
 		return IXGBE_ERR_SWFW_SYNC;
 	}
 
@@ -806,8 +809,10 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
 	 * of the requested resource(s) while ignoring the corresponding FW/HW
 	 * bits in the SW_FW_SYNC register.
 	 */
-	if (ixgbe_get_swfw_sync_semaphore(hw))
+	if (ixgbe_get_swfw_sync_semaphore(hw)) {
+		DEBUGOUT("Failed to get NVM sempahore and register semaphore while forcefully ignoring FW sempahore bit(s) and setting SW semaphore bit(s), returning IXGBE_ERR_SWFW_SYNC\n");
 		return IXGBE_ERR_SWFW_SYNC;
+	}
 	swfw_sync = IXGBE_READ_REG(hw, IXGBE_SWFW_SYNC_BY_MAC(hw));
 	if (swfw_sync & (fwmask | hwmask)) {
 		swfw_sync |= swmask;
@@ -829,9 +834,11 @@ s32 ixgbe_acquire_swfw_sync_X540(struct ixgbe_hw *hw, u32 mask)
 			rmask |= IXGBE_GSSR_I2C_MASK;
 		ixgbe_release_swfw_sync_X540(hw, rmask);
 		ixgbe_release_swfw_sync_semaphore(hw);
+		DEBUGOUT("Resource not released by other SW, returning IXGBE_ERR_SWFW_SYNC\n");
 		return IXGBE_ERR_SWFW_SYNC;
 	}
 	ixgbe_release_swfw_sync_semaphore(hw);
+	DEBUGOUT("Returning error IXGBE_ERR_SWFW_SYNC\n");
 
 	return IXGBE_ERR_SWFW_SYNC;
 }
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index e15054b..6f9c034 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1311,13 +1311,20 @@ s32 ixgbe_get_phy_token(struct ixgbe_hw *hw)
 					      sizeof(token_cmd),
 					      IXGBE_HI_COMMAND_TIMEOUT,
 					      true);
-	if (status)
+	if (status) {
+		DEBUGOUT1("Issuing host interface command failed with Status = %d\n",
+			  status);
 		return status;
+	}
 	if (token_cmd.hdr.cmd_or_resp.ret_status == FW_PHY_TOKEN_OK)
 		return IXGBE_SUCCESS;
-	if (token_cmd.hdr.cmd_or_resp.ret_status != FW_PHY_TOKEN_RETRY)
+	if (token_cmd.hdr.cmd_or_resp.ret_status != FW_PHY_TOKEN_RETRY) {
+		DEBUGOUT1("Host interface command returned 0x%08x , returning IXGBE_ERR_FW_RESP_INVALID\n",
+			  token_cmd.hdr.cmd_or_resp.ret_status);
 		return IXGBE_ERR_FW_RESP_INVALID;
+	}
 
+	DEBUGOUT("Returning  IXGBE_ERR_TOKEN_RETRY\n");
 	return IXGBE_ERR_TOKEN_RETRY;
 }
 
@@ -2495,9 +2502,10 @@ s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
 
 	/* Call adapter stop to disable Tx/Rx and clear interrupts */
 	status = hw->mac.ops.stop_adapter(hw);
-	if (status != IXGBE_SUCCESS)
+	if (status != IXGBE_SUCCESS) {
+		DEBUGOUT1("Failed to stop adapter, STATUS = %d\n", status);
 		return status;
-
+	}
 	/* flush pending Tx transactions */
 	ixgbe_clear_tx_pending(hw);
 
@@ -2506,14 +2514,23 @@ s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
 	/* PHY ops must be identified and initialized prior to reset */
 	status = hw->phy.ops.init(hw);
 
-	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED)
+	if (status)
+		DEBUGOUT1("Failed to initialize PHY ops, STATUS = %d\n",
+			  status);
+
+	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
+		DEBUGOUT("Returning from reset HW since PHY ops init returned IXGBE_ERR_SFP_NOT_SUPPORTED\n");
 		return status;
+	}
 
 	/* start the external PHY */
 	if (hw->phy.type == ixgbe_phy_x550em_ext_t) {
 		status = ixgbe_init_ext_t_x550em(hw);
-		if (status)
+		if (status) {
+			DEBUGOUT1("Failed to start the external PHY, STATUS = %d\n",
+				  status);
 			return status;
+		}
 	}
 
 	/* Setup SFP module if there is one present. */
@@ -2587,6 +2604,9 @@ s32 ixgbe_reset_hw_X550em(struct ixgbe_hw *hw)
 	if (hw->device_id == IXGBE_DEV_ID_X550EM_X_SFP)
 		ixgbe_setup_mux_ctl(hw);
 
+	if (status != IXGBE_SUCCESS)
+		DEBUGOUT1("Reset HW failed, STATUS = %d\n", status);
+
 	return status;
 }
 
@@ -4336,21 +4356,34 @@ STATIC s32 ixgbe_acquire_swfw_sync_X550a(struct ixgbe_hw *hw, u32 mask)
 		status = IXGBE_SUCCESS;
 		if (hmask)
 			status = ixgbe_acquire_swfw_sync_X540(hw, hmask);
-		if (status)
+		if (status) {
+			DEBUGOUT1("Could not acquire SWFW semaphore, Status = %d\n",
+				  status);
 			return status;
+		}
 		if (!(mask & IXGBE_GSSR_TOKEN_SM))
 			return IXGBE_SUCCESS;
 
 		status = ixgbe_get_phy_token(hw);
+		if (status == IXGBE_ERR_TOKEN_RETRY)
+			DEBUGOUT1("Could not acquire PHY token, Status = %d\n",
+				  status);
+
 		if (status == IXGBE_SUCCESS)
 			return IXGBE_SUCCESS;
 
 		if (hmask)
 			ixgbe_release_swfw_sync_X540(hw, hmask);
-		if (status != IXGBE_ERR_TOKEN_RETRY)
+
+		if (status != IXGBE_ERR_TOKEN_RETRY) {
+			DEBUGOUT1("Unable to retry acquiring the PHY token, Status = %d\n",
+				  status);
 			return status;
+		}
 	}
 
+	DEBUGOUT1("Semaphore acquisition retries failed!: PHY ID = 0x%08X\n",
+		  hw->phy.id);
 	return status;
 }
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 7/8] ethdev: break ethernet driver and pci_driver connection
From: Jan Blunck @ 2017-01-10 16:11 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Stephen Hemminger
In-Reply-To: <20170107181756.1944-8-sthemmin@microsoft.com>

On Sat, Jan 7, 2017 at 7:17 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> There are multiple buses and device types now. Therefore it no longer
> makes sense that PCI driver information is part of the Ethernet driver
> structure.

The Ethernet driver itself doesn't over alot of value from an
abstraction point of view. Its questionable if there ever will be an
Ethernet driver that is able to operate on different types of
low-level devices. The virtual devices are anyway able to operate
without an Ethernet driver structure. Most of that functionality
should get moved either into the bus abstraction or the low-level
device probe function.


>
> This patch removes pci_driver from eth_driver and introduces a
> new combined structure for use in all existing PMD's. The rationale
> is that although all existing PCI drivers are Ethernet drivers,
> it make sense that future projects may want to support PCI devices
> that are not Ethernet.
>
> It also removes the requirement that driver is first element in
> PCI driver structure.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  app/test/virtual_pmd.c                  | 22 ++++++++---------
>  drivers/net/bnx2x/bnx2x_ethdev.c        | 16 ++++++++-----
>  drivers/net/bnxt/bnxt_ethdev.c          | 22 +++++++++--------
>  drivers/net/cxgbe/cxgbe_ethdev.c        |  8 ++++---
>  drivers/net/e1000/em_ethdev.c           | 10 ++++----
>  drivers/net/e1000/igb_ethdev.c          | 20 +++++++++-------
>  drivers/net/ena/ena_ethdev.c            |  8 ++++---
>  drivers/net/enic/enic_ethdev.c          |  8 ++++---
>  drivers/net/fm10k/fm10k_ethdev.c        | 10 ++++----
>  drivers/net/i40e/i40e_ethdev.c          | 10 ++++----
>  drivers/net/i40e/i40e_ethdev_vf.c       | 10 ++++----
>  drivers/net/ixgbe/ixgbe_ethdev.c        | 20 +++++++++-------
>  drivers/net/mlx4/mlx4.c                 |  8 ++++---
>  drivers/net/mlx5/mlx5.c                 |  8 ++++---
>  drivers/net/nfp/nfp_net.c               |  8 ++++---
>  drivers/net/qede/qede_ethdev.c          | 42 +++++++++++++++++----------------
>  drivers/net/szedata2/rte_eth_szedata2.c | 10 ++++----
>  drivers/net/thunderx/nicvf_ethdev.c     |  8 ++++---
>  drivers/net/virtio/virtio_ethdev.c      | 10 ++++----
>  drivers/net/vmxnet3/vmxnet3_ethdev.c    | 10 ++++----
>  lib/librte_ether/rte_ethdev.c           |  9 +++----
>  lib/librte_ether/rte_ethdev.h           | 18 +++++++++-----
>  22 files changed, 172 insertions(+), 123 deletions(-)
>
> diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c
> index 6e4dcd8f..e7f56527 100644
> --- a/app/test/virtual_pmd.c
> +++ b/app/test/virtual_pmd.c
> @@ -533,7 +533,7 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
>         struct rte_pci_device *pci_dev = NULL;
>         struct rte_eth_dev *eth_dev = NULL;
>         struct eth_driver *eth_drv = NULL;
> -       struct rte_pci_driver *pci_drv = NULL;
> +       struct rte_pci_eth_driver *pci_eth_drv = NULL;
>         struct rte_pci_id *id_table = NULL;
>         struct virtual_ethdev_private *dev_private = NULL;
>         char name_buf[RTE_RING_NAMESIZE];
> @@ -554,8 +554,8 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
>         if (eth_drv == NULL)
>                 goto err;
>
> -       pci_drv = rte_zmalloc_socket(name, sizeof(*pci_drv), 0, socket_id);
> -       if (pci_drv == NULL)
> +       pci_eth_drv = rte_zmalloc_socket(name, sizeof(*pci_eth_drv), 0, socket_id);
> +       if (pci_eth_drv == NULL)
>                 goto err;
>
>         id_table = rte_zmalloc_socket(name, sizeof(*id_table), 0, socket_id);
> @@ -585,17 +585,15 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
>                 goto err;
>
>         pci_dev->device.numa_node = socket_id;
> -       pci_drv->driver.name = virtual_ethdev_driver_name;
> -       pci_drv->id_table = id_table;
> +       pci_eth_drv->pci_drv.driver.name = virtual_ethdev_driver_name;
> +       pci_eth_drv->pci_drv.id_table = id_table;
>
>         if (isr_support)
> -               pci_drv->drv_flags |= RTE_PCI_DRV_INTR_LSC;
> +               pci_eth_drv->pci_drv.drv_flags |= RTE_PCI_DRV_INTR_LSC;
>         else
> -               pci_drv->drv_flags &= ~RTE_PCI_DRV_INTR_LSC;
> +               pci_eth_drv->pci_drv.drv_flags &= ~RTE_PCI_DRV_INTR_LSC;
>
> -
> -       eth_drv->pci_drv = (struct rte_pci_driver)(*pci_drv);
> -       eth_dev->driver = eth_drv;
> +       eth_dev->driver = &pci_eth_drv->eth_drv;
>
>         eth_dev->data->nb_rx_queues = (uint16_t)1;
>         eth_dev->data->nb_tx_queues = (uint16_t)1;
> @@ -622,7 +620,7 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
>         dev_private->dev_ops = virtual_ethdev_default_dev_ops;
>         eth_dev->dev_ops = &dev_private->dev_ops;
>
> -       pci_dev->device.driver = &eth_drv->pci_drv.driver;
> +       pci_dev->device.driver = &pci_eth_drv->pci_drv.driver;
>         eth_dev->device = &pci_dev->device;
>
>         eth_dev->rx_pkt_burst = virtual_ethdev_rx_burst_success;
> @@ -632,7 +630,7 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
>
>  err:
>         rte_free(pci_dev);
> -       rte_free(pci_drv);
> +       rte_free(pci_eth_drv);
>         rte_free(eth_drv);
>         rte_free(id_table);
>         rte_free(dev_private);
> diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
> index 7140118f..ef704d72 100644
> --- a/drivers/net/bnx2x/bnx2x_ethdev.c
> +++ b/drivers/net/bnx2x/bnx2x_ethdev.c
> @@ -618,29 +618,33 @@ eth_bnx2xvf_dev_init(struct rte_eth_dev *eth_dev)
>         return bnx2x_common_dev_init(eth_dev, 1);
>  }
>
> -static struct eth_driver rte_bnx2x_pmd = {
> +static struct rte_pci_eth_driver rte_bnx2x_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_bnx2x_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_bnx2x_dev_init,
> -       .dev_private_size = sizeof(struct bnx2x_softc),
> +       eth_drv = {
> +               .eth_dev_init = eth_bnx2x_dev_init,
> +               .dev_private_size = sizeof(struct bnx2x_softc),
> +       },
>  };
>
>  /*
>   * virtual function driver struct
>   */
> -static struct eth_driver rte_bnx2xvf_pmd = {
> +static struct rte_pci_eth_driver rte_bnx2xvf_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_bnx2xvf_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_bnx2xvf_dev_init,
> -       .dev_private_size = sizeof(struct bnx2x_softc),
> +       eth_drv = {
> +               .eth_dev_init = eth_bnx2xvf_dev_init,
> +               .dev_private_size = sizeof(struct bnx2x_softc),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_bnx2x, rte_bnx2x_pmd.pci_drv);
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 7518b6b7..9017825b 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -1164,17 +1164,19 @@ bnxt_dev_uninit(struct rte_eth_dev *eth_dev) {
>         return rc;
>  }
>
> -static struct eth_driver bnxt_rte_pmd = {
> +static struct rte_pci_eth_driver bnxt_rte_pmd = {
>         .pci_drv = {
> -                   .id_table = bnxt_pci_id_map,
> -                   .drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> -                           RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_INTR_LSC,
> -                   .probe = rte_eth_dev_pci_probe,
> -                   .remove = rte_eth_dev_pci_remove
> -                   },
> -       .eth_dev_init = bnxt_dev_init,
> -       .eth_dev_uninit = bnxt_dev_uninit,
> -       .dev_private_size = sizeof(struct bnxt),
> +               .id_table = bnxt_pci_id_map,
> +               .drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> +                            RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_INTR_LSC,
> +               .probe = rte_eth_dev_pci_probe,
> +               .remove = rte_eth_dev_pci_remove
> +       },
> +       .eth_drv = {
> +               .eth_dev_init = bnxt_dev_init,
> +               .eth_dev_uninit = bnxt_dev_uninit,
> +               .dev_private_size = sizeof(struct bnxt),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_bnxt, bnxt_rte_pmd.pci_drv);
> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
> index 64345e37..ccf93904 100644
> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -1039,15 +1039,17 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
>         return err;
>  }
>
> -static struct eth_driver rte_cxgbe_pmd = {
> +static struct rte_pci_eth_driver rte_cxgbe_pmd = {
>         .pci_drv = {
>                 .id_table = cxgb4_pci_tbl,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_cxgbe_dev_init,
> -       .dev_private_size = sizeof(struct port_info),
> +       .eth_drv = {
> +               .eth_dev_init = eth_cxgbe_dev_init,
> +               .dev_private_size = sizeof(struct port_info),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_cxgbe, rte_cxgbe_pmd.pci_drv);
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 5f6e66dd..5b87d729 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -389,7 +389,7 @@ eth_em_dev_uninit(struct rte_eth_dev *eth_dev)
>         return 0;
>  }
>
> -static struct eth_driver rte_em_pmd = {
> +static struct rte_pci_eth_driver rte_em_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_em_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> @@ -397,9 +397,11 @@ static struct eth_driver rte_em_pmd = {
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_em_dev_init,
> -       .eth_dev_uninit = eth_em_dev_uninit,
> -       .dev_private_size = sizeof(struct e1000_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = eth_em_dev_init,
> +               .eth_dev_uninit = eth_em_dev_uninit,
> +               .dev_private_size = sizeof(struct e1000_adapter),
> +       },
>  };
>
>  static int
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 2bb57f54..4a2d3b3f 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -1082,7 +1082,7 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
>         return 0;
>  }
>
> -static struct eth_driver rte_igb_pmd = {
> +static struct rte_pci_eth_driver rte_igb_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_igb_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> @@ -1090,24 +1090,28 @@ static struct eth_driver rte_igb_pmd = {
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_igb_dev_init,
> -       .eth_dev_uninit = eth_igb_dev_uninit,
> -       .dev_private_size = sizeof(struct e1000_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = eth_igb_dev_init,
> +               .eth_dev_uninit = eth_igb_dev_uninit,
> +               .dev_private_size = sizeof(struct e1000_adapter),
> +       },
>  };
>
>  /*
>   * virtual function driver struct
>   */
> -static struct eth_driver rte_igbvf_pmd = {
> +static struct rte_pci_eth_driver rte_igbvf_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_igbvf_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_igbvf_dev_init,
> -       .eth_dev_uninit = eth_igbvf_dev_uninit,
> -       .dev_private_size = sizeof(struct e1000_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = eth_igbvf_dev_init,
> +               .eth_dev_uninit = eth_igbvf_dev_uninit,
> +               .dev_private_size = sizeof(struct e1000_adapter),
> +       },
>  };
>
>  static void
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index e99bf299..d6406fa1 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1756,15 +1756,17 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>         return sent_idx;
>  }
>
> -static struct eth_driver rte_ena_pmd = {
> +static struct rte_pci_eth_driver rte_ena_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_ena_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_ena_dev_init,
> -       .dev_private_size = sizeof(struct ena_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = eth_ena_dev_init,
> +               .dev_private_size = sizeof(struct ena_adapter),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_ena, rte_ena_pmd.pci_drv);
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index e5ceb98e..b47975d1 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -633,15 +633,17 @@ static int eth_enicpmd_dev_init(struct rte_eth_dev *eth_dev)
>         return enic_probe(enic);
>  }
>
> -static struct eth_driver rte_enic_pmd = {
> +static struct rte_pci_eth_driver rte_enic_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_enic_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_enicpmd_dev_init,
> -       .dev_private_size = sizeof(struct enic),
> +       .eth_drv = {
> +               .eth_dev_init = eth_enicpmd_dev_init,
> +               .dev_private_size = sizeof(struct enic),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_enic, rte_enic_pmd.pci_drv);
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index d8353e9d..4dea1fd6 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -3077,7 +3077,7 @@ static const struct rte_pci_id pci_id_fm10k_map[] = {
>         { .vendor_id = 0, /* sentinel */ },
>  };
>
> -static struct eth_driver rte_pmd_fm10k = {
> +static struct rte_pci_eth_driver rte_pmd_fm10k = {
>         .pci_drv = {
>                 .id_table = pci_id_fm10k_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> @@ -3085,9 +3085,11 @@ static struct eth_driver rte_pmd_fm10k = {
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_fm10k_dev_init,
> -       .eth_dev_uninit = eth_fm10k_dev_uninit,
> -       .dev_private_size = sizeof(struct fm10k_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = eth_fm10k_dev_init,
> +               .eth_dev_uninit = eth_fm10k_dev_uninit,
> +               .dev_private_size = sizeof(struct fm10k_adapter),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_fm10k, rte_pmd_fm10k.pci_drv);
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 0eb4c990..8b4c6079 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -668,7 +668,7 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
>  #define I40E_NB_TXQ_PRIO_XSTATS (sizeof(rte_i40e_txq_prio_strings) / \
>                 sizeof(rte_i40e_txq_prio_strings[0]))
>
> -static struct eth_driver rte_i40e_pmd = {
> +static struct rte_pci_eth_driver rte_i40e_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_i40e_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> @@ -676,9 +676,11 @@ static struct eth_driver rte_i40e_pmd = {
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_i40e_dev_init,
> -       .eth_dev_uninit = eth_i40e_dev_uninit,
> -       .dev_private_size = sizeof(struct i40e_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = eth_i40e_dev_init,
> +               .eth_dev_uninit = eth_i40e_dev_uninit,
> +               .dev_private_size = sizeof(struct i40e_adapter),
> +       },
>  };
>
>  static inline int
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index 0dc0af52..6dbcc88c 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1526,16 +1526,18 @@ i40evf_dev_uninit(struct rte_eth_dev *eth_dev)
>  /*
>   * virtual function driver struct
>   */
> -static struct eth_driver rte_i40evf_pmd = {
> +static struct rte_pci_eth_driver rte_i40evf_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_i40evf_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = i40evf_dev_init,
> -       .eth_dev_uninit = i40evf_dev_uninit,
> -       .dev_private_size = sizeof(struct i40e_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = i40evf_dev_init,
> +               .eth_dev_uninit = i40evf_dev_uninit,
> +               .dev_private_size = sizeof(struct i40e_adapter),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_i40e_vf, rte_i40evf_pmd.pci_drv);
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 060772d4..6fdf227e 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1563,7 +1563,7 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
>         return 0;
>  }
>
> -static struct eth_driver rte_ixgbe_pmd = {
> +static struct rte_pci_eth_driver rte_ixgbe_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_ixgbe_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> @@ -1571,24 +1571,28 @@ static struct eth_driver rte_ixgbe_pmd = {
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_ixgbe_dev_init,
> -       .eth_dev_uninit = eth_ixgbe_dev_uninit,
> -       .dev_private_size = sizeof(struct ixgbe_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = eth_ixgbe_dev_init,
> +               .eth_dev_uninit = eth_ixgbe_dev_uninit,
> +               .dev_private_size = sizeof(struct ixgbe_adapter),
> +       },
>  };
>
>  /*
>   * virtual function driver struct
>   */
> -static struct eth_driver rte_ixgbevf_pmd = {
> +static struct rte_pci_eth_driver rte_ixgbevf_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_ixgbevf_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_ixgbevf_dev_init,
> -       .eth_dev_uninit = eth_ixgbevf_dev_uninit,
> -       .dev_private_size = sizeof(struct ixgbe_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = eth_ixgbevf_dev_init,
> +               .eth_dev_uninit = eth_ixgbevf_dev_uninit,
> +               .dev_private_size = sizeof(struct ixgbe_adapter),
> +       },
>  };
>
>  static int
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index eb06f56a..7b184019 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -5524,7 +5524,7 @@ priv_dev_interrupt_handler_install(struct priv *priv, struct rte_eth_dev *dev)
>         }
>  }
>
> -static struct eth_driver mlx4_driver;
> +static struct rte_pci_eth_driver mlx4_driver;
>
>  /**
>   * DPDK callback to register a PCI device.
> @@ -5903,7 +5903,7 @@ static const struct rte_pci_id mlx4_pci_id_map[] = {
>         }
>  };
>
> -static struct eth_driver mlx4_driver = {
> +static struct rte_pci_eth_driver mlx4_driver = {
>         .pci_drv = {
>                 .driver = {
>                         .name = MLX4_DRIVER_NAME
> @@ -5912,7 +5912,9 @@ static struct eth_driver mlx4_driver = {
>                 .probe = mlx4_pci_probe,
>                 .drv_flags = RTE_PCI_DRV_INTR_LSC,
>         },
> -       .dev_private_size = sizeof(struct priv)
> +       .eth_drv = {
> +               .dev_private_size = sizeof(struct priv),
> +       },
>  };
>
>  /**
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index b97b6d16..efc0430c 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -338,7 +338,7 @@ mlx5_args(struct priv *priv, struct rte_devargs *devargs)
>         return 0;
>  }
>
> -static struct eth_driver mlx5_driver;
> +static struct rte_pci_eth_driver mlx5_driver;
>
>  /**
>   * DPDK callback to register a PCI device.
> @@ -723,7 +723,7 @@ static const struct rte_pci_id mlx5_pci_id_map[] = {
>         }
>  };
>
> -static struct eth_driver mlx5_driver = {
> +static struct rte_pci_eth_driver mlx5_driver = {
>         .pci_drv = {
>                 .driver = {
>                         .name = MLX5_DRIVER_NAME
> @@ -732,7 +732,9 @@ static struct eth_driver mlx5_driver = {
>                 .probe = mlx5_pci_probe,
>                 .drv_flags = RTE_PCI_DRV_INTR_LSC,
>         },
> -       .dev_private_size = sizeof(struct priv)
> +       .eth_drv = {
> +               .dev_private_size = sizeof(struct priv),
> +       },
>  };
>
>  /**
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index 970b5c84..f5c6634f 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -2470,7 +2470,7 @@ static struct rte_pci_id pci_id_nfp_net_map[] = {
>         },
>  };
>
> -static struct eth_driver rte_nfp_net_pmd = {
> +static struct rte_pci_eth_driver rte_nfp_net_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_nfp_net_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
> @@ -2478,8 +2478,10 @@ static struct eth_driver rte_nfp_net_pmd = {
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = nfp_net_init,
> -       .dev_private_size = sizeof(struct nfp_net_adapter),
> +       .eth_drv = {
> +               .eth_dev_init = nfp_net_init,
> +               .dev_private_size = sizeof(struct nfp_net_adapter),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_nfp, rte_nfp_net_pmd.pci_drv);
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index edc5b43b..13d76a6d 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -1643,30 +1643,32 @@ static struct rte_pci_id pci_id_qede_map[] = {
>         {.vendor_id = 0,}
>  };
>
> -static struct eth_driver rte_qedevf_pmd = {
> +static struct rte_pci_eth_driver rte_qedevf_pmd = {
>         .pci_drv = {
> -                   .id_table = pci_id_qedevf_map,
> -                   .drv_flags =
> -                   RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> -                   .probe = rte_eth_dev_pci_probe,
> -                   .remove = rte_eth_dev_pci_remove,
> -                  },
> -       .eth_dev_init = qedevf_eth_dev_init,
> -       .eth_dev_uninit = qedevf_eth_dev_uninit,
> -       .dev_private_size = sizeof(struct qede_dev),
> +               .id_table = pci_id_qedevf_map,
> +               .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> +               .probe = rte_eth_dev_pci_probe,
> +               .remove = rte_eth_dev_pci_remove,
> +       },
> +       .eth_drv = {
> +               .eth_dev_init = qedevf_eth_dev_init,
> +               .eth_dev_uninit = qedevf_eth_dev_uninit,
> +               .dev_private_size = sizeof(struct qede_dev),
> +       },
>  };
>
> -static struct eth_driver rte_qede_pmd = {
> +static struct rte_pci_eth_driver rte_qede_pmd = {
>         .pci_drv = {
> -                   .id_table = pci_id_qede_map,
> -                   .drv_flags =
> -                   RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> -                   .probe = rte_eth_dev_pci_probe,
> -                   .remove = rte_eth_dev_pci_remove,
> -                  },
> -       .eth_dev_init = qede_eth_dev_init,
> -       .eth_dev_uninit = qede_eth_dev_uninit,
> -       .dev_private_size = sizeof(struct qede_dev),
> +               .id_table = pci_id_qede_map,
> +               .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
> +               .probe = rte_eth_dev_pci_probe,
> +               .remove = rte_eth_dev_pci_remove,
> +       },
> +       .eth_drv = {
> +               .eth_dev_init = qede_eth_dev_init,
> +               .eth_dev_uninit = qede_eth_dev_uninit,
> +               .dev_private_size = sizeof(struct qede_dev),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_qede, rte_qede_pmd.pci_drv);
> diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
> index fe7a6b3b..b9054671 100644
> --- a/drivers/net/szedata2/rte_eth_szedata2.c
> +++ b/drivers/net/szedata2/rte_eth_szedata2.c
> @@ -1587,15 +1587,17 @@ static const struct rte_pci_id rte_szedata2_pci_id_table[] = {
>         }
>  };
>
> -static struct eth_driver szedata2_eth_driver = {
> +static struct rte_pci_eth_driver szedata2_eth_driver = {
>         .pci_drv = {
>                 .id_table = rte_szedata2_pci_id_table,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init     = rte_szedata2_eth_dev_init,
> -       .eth_dev_uninit   = rte_szedata2_eth_dev_uninit,
> -       .dev_private_size = sizeof(struct pmd_internals),
> +       .eth_drv = {
> +               .eth_dev_init     = rte_szedata2_eth_dev_init,
> +               .eth_dev_uninit   = rte_szedata2_eth_dev_uninit,
> +               .dev_private_size = sizeof(struct pmd_internals),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(RTE_SZEDATA2_DRIVER_NAME, szedata2_eth_driver.pci_drv);
> diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
> index 10603197..f13fad90 100644
> --- a/drivers/net/thunderx/nicvf_ethdev.c
> +++ b/drivers/net/thunderx/nicvf_ethdev.c
> @@ -2111,15 +2111,17 @@ static const struct rte_pci_id pci_id_nicvf_map[] = {
>         },
>  };
>
> -static struct eth_driver rte_nicvf_pmd = {
> +static struct rte_pci_eth_driver rte_nicvf_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_nicvf_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = nicvf_eth_dev_init,
> -       .dev_private_size = sizeof(struct nicvf),
> +       .eth_drv = {
> +               .eth_dev_init = nicvf_eth_dev_init,
> +               .dev_private_size = sizeof(struct nicvf),
> +       },
>  };
>
>  RTE_PMD_REGISTER_PCI(net_thunderx, rte_nicvf_pmd.pci_drv);
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 54ea7d77..e6f241ad 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1377,7 +1377,7 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
>         return 0;
>  }
>
> -static struct eth_driver rte_virtio_pmd = {
> +static struct rte_pci_eth_driver rte_virtio_pmd = {
>         .pci_drv = {
>                 .driver = {
>                         .name = "net_virtio",
> @@ -1387,9 +1387,11 @@ static struct eth_driver rte_virtio_pmd = {
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_virtio_dev_init,
> -       .eth_dev_uninit = eth_virtio_dev_uninit,
> -       .dev_private_size = sizeof(struct virtio_hw),
> +       .eth_drv = {
> +               .eth_dev_init = eth_virtio_dev_init,
> +               .eth_dev_uninit = eth_virtio_dev_uninit,
> +               .dev_private_size = sizeof(struct virtio_hw),
> +       },
>  };
>
>  RTE_INIT(rte_virtio_pmd_init);
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 54533ca5..cb9221e6 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -337,16 +337,18 @@ eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev)
>         return 0;
>  }
>
> -static struct eth_driver rte_vmxnet3_pmd = {
> +static struct rte_pci_eth_driver rte_vmxnet3_pmd = {
>         .pci_drv = {
>                 .id_table = pci_id_vmxnet3_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
>                 .probe = rte_eth_dev_pci_probe,
>                 .remove = rte_eth_dev_pci_remove,
>         },
> -       .eth_dev_init = eth_vmxnet3_dev_init,
> -       .eth_dev_uninit = eth_vmxnet3_dev_uninit,
> -       .dev_private_size = sizeof(struct vmxnet3_hw),
> +       .eth_drv = {
> +               .eth_dev_init = eth_vmxnet3_dev_init,
> +               .eth_dev_uninit = eth_vmxnet3_dev_uninit,
> +               .dev_private_size = sizeof(struct vmxnet3_hw),
> +       },
>  };
>
>  static int
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 9dea1f15..7c212096 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -239,13 +239,14 @@ int
>  rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>                       struct rte_pci_device *pci_dev)
>  {
> -       struct eth_driver    *eth_drv;
> +       const struct rte_pci_eth_driver *pci_eth_drv;
> +       const struct eth_driver *eth_drv;
>         struct rte_eth_dev *eth_dev;
>         char ethdev_name[RTE_ETH_NAME_MAX_LEN];
> -
>         int diag;
>
> -       eth_drv = (struct eth_driver *)pci_drv;
> +       pci_eth_drv = container_of(pci_drv, struct rte_pci_eth_driver, pci_drv);
> +       eth_drv = &pci_eth_drv->eth_drv;
>
>         rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
>                         sizeof(ethdev_name));
> @@ -263,7 +264,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>         }
>         eth_dev->device = &pci_dev->device;
>         eth_dev->intr_handle = &pci_dev->intr_handle;
> -       eth_dev->driver = eth_drv;
> +       eth_dev->driver = &pci_eth_drv->eth_drv;
>
>         /* Invoke PMD device initialization function */
>         diag = (*eth_drv->eth_dev_init)(eth_dev);
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index b4168830..1a62a322 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1884,25 +1884,31 @@ typedef int (*eth_dev_uninit_t)(struct rte_eth_dev *eth_dev);
>   * @internal
>   * The structure associated with a PMD Ethernet driver.
>   *
> - * Each Ethernet driver acts as a PCI driver and is represented by a generic
> + * Each Ethernet driver acts is represented by a generic
>   * *eth_driver* structure that holds:
>   *
> - * - An *rte_pci_driver* structure (which must be the first field).
> + * - The *eth_dev_init* function invoked for each matching device.
>   *
> - * - The *eth_dev_init* function invoked for each matching PCI device.
> - *
> - * - The *eth_dev_uninit* function invoked for each matching PCI device.
> + * - The *eth_dev_uninit* function invoked for each matching device.
>   *
>   * - The size of the private data to allocate for each matching device.
>   */
>  struct eth_driver {
> -       struct rte_pci_driver pci_drv;    /**< The PMD is also a PCI driver. */
>         eth_dev_init_t eth_dev_init;      /**< Device init function. */
>         eth_dev_uninit_t eth_dev_uninit;  /**< Device uninit function. */
>         unsigned int dev_private_size;    /**< Size of device private data. */
>  };
>
>  /**
> + * @internal
> + * The structure associated with a PMD PCI Ethernet driver.
> + */
> +struct rte_pci_eth_driver {
> +       struct rte_pci_driver   pci_drv;        /**< Underlying PCI driver. */
> +       struct eth_driver       eth_drv;        /**< Ethernet driver. */
> +};
> +
> +/**
>   * Convert a numerical speed in Mbps to a bitmap flag that can be used in
>   * the bitmap link_speeds of the struct rte_eth_conf
>   *
> --
> 2.11.0
>

^ permalink raw reply

* Re: [PATCH v4] rte_mbuf: add rte_pktmbuf_linearize
From: Olivier MATZ @ 2017-01-10 16:50 UTC (permalink / raw)
  To: Tomasz Kulasek; +Cc: dev, Pablo de Lara, Olivier Matz
In-Reply-To: <1483634640-33892-1-git-send-email-tomaszx.kulasek@intel.com>

Hi Tomasz,

On Thu,  5 Jan 2017 17:44:00 +0100, Tomasz Kulasek
<tomaszx.kulasek@intel.com> wrote:
> This patch adds function rte_pktmbuf_linearize to let crypto PMD
> coalesce chained mbuf before crypto operation and extend their
> capabilities to support segmented mbufs when device cannot handle
> them natively.
> 
> Included unit tests for rte_pktmbuf_linearize functionality:
> 
>  1) Creates banch of segmented mbufs with different size and number of
>     segments.
>  2) Generates pkt_len bytes of random data.
>  3) Fills noncontigouos mbuf with randomly generated data.
>  4) Uses rte_pktmbuf_linearize to coalesce segmented buffer into one
>     contiguous.
>  5) Verifies data in linearized buffer.
> 
> Dependencies:
> 
> This patch is rebased to the dpdk-next-crypto and should be applied
> before "Chained Mbufs support in SW PMDs" patchset.
> 
> 
> changes in v4:
>  - separated from "Chained Mbufs support in SW PMDs" patch set for
>    better reviewing,
>  - merged "rte_pktmbuf_linearize" implementation with unit tests,
> 
> changes in v3:
>  - rebased to dpdk-next-crypto
> 
> changes in v2:
>  - rte_pktmbuf_coalesce replaced with rte_pktmbuf_linearize
> 
> 
> Cc: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Cc: Olivier Matz <olivier.matz@6wind.com>
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>

The overall idea and code looks good to me. I have some minor
comments, please see below.

To avoid the warning of check-git-log.sh, the title of the commit could
be change to:
  mbuf: add a function to linearize a packet


> diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
> index c0823ea..39577e7 100644
> --- a/app/test/test_mbuf.c
> +++ b/app/test/test_mbuf.c
> @@ -930,6 +930,124 @@
>  	return 0;
>  }
>  
> +static int
> +test_mbuf_linearize(int pkt_len, int nb_segs) {
> +
> +	struct rte_mbuf *m = NULL, *mbuf_src = NULL;
> +	uint8_t data[pkt_len], *src, *dst;
> +	int data_len = 0;
> +	int i, size;
> +	int t_len;
> +
> +	if (pkt_len < 1) {
> +		printf("Packet size must be 1 or more (is %d)\n",
> pkt_len);
> +		return -1;
> +	}
> +
> +	if (nb_segs < 1) {
> +		printf("Number of segments must be 1 or more (is
> %d)\n",
> +				nb_segs);
> +		return -1;
> +	}
> +
> +	/* Setup buffer */
> +	for (i = 0; i < pkt_len; i++)
> +		data[i] = (uint8_t) rte_rand();

This is maybe a bit overkill.
What about data[i] = i ?

And with this assumption, the local buffer would become useless.

> +
> +	t_len = pkt_len >= nb_segs ? pkt_len / nb_segs : 1;

Hmm, I think we could do clearer here. Maybe:

	seg_len = pkt_len / nb_segs;
	if (seg_len == 0)
		seg_len = 1;

> +	src = data;
> +	size = pkt_len;

rename 'size' as 'remain'?

> +
> +	/* Create chained mbuf_src and fill it generated data */
> +	for (i = 0; size > 0; i++) {
> +
> +		m = rte_pktmbuf_alloc(pktmbuf_pool);
> +		if (i == 0)
> +			mbuf_src = m;
> +
> +		if (!m) {

(x == NULL) should be preferred when the variable is a pointer (see
coding_style.rst).

> +			printf("Cannot create segment for source
> mbuf");
> +			goto fail;
> +		}
> +
> +		/* Make sure if tailroom is zeroed */
> +		memset(rte_pktmbuf_mtod(m, uint8_t *), 0,
> +				rte_pktmbuf_tailroom(m));
> +
> +		data_len = size > t_len ? t_len : size;

I think the following is easier to read:

	data_len = remain;
	if (data_len > seg_len)
		data_len = seg_len;

But for that one, maybe it's me, I let you decide ;)


> +		dst = (uint8_t *)rte_pktmbuf_append(m, data_len);
> +		if (!dst) {

if (dst == NULL)

> +			printf("Cannot append %d bytes to the
> mbuf\n",
> +					data_len);
> +			goto fail;
> +		}
> +
> +		rte_memcpy(dst, src, data_len);
> +		src += data_len;
> +
> +		if (mbuf_src != m)
> +			rte_pktmbuf_chain(mbuf_src, m);
> +
> +		size -= data_len;
> +
> +	}
> +
> +	/* Create destination buffer to store coalesced data */
> +	if (rte_pktmbuf_linearize(mbuf_src)) {
> +		printf("Mbuf linearization failed\n");
> +		goto fail;
> +	}
> +
> +	if (!rte_pktmbuf_is_contiguous(mbuf_src)) {
> +		printf("Source buffer should be contiguous after "
> +				"linearization\n");
> +		goto fail;
> +	}
> +
> +	src = rte_pktmbuf_mtod(mbuf_src, uint8_t *);
> +
> +	if (memcmp(src, data, rte_pktmbuf_pkt_len(mbuf_src))) {
> +		printf("Incorrect data in coalesced mbuf\n");
> +		goto fail;
> +	}
> +
> +	if (mbuf_src)
> +		rte_pktmbuf_free(mbuf_src);

Is it possible that mbuf_src is NULL here?

> +	return 0;
> +
> +fail:
> +	if (mbuf_src)
> +		rte_pktmbuf_free(mbuf_src);
> +	return -1;
> +}
> +
> +static int
> +test_mbuf_linearize_check(void)
> +{
> +	struct test_mbuf_array {
> +		int size;
> +		int nb_segs;
> +	} mbuf_array[5] = {

you could change [5] by []

> +			{ 128, 1 },
> +			{ 64, 64 },
> +			{ 512, 10 },
> +			{ 250, 11 },
> +			{ 123, 8 },
> +	};
> +	unsigned int i;
> +
> +	printf("Test mbuf linearize API\n");
> +
> +	for (i = 0; i < RTE_DIM(mbuf_array); i++)
> +		if (test_mbuf_linearize(mbuf_array[i].size,
> +				mbuf_array[i].nb_segs)) {
> +			printf("Test failed for %d, %d\n",
> mbuf_array[i].size,
> +					mbuf_array[i].nb_segs);
> +			return -1;
> +		}
> +
> +	return 0;
> +}
>  
>  static int
>  test_mbuf(void)
> @@ -1023,6 +1141,11 @@
>  		printf("test_failing_mbuf_sanity_check() failed\n");
>  		return -1;
>  	}
> +
> +	if (test_mbuf_linearize_check() < 0) {
> +		printf("test_mbuf_linearize_check() failed\n");
> +		return -1;
> +	}
>  	return 0;
>  }
>  
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index ead7c6e..b11a31d 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1647,6 +1647,62 @@ static inline int rte_pktmbuf_chain(struct
> rte_mbuf *head, struct rte_mbuf *tail }
>  
>  /**
> + * Linearize data in mbuf.
> + *
> + * This function coalesce mbuf merging data in the first segment,
> unchaining
> + * rest, and then frees them.

I suggest this description instead:

This function moves the mbuf data in the first segment if there is
enough tailroom. The subsequent segments are unchained and freed.

> + *
> + * All operations are done in-place, so the structure of incoming
> mbuf
> + * is changed.

Not sure what you mean here. Are you talking about pointers to data
that could become invalid?

> + *
> + * @param mbuf
> + *   mbuf to linearize
> + * @return
> + *   - 0, on success
> + *   - -1, on error
> + */
> +static inline int
> +rte_pktmbuf_linearize(struct rte_mbuf *mbuf)
> +{
> +	int l, n;
> +	struct rte_mbuf *m;
> +	struct rte_mbuf *m_next;
> +	char *buffer;
> +
> +	if (rte_pktmbuf_is_contiguous(mbuf))
> +		return 0;
> +
> +	/* Extend first segment to the total packet length
> +	 */

For one-line comments, it can be:
	/* foo bar */

> +	n = rte_pktmbuf_pkt_len(mbuf) - rte_pktmbuf_data_len(mbuf);

rename 'n' as 'copy_len'?

> +
> +	if (unlikely(n > rte_pktmbuf_tailroom(mbuf)))
> +		return -1;
> +
> +	buffer = rte_pktmbuf_mtod_offset(mbuf, char *,
> mbuf->data_len);
> +	mbuf->data_len = (uint16_t)(mbuf->pkt_len);
> +
> +	/* Append data from next segments to the first one
> +	 */

/* foo bar */

> +	m = mbuf->next;
> +	while (m != NULL) {
> +		m_next = m->next;
> +
> +		l = rte_pktmbuf_data_len(m);

rename 'l' as 'seg_len'?

> +		rte_memcpy(buffer, rte_pktmbuf_mtod(m, char *), l);
> +		buffer += l;
> +
> +		rte_pktmbuf_free_seg(m);
> +		m = m_next;
> +	}
> +
> +	mbuf->next = NULL;
> +	mbuf->nb_segs = 1;
> +
> +	return 0;
> +}


Thank you Tomasz , I think it could be useful in other scenarii. In the
future it could be extended to memmove() the data of the first segment
if the tailroom is too small.

Regards,
Olivier

^ permalink raw reply

* Re: [PATCH v3 0/8] Add crypto PMD optimized for ARMv8
From: De Lara Guarch, Pablo @ 2017-01-10 17:11 UTC (permalink / raw)
  To: zbigniew.bodek@caviumnetworks.com, dev@dpdk.org
  Cc: Doherty, Declan, jerin.jacob@caviumnetworks.com
In-Reply-To: <1483551207-18236-1-git-send-email-zbigniew.bodek@caviumnetworks.com>

Hi Zbigniew,


> -----Original Message-----
> From: zbigniew.bodek@caviumnetworks.com
> [mailto:zbigniew.bodek@caviumnetworks.com]
> Sent: Wednesday, January 04, 2017 5:33 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Doherty, Declan;
> jerin.jacob@caviumnetworks.com; Zbigniew Bodek
> Subject: [PATCH v3 0/8] Add crypto PMD optimized for ARMv8
> 
> From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>

...

> 
> Zbigniew Bodek (8):
>   mk: fix build of assembly files for ARM64
>   lib: add cryptodev type for the upcoming ARMv8 PMD
>   crypto/armv8: add PMD optimized for ARMv8 processors
>   mk/crypto/armv8: add PMD to the build system
>   doc/armv8: update documentation about crypto PMD
>   crypto/armv8: enable ARMv8 PMD in the configuration
>   crypto/armv8: update MAINTAINERS entry for ARMv8 crypto
>   app/test: add ARMv8 crypto tests and test vectors

Thanks for this patchset.

Could you change the titles of some of these patches?
The prefix should be "mk:" and not "mk/crypto/armv8", for instance.
The other ones that should be changed are "doc/armv8" -> "doc" and "crypto/armv8: update MAINTAINERS" to "MAINTAINERS:".

I can do this for you, if you are OK with these changes.

Apart from this, can anyone review these changes? I do not have access to an ARM board,
so it is a bit difficult for me to review it.

Thanks,
Pablo 

^ permalink raw reply

* [PATCH v2] tools: fix active interface detection in dpdk-devbind.py
From: Yoni Gilad @ 2017-01-10 17:14 UTC (permalink / raw)
  To: dev; +Cc: pablo.de.lara.guarch, Yoni Gilad, stable
In-Reply-To: <1483979853-16386-1-git-send-email-yonig@radcom.com>

When adding crypto devices, the "Active" and "Ssh_if" attributes of
existing network devices were reset. This causes the following issues:

- Network interfaces aren't marked as "*Active*" in the --status output.
- Active network interfaces can be unbound without the --force option,
  causing loss of network connection.

The reset was caused by the call to devices[d].update in
get_crypto_details.

This patch prevents the update on non-crypto devices.

Fixes: cb4a1d14bf3e ("tools: bind crypto devices")

CC: stable@dpdk.org

Signed-off-by: Yoni Gilad <yonig@radcom.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---

v2:
- Fixed commit message

 usertools/dpdk-devbind.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index e057b87..1b9c651 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -328,6 +328,9 @@ def get_crypto_details():
 
     # based on the basic info, get extended text details
     for d in devices.keys():
+        if devices[d]["Class"][0:2] != CRYPTO_BASE_CLASS:
+            continue
+
         # get additional info and add it to existing data
         devices[d] = devices[d].copy()
         devices[d].update(get_pci_device_details(d).items())
-- 
2.5.5

^ permalink raw reply related

* Re: [PATCH 8/8] eal: VMBUS infrastructure
From: Jan Blunck @ 2017-01-10 17:27 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Stephen Hemminger
In-Reply-To: <20170107181756.1944-9-sthemmin@microsoft.com>

On Sat, Jan 7, 2017 at 7:17 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> Add support for VMBUS on Hyper-V/Azure. VMBUS is similar to PCI
> but has different addressing and internal API's.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  lib/librte_eal/common/Makefile              |   2 +-
>  lib/librte_eal/common/eal_common_devargs.c  |   7 +
>  lib/librte_eal/common/eal_common_options.c  |  38 ++
>  lib/librte_eal/common/eal_internal_cfg.h    |   1 +
>  lib/librte_eal/common/eal_options.h         |   6 +
>  lib/librte_eal/common/eal_private.h         |   5 +
>  lib/librte_eal/common/include/rte_devargs.h |   8 +
>  lib/librte_eal/common/include/rte_vmbus.h   | 249 ++++++++
>  lib/librte_eal/linuxapp/eal/Makefile        |   6 +
>  lib/librte_eal/linuxapp/eal/eal.c           |  13 +
>  lib/librte_eal/linuxapp/eal/eal_vmbus.c     | 911 ++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ethdev.c               |  90 +++
>  lib/librte_ether/rte_ethdev.h               |  31 +
>  mk/rte.app.mk                               |   1 +
>  14 files changed, 1367 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_eal/common/include/rte_vmbus.h
>  create mode 100644 lib/librte_eal/linuxapp/eal/eal_vmbus.c
>
> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
> index 09a3d3af..ceb77bed 100644
> --- a/lib/librte_eal/common/Makefile
> +++ b/lib/librte_eal/common/Makefile
> @@ -33,7 +33,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
>
>  INC := rte_branch_prediction.h rte_common.h
>  INC += rte_debug.h rte_eal.h rte_errno.h rte_launch.h rte_lcore.h
> -INC += rte_log.h rte_memory.h rte_memzone.h rte_pci.h
> +INC += rte_log.h rte_memory.h rte_memzone.h rte_pci.h rte_vmbus.h
>  INC += rte_per_lcore.h rte_random.h
>  INC += rte_tailq.h rte_interrupts.h rte_alarm.h
>  INC += rte_string_fns.h rte_version.h
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index e403717b..934ca840 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -113,6 +113,13 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>                         goto fail;
>
>                 break;
> +       case RTE_DEVTYPE_WHITELISTED_VMBUS:
> +       case RTE_DEVTYPE_BLACKLISTED_VMBUS:
> +#ifdef RTE_LIBRTE_HV_PMD
> +               if (uuid_parse(buf, devargs->uuid) == 0)
> +                       break;
> +#endif
> +               goto fail;
>         }
>
>         free(buf);
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index f36bc556..1a2b418c 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -95,6 +95,11 @@ eal_long_options[] = {
>         {OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
>         {OPT_VMWARE_TSC_MAP,    0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
>         {OPT_XEN_DOM0,          0, NULL, OPT_XEN_DOM0_NUM         },
> +#ifdef RTE_LIBRTE_HV_PMD
> +       {OPT_NO_VMBUS,          0, NULL, OPT_NO_VMBUS_NUM         },
> +       {OPT_VMBUS_BLACKLIST,   1, NULL, OPT_VMBUS_BLACKLIST_NUM  },
> +       {OPT_VMBUS_WHITELIST,   1, NULL, OPT_VMBUS_WHITELIST_NUM  },
> +#endif
>         {0,                     0, NULL, 0                        }
>  };
>
> @@ -858,6 +863,21 @@ eal_parse_common_option(int opt, const char *optarg,
>                 conf->no_pci = 1;
>                 break;
>
> +#ifdef RTE_LIBRTE_HV_PMD
> +       case OPT_NO_VMBUS_NUM:
> +               conf->no_vmbus = 1;
> +               break;
> +       case OPT_VMBUS_BLACKLIST_NUM:
> +               if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_VMBUS,
> +                                       optarg) < 0)
> +                       return -1;
> +               break;
> +       case OPT_VMBUS_WHITELIST_NUM:
> +               if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_VMBUS,
> +                               optarg) < 0)
> +                       return -1;
> +               break;
> +#endif
>         case OPT_NO_HPET_NUM:
>                 conf->no_hpet = 1;
>                 break;
> @@ -1017,6 +1037,14 @@ eal_check_common_options(struct internal_config *internal_cfg)
>                 return -1;
>         }
>
> +#ifdef RTE_LIBRTE_HV_PMD
> +       if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_VMBUS) != 0 &&
> +               rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_VMBUS) != 0) {
> +               RTE_LOG(ERR, EAL, "Options vmbus blacklist and whitelist "
> +                       "cannot be used at the same time\n");
> +               return -1;
> +       }
> +#endif
>         return 0;
>  }
>
> @@ -1066,5 +1094,15 @@ eal_common_usage(void)
>                "  --"OPT_NO_PCI"            Disable PCI\n"
>                "  --"OPT_NO_HPET"           Disable HPET\n"
>                "  --"OPT_NO_SHCONF"         No shared config (mmap'd files)\n"
> +#ifdef RTE_LIBRTE_HV_PMD
> +              "  --"OPT_NO_VMBUS"          Disable VMBUS\n"
> +              "  --"OPT_VMBUS_BLACKLIST" Add a VMBUS device to black list.\n"
> +              "                      Prevent EAL from using this PCI device. The argument\n"
> +              "                      format is device UUID.\n"
> +              "  --"OPT_VMBUS_WHITELIST" Add a VMBUS device to white list.\n"
> +              "                      Only use the specified VMBUS devices. The argument format\n"
> +              "                      is device UUID This option can be present\n"
> +              "                      several times (once per device).\n"
> +#endif
>                "\n", RTE_MAX_LCORE);
>  }
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index 5f1367eb..4b6af937 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -67,6 +67,7 @@ struct internal_config {
>         unsigned hugepage_unlink;         /**< true to unlink backing files */
>         volatile unsigned xen_dom0_support; /**< support app running on Xen Dom0*/
>         volatile unsigned no_pci;         /**< true to disable PCI */
> +       volatile unsigned no_vmbus;       /**< true to disable VMBUS */
>         volatile unsigned no_hpet;        /**< true to disable HPET */
>         volatile unsigned vmware_tsc_map; /**< true to use VMware TSC mapping
>                                                                                 * instead of native TSC */
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index a881c62e..156727e7 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -83,6 +83,12 @@ enum {
>         OPT_VMWARE_TSC_MAP_NUM,
>  #define OPT_XEN_DOM0          "xen-dom0"
>         OPT_XEN_DOM0_NUM,
> +#define OPT_NO_VMBUS          "no-vmbus"
> +       OPT_NO_VMBUS_NUM,
> +#define OPT_VMBUS_BLACKLIST   "vmbus-blacklist"
> +       OPT_VMBUS_BLACKLIST_NUM,
> +#define OPT_VMBUS_WHITELIST   "vmbus-whitelist"
> +       OPT_VMBUS_WHITELIST_NUM,
>         OPT_LONG_MAX_NUM
>  };
>
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 9e7d8f6b..c856c63e 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -210,6 +210,11 @@ int pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
>                 struct mapped_pci_resource *uio_res, int map_idx);
>
>  /**
> + * VMBUS related functions and structures
> + */
> +int rte_eal_vmbus_init(void);
> +
> +/**
>   * Init tail queues for non-EAL library structures. This is to allow
>   * the rings, mempools, etc. lists to be shared among multiple processes
>   *
> diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> index 88120a1c..c079d289 100644
> --- a/lib/librte_eal/common/include/rte_devargs.h
> +++ b/lib/librte_eal/common/include/rte_devargs.h
> @@ -51,6 +51,9 @@ extern "C" {
>  #include <stdio.h>
>  #include <sys/queue.h>
>  #include <rte_pci.h>
> +#ifdef RTE_LIBRTE_HV_PMD
> +#include <uuid/uuid.h>
> +#endif
>
>  /**
>   * Type of generic device
> @@ -59,6 +62,8 @@ enum rte_devtype {
>         RTE_DEVTYPE_WHITELISTED_PCI,
>         RTE_DEVTYPE_BLACKLISTED_PCI,
>         RTE_DEVTYPE_VIRTUAL,
> +       RTE_DEVTYPE_WHITELISTED_VMBUS,
> +       RTE_DEVTYPE_BLACKLISTED_VMBUS,
>  };
>
>  /**
> @@ -88,6 +93,9 @@ struct rte_devargs {
>                         /** Driver name. */
>                         char drv_name[32];
>                 } virt;
> +#ifdef RTE_LIBRTE_HV_PMD
> +               uuid_t uuid;
> +#endif
>         };
>         /** Arguments string as given by user or "" for no argument. */
>         char *args;
> diff --git a/lib/librte_eal/common/include/rte_vmbus.h b/lib/librte_eal/common/include/rte_vmbus.h
> new file mode 100644
> index 00000000..f96d753e
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_vmbus.h
> @@ -0,0 +1,249 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2013-2016 Brocade Communications Systems, Inc.
> + *   Copyright(c) 2016 Microsoft Corporation
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + */
> +
> +#ifndef _RTE_VMBUS_H_
> +#define _RTE_VMBUS_H_
> +
> +/**
> + * @file
> + *
> + * RTE VMBUS Interface
> + */
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include <uuid/uuid.h>
> +#include <sys/queue.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +
> +#include <rte_debug.h>
> +#include <rte_interrupts.h>
> +#include <rte_dev.h>
> +
> +TAILQ_HEAD(vmbus_device_list, rte_vmbus_device);
> +TAILQ_HEAD(vmbus_driver_list, rte_vmbus_driver);
> +
> +extern struct vmbus_driver_list vmbus_driver_list;
> +extern struct vmbus_device_list vmbus_device_list;
> +
> +/** Pathname of VMBUS devices directory. */
> +#define SYSFS_VMBUS_DEVICES "/sys/bus/vmbus/devices"
> +
> +#define UUID_BUF_SZ    (36 + 1)
> +
> +
> +/** Maximum number of VMBUS resources. */
> +#define VMBUS_MAX_RESOURCE 7
> +
> +/**
> + * A structure describing a VMBUS device.
> + */
> +struct rte_vmbus_device {
> +       TAILQ_ENTRY(rte_vmbus_device) next;     /**< Next probed VMBUS device. */
> +       struct rte_device device;               /**< Inherit core device */
> +       uuid_t device_id;                       /**< VMBUS device id */
> +       uuid_t class_id;                        /**< VMBUS device type */
> +       uint32_t relid;                         /**< VMBUS id for notification */
> +       uint8_t monitor_id;
> +       struct rte_intr_handle intr_handle;     /**< Interrupt handle */
> +       const struct rte_vmbus_driver *driver;  /**< Associated driver */
> +
> +       struct rte_mem_resource mem_resource[VMBUS_MAX_RESOURCE];
> +                                               /**< VMBUS Memory Resource */
> +       char sysfs_name[];                      /**< Name in sysfs bus directory */
> +};
> +
> +struct rte_vmbus_driver;
> +
> +/**
> + * Initialisation function for the driver called during VMBUS probing.
> + */
> +typedef int (vmbus_probe_t)(struct rte_vmbus_driver *,
> +                           struct rte_vmbus_device *);
> +
> +/**
> + * Uninitialisation function for the driver called during hotplugging.
> + */
> +typedef int (vmbus_remove_t)(struct rte_vmbus_device *);
> +
> +/**
> + * A structure describing a VMBUS driver.
> + */
> +struct rte_vmbus_driver {
> +       TAILQ_ENTRY(rte_vmbus_driver) next;     /**< Next in list. */
> +       struct rte_driver driver;
> +       vmbus_probe_t *probe;                   /**< Device Probe function. */
> +       vmbus_remove_t *remove;                 /**< Device Remove function. */
> +
> +       const uuid_t *id_table;                 /**< ID table. */
> +};
> +
> +struct vmbus_map {
> +       void *addr;
> +       char *path;
> +       uint64_t offset;
> +       uint64_t size;
> +       uint64_t phaddr;
> +};
> +
> +/*
> + * For multi-process we need to reproduce all vmbus mappings in secondary
> + * processes, so save them in a tailq.
> + */
> +struct mapped_vmbus_resource {
> +       TAILQ_ENTRY(mapped_vmbus_resource) next;
> +
> +       uuid_t uuid;
> +       char path[PATH_MAX];
> +       int nb_maps;
> +       struct vmbus_map maps[VMBUS_MAX_RESOURCE];
> +};
> +
> +TAILQ_HEAD(mapped_vmbus_res_list, mapped_vmbus_resource);
> +
> +/**
> + * Scan the content of the VMBUS bus, and the devices in the devices list
> + *
> + * @return
> + *  0 on success, negative on error
> + */
> +int rte_eal_vmbus_scan(void);
> +
> +/**
> + * Probe the VMBUS bus for registered drivers.
> + *
> + * Scan the content of the VMBUS bus, and call the probe() function for
> + * all registered drivers that have a matching entry in its id_table
> + * for discovered devices.
> + *
> + * @return
> + *   - 0 on success.
> + *   - Negative on error.
> + */
> +int rte_eal_vmbus_probe(void);
> +
> +/**
> + * Map the VMBUS device resources in user space virtual memory address
> + *
> + * @param dev
> + *   A pointer to a rte_vmbus_device structure describing the device
> + *   to use
> + *
> + * @return
> + *   0 on success, negative on error and positive if no driver
> + *   is found for the device.
> + */
> +int rte_eal_vmbus_map_device(struct rte_vmbus_device *dev);
> +
> +/**
> + * Unmap this device
> + *
> + * @param dev
> + *   A pointer to a rte_vmbus_device structure describing the device
> + *   to use
> + */
> +void rte_eal_vmbus_unmap_device(struct rte_vmbus_device *dev);
> +
> +/**
> + * Probe the single VMBUS device.
> + *
> + * Scan the content of the VMBUS bus, and find the vmbus device
> + * specified by device uuid, then call the probe() function for
> + * registered driver that has a matching entry in its id_table for
> + * discovered device.
> + *
> + * @param id
> + *   The VMBUS device uuid.
> + * @return
> + *   - 0 on success.
> + *   - Negative on error.
> + */
> +int rte_eal_vmbus_probe_one(uuid_t id);
> +
> +/**
> + * Close the single VMBUS device.
> + *
> + * Scan the content of the VMBUS bus, and find the vmbus device id,
> + * then call the remove() function for registered driver that has a
> + * matching entry in its id_table for discovered device.
> + *
> + * @param id
> + *   The VMBUS device uuid.
> + * @return
> + *   - 0 on success.
> + *   - Negative on error.
> + */
> +int rte_eal_vmbus_detach(uuid_t id);
> +
> +/**
> + * Register a VMBUS driver.
> + *
> + * @param driver
> + *   A pointer to a rte_vmbus_driver structure describing the driver
> + *   to be registered.
> + */
> +void rte_eal_vmbus_register(struct rte_vmbus_driver *driver);
> +
> +/** Helper for VMBUS device registration from driver nstance */
> +#define RTE_PMD_REGISTER_VMBUS(nm, vmbus_drv) \
> +RTE_INIT(vmbusinitfn_ ##nm); \
> +static void vmbusinitfn_ ##nm(void) \
> +{\
> +       (vmbus_drv).driver.name = RTE_STR(nm);\
> +       (vmbus_drv).driver.type = PMD_VMBUS; \
> +       rte_eal_vmbus_register(&vmbus_drv); \
> +} \
> +RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
> +
> +/**
> + * Unregister a VMBUS driver.
> + *
> + * @param driver
> + *   A pointer to a rte_vmbus_driver structure describing the driver
> + *   to be unregistered.
> + */
> +void rte_eal_vmbus_unregister(struct rte_vmbus_driver *driver);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_VMBUS_H_ */
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> index 4e206f09..f6ca3848 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -71,6 +71,11 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_timer.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_interrupts.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_alarm.c
>
> +ifeq ($(CONFIG_RTE_LIBRTE_HV_PMD),y)
> +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_vmbus.c
> +LDLIBS += -luuid
> +endif
> +
>  # from common dir
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_lcore.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_timer.c
> @@ -114,6 +119,7 @@ CFLAGS_eal_hugepage_info.o := -D_GNU_SOURCE
>  CFLAGS_eal_pci.o := -D_GNU_SOURCE
>  CFLAGS_eal_pci_uio.o := -D_GNU_SOURCE
>  CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE
> +CFLAGS_eal_vmbux.o := -D_GNU_SOURCE
>  CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE
>  CFLAGS_eal_common_options.o := -D_GNU_SOURCE
>  CFLAGS_eal_common_thread.o := -D_GNU_SOURCE
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 16dd5b9c..1bc0814a 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -70,6 +70,9 @@
>  #include <rte_cpuflags.h>
>  #include <rte_interrupts.h>
>  #include <rte_pci.h>
> +#ifdef RTE_LIBRTE_HV_PMD
> +#include <rte_vmbus.h>
> +#endif
>  #include <rte_dev.h>
>  #include <rte_devargs.h>
>  #include <rte_common.h>
> @@ -830,6 +833,11 @@ rte_eal_init(int argc, char **argv)
>
>         eal_check_mem_on_local_socket();
>
> +#ifdef RTE_LIBRTE_HV_PMD
> +       if (rte_eal_vmbus_init() < 0)
> +               RTE_LOG(ERR, EAL, "Cannot init VMBUS\n");
> +#endif
> +
>         if (eal_plugins_init() < 0)
>                 rte_panic("Cannot init plugins\n");
>
> @@ -884,6 +892,11 @@ rte_eal_init(int argc, char **argv)
>         if (rte_eal_pci_probe())
>                 rte_panic("Cannot probe PCI\n");
>
> +#ifdef RTE_LIBRTE_HV_PMD
> +       if (rte_eal_vmbus_probe() < 0)
> +               rte_panic("Cannot probe VMBUS\n");
> +#endif
> +
>         if (rte_eal_dev_init() < 0)
>                 rte_panic("Cannot init pmd devices\n");
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_vmbus.c b/lib/librte_eal/linuxapp/eal/eal_vmbus.c
> new file mode 100644
> index 00000000..729f93a9
> --- /dev/null
> +++ b/lib/librte_eal/linuxapp/eal/eal_vmbus.c
> @@ -0,0 +1,911 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2013-2016 Brocade Communications Systems, Inc.
> + *   Copyright(c) 2016 Microsoft Corporation
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *      notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *      notice, this list of conditions and the following disclaimer in
> + *      the documentation and/or other materials provided with the
> + *      distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *      contributors may be used to endorse or promote products derived
> + *      from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + */
> +
> +#include <string.h>
> +#include <unistd.h>
> +#include <dirent.h>
> +#include <fcntl.h>
> +#include <sys/mman.h>
> +
> +#include <rte_eal.h>
> +#include <rte_tailq.h>
> +#include <rte_log.h>
> +#include <rte_devargs.h>
> +#include <rte_vmbus.h>
> +#include <rte_malloc.h>
> +
> +#include "eal_private.h"
> +#include "eal_pci_init.h"
> +#include "eal_filesystem.h"
> +
> +struct vmbus_driver_list vmbus_driver_list =
> +       TAILQ_HEAD_INITIALIZER(vmbus_driver_list);
> +struct vmbus_device_list vmbus_device_list =
> +       TAILQ_HEAD_INITIALIZER(vmbus_device_list);
> +
> +static void *vmbus_map_addr;
> +
> +static struct rte_tailq_elem rte_vmbus_uio_tailq = {
> +       .name = "UIO_RESOURCE_LIST",
> +};
> +EAL_REGISTER_TAILQ(rte_vmbus_uio_tailq);
> +
> +/*
> + * parse a sysfs file containing one integer value
> + * different to the eal version, as it needs to work with 64-bit values
> + */
> +static int
> +vmbus_get_sysfs_uuid(const char *filename, uuid_t uu)
> +{
> +       char buf[BUFSIZ];
> +       char *cp, *in = buf;
> +       FILE *f;
> +
> +       f = fopen(filename, "r");
> +       if (f == NULL) {
> +               RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
> +                               __func__, filename);
> +               return -1;
> +       }
> +
> +       if (fgets(buf, sizeof(buf), f) == NULL) {
> +               RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
> +                               __func__, filename);
> +               fclose(f);
> +               return -1;
> +       }
> +       fclose(f);
> +
> +       cp = strchr(buf, '\n');
> +       if (cp)
> +               *cp = '\0';
> +
> +       /* strip { } notation */
> +       if (buf[0] == '{') {
> +               in = buf + 1;
> +               cp = strchr(in, '}');
> +               if (cp)
> +                       *cp = '\0';
> +       }
> +
> +       if (uuid_parse(in, uu) < 0) {
> +               RTE_LOG(ERR, EAL, "%s %s not a valid UUID\n",
> +                       filename, buf);
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> +
> +/* map a particular resource from a file */
> +static void *
> +vmbus_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
> +                  int flags)
> +{
> +       void *mapaddr;
> +
> +       /* Map the memory resource of device */
> +       mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
> +                      MAP_SHARED | flags, fd, offset);
> +       if (mapaddr == MAP_FAILED ||
> +           (requested_addr != NULL && mapaddr != requested_addr)) {
> +               RTE_LOG(ERR, EAL,
> +                       "%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s)\n",
> +                       __func__, fd, requested_addr,
> +                       (unsigned long)size, (unsigned long)offset,
> +                       strerror(errno));
> +       } else
> +               RTE_LOG(DEBUG, EAL, "  VMBUS memory mapped at %p\n", mapaddr);
> +
> +       return mapaddr;
> +}
> +
> +/* unmap a particular resource */
> +static void
> +vmbus_unmap_resource(void *requested_addr, size_t size)
> +{
> +       if (requested_addr == NULL)
> +               return;
> +
> +       /* Unmap the VMBUS memory resource of device */
> +       if (munmap(requested_addr, size)) {
> +               RTE_LOG(ERR, EAL, "%s(): cannot munmap(%p, 0x%lx): %s\n",
> +                       __func__, requested_addr, (unsigned long)size,
> +                       strerror(errno));
> +       } else
> +               RTE_LOG(DEBUG, EAL, "  VMBUS memory unmapped at %p\n",
> +                               requested_addr);
> +}
> +
> +/* Only supports current kernel version
> + * Unlike PCI there is no option (or need) to create UIO device.
> + */
> +static int vmbus_get_uio_dev(const char *name,
> +                            char *dstbuf, size_t buflen)
> +{
> +       char dirname[PATH_MAX];
> +       unsigned int uio_num;
> +       struct dirent *e;
> +       DIR *dir;
> +
> +       snprintf(dirname, sizeof(dirname),
> +                "/sys/bus/vmbus/devices/%s/uio", name);
> +
> +       dir = opendir(dirname);
> +       if (dir == NULL) {
> +               RTE_LOG(ERR, EAL, "Cannot map uio resources for %s: %s\n",
> +                       name, strerror(errno));
> +               return -1;
> +       }
> +
> +       /* take the first file starting with "uio" */
> +       while ((e = readdir(dir)) != NULL) {
> +               if (sscanf(e->d_name, "uio%u", &uio_num) != 1)
> +                       continue;
> +
> +               snprintf(dstbuf, buflen, "%s/uio%u", dirname, uio_num);
> +               break;
> +       }
> +       closedir(dir);
> +
> +       return e ? (int) uio_num : -1;
> +}
> +
> +/*
> + * parse a sysfs file containing one integer value
> + * different to the eal version, as it needs to work with 64-bit values
> + */
> +static int
> +vmbus_parse_sysfs_value(const char *dir, const char *name,
> +                       uint64_t *val)
> +{
> +       char filename[PATH_MAX];
> +       FILE *f;
> +       char buf[BUFSIZ];
> +       char *end = NULL;
> +
> +       snprintf(filename, sizeof(filename), "%s/%s", dir, name);
> +       f = fopen(filename, "r");
> +       if (f == NULL) {
> +               RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
> +                               __func__, filename);
> +               return -1;
> +       }
> +
> +       if (fgets(buf, sizeof(buf), f) == NULL) {
> +               RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
> +                               __func__, filename);
> +               fclose(f);
> +               return -1;
> +       }
> +       fclose(f);
> +
> +       *val = strtoull(buf, &end, 0);
> +       if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
> +               RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n",
> +                               __func__, filename);
> +               return -1;
> +       }
> +       return 0;
> +}
> +
> +/* Get mappings out of values provided by uio */
> +static int
> +vmbus_uio_get_mappings(const char *uioname,
> +                      struct vmbus_map maps[])
> +{
> +       int i;
> +
> +       for (i = 0; i != VMBUS_MAX_RESOURCE; i++) {
> +               struct vmbus_map *map = &maps[i];
> +               char dirname[PATH_MAX];
> +
> +               /* check if map directory exists */
> +               snprintf(dirname, sizeof(dirname),
> +                        "%s/maps/map%d", uioname, i);
> +
> +               if (access(dirname, F_OK) != 0)
> +                       break;
> +
> +               /* get mapping offset */
> +               if (vmbus_parse_sysfs_value(dirname, "offset",
> +                                           &map->offset) < 0)
> +                       return -1;
> +
> +               /* get mapping size */
> +               if (vmbus_parse_sysfs_value(dirname, "size",
> +                                           &map->size) < 0)
> +                       return -1;
> +
> +               /* get mapping physical address */
> +               if (vmbus_parse_sysfs_value(dirname, "addr",
> +                                           &maps->phaddr) < 0)
> +                       return -1;
> +       }
> +
> +       return i;
> +}
> +
> +static void
> +vmbus_uio_free_resource(struct rte_vmbus_device *dev,
> +               struct mapped_vmbus_resource *uio_res)
> +{
> +       rte_free(uio_res);
> +
> +       if (dev->intr_handle.fd) {
> +               close(dev->intr_handle.fd);
> +               dev->intr_handle.fd = -1;
> +               dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
> +       }
> +}
> +
> +static struct mapped_vmbus_resource *
> +vmbus_uio_alloc_resource(struct rte_vmbus_device *dev)
> +{
> +       struct mapped_vmbus_resource *uio_res;
> +       char dirname[PATH_MAX], devname[PATH_MAX];
> +       int uio_num, nb_maps;
> +
> +       uio_num = vmbus_get_uio_dev(dev->sysfs_name, dirname, sizeof(dirname));
> +       if (uio_num < 0) {
> +               RTE_LOG(WARNING, EAL,
> +                       "  %s not managed by UIO driver, skipping\n",
> +                       dev->sysfs_name);
> +               return NULL;
> +       }
> +
> +       /* allocate the mapping details for secondary processes*/
> +       uio_res = rte_zmalloc("UIO_RES", sizeof(*uio_res), 0);
> +       if (uio_res == NULL) {
> +               RTE_LOG(ERR, EAL,
> +                       "%s(): cannot store uio mmap details\n", __func__);
> +               goto error;
> +       }
> +
> +       snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
> +       dev->intr_handle.fd = open(devname, O_RDWR);
> +       if (dev->intr_handle.fd < 0) {
> +               RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> +                       devname, strerror(errno));
> +               goto error;
> +       }
> +
> +       dev->intr_handle.type = RTE_INTR_HANDLE_UIO_INTX;
> +
> +       snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
> +       uuid_copy(uio_res->uuid, dev->device_id);
> +
> +       nb_maps = vmbus_uio_get_mappings(dirname, uio_res->maps);
> +       if (nb_maps < 0)
> +               goto error;
> +
> +       RTE_LOG(DEBUG, EAL, "Found %d memory maps for device %s\n",
> +               nb_maps, dev->sysfs_name);
> +
> +       return uio_res;
> +
> + error:
> +       vmbus_uio_free_resource(dev, uio_res);
> +       return NULL;
> +}
> +
> +static int
> +vmbus_uio_map_resource_by_index(struct rte_vmbus_device *dev,
> +                               unsigned int res_idx,
> +                               struct mapped_vmbus_resource *uio_res,
> +                               unsigned int map_idx)
> +{
> +       struct vmbus_map *maps = uio_res->maps;
> +       char devname[PATH_MAX];
> +       void *mapaddr;
> +       int fd;
> +
> +       snprintf(devname, sizeof(devname),
> +                "/sys/bus/vmbus/%s/resource%u", dev->sysfs_name, res_idx);
> +
> +       fd = open(devname, O_RDWR);
> +       if (fd < 0) {
> +               RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> +                               devname, strerror(errno));
> +               return -1;
> +       }
> +
> +       /* allocate memory to keep path */
> +       maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
> +       if (maps[map_idx].path == NULL) {
> +               RTE_LOG(ERR, EAL, "Cannot allocate memory for path: %s\n",
> +                               strerror(errno));
> +               return -1;
> +       }
> +
> +       /* try mapping somewhere close to the end of hugepages */
> +       if (vmbus_map_addr == NULL)
> +               vmbus_map_addr = pci_find_max_end_va();
> +
> +       mapaddr = vmbus_map_resource(vmbus_map_addr, fd, 0,
> +                                    dev->mem_resource[res_idx].len, 0);
> +       close(fd);
> +       if (mapaddr == MAP_FAILED) {
> +               rte_free(maps[map_idx].path);
> +               return -1;
> +       }
> +
> +       vmbus_map_addr = RTE_PTR_ADD(mapaddr,
> +                                    dev->mem_resource[res_idx].len);
> +
> +       maps[map_idx].phaddr = dev->mem_resource[res_idx].phys_addr;
> +       maps[map_idx].size = dev->mem_resource[res_idx].len;
> +       maps[map_idx].addr = mapaddr;
> +       maps[map_idx].offset = 0;
> +       strcpy(maps[map_idx].path, devname);
> +       dev->mem_resource[res_idx].addr = mapaddr;
> +
> +       return 0;
> +}
> +
> +static void
> +vmbus_uio_unmap(struct mapped_vmbus_resource *uio_res)
> +{
> +       int i;
> +
> +       if (uio_res == NULL)
> +               return;
> +
> +       for (i = 0; i != uio_res->nb_maps; i++) {
> +               vmbus_unmap_resource(uio_res->maps[i].addr,
> +                                    uio_res->maps[i].size);
> +
> +               if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +                       rte_free(uio_res->maps[i].path);
> +       }
> +}
> +
> +static struct mapped_vmbus_resource *
> +vmbus_uio_find_resource(struct rte_vmbus_device *dev)
> +{
> +       struct mapped_vmbus_resource *uio_res;
> +       struct mapped_vmbus_res_list *uio_res_list =
> +                       RTE_TAILQ_CAST(rte_vmbus_uio_tailq.head,
> +                                      mapped_vmbus_res_list);
> +
> +       if (dev == NULL)
> +               return NULL;
> +
> +       TAILQ_FOREACH(uio_res, uio_res_list, next) {
> +               if (uuid_compare(uio_res->uuid, dev->device_id) == 0)
> +                       return uio_res;
> +       }
> +       return NULL;
> +}
> +
> +/* unmap the VMBUS resource of a VMBUS device in virtual memory */
> +static void
> +vmbus_uio_unmap_resource(struct rte_vmbus_device *dev)
> +{
> +       struct mapped_vmbus_resource *uio_res;
> +       struct mapped_vmbus_res_list *uio_res_list =
> +                       RTE_TAILQ_CAST(rte_vmbus_uio_tailq.head,
> +                                      mapped_vmbus_res_list);
> +
> +       if (dev == NULL)
> +               return;
> +
> +       /* find an entry for the device */
> +       uio_res = vmbus_uio_find_resource(dev);
> +       if (uio_res == NULL)
> +               return;
> +
> +       /* secondary processes - just free maps */
> +       if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +               return vmbus_uio_unmap(uio_res);
> +
> +       TAILQ_REMOVE(uio_res_list, uio_res, next);
> +
> +       /* unmap all resources */
> +       vmbus_uio_unmap(uio_res);
> +
> +       /* free uio resource */
> +       rte_free(uio_res);
> +
> +       /* close fd if in primary process */
> +       close(dev->intr_handle.fd);
> +       if (dev->intr_handle.uio_cfg_fd >= 0) {
> +               close(dev->intr_handle.uio_cfg_fd);
> +               dev->intr_handle.uio_cfg_fd = -1;
> +       }
> +
> +       dev->intr_handle.fd = -1;
> +       dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
> +}
> +
> +static int
> +vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
> +{
> +       struct mapped_vmbus_resource *uio_res;
> +       struct mapped_vmbus_res_list *uio_res_list =
> +                       RTE_TAILQ_CAST(rte_vmbus_uio_tailq.head,
> +                                      mapped_vmbus_res_list);
> +
> +       TAILQ_FOREACH(uio_res, uio_res_list, next) {
> +               int i;
> +
> +               /* skip this element if it doesn't match our id */
> +               if (uuid_compare(uio_res->uuid, dev->device_id))
> +                       continue;
> +
> +               for (i = 0; i != uio_res->nb_maps; i++) {
> +                       void *mapaddr;
> +                       int fd;
> +
> +                       fd = open(uio_res->maps[i].path, O_RDWR);
> +                       if (fd < 0) {
> +                               RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
> +                                       uio_res->maps[i].path, strerror(errno));
> +                               return -1;
> +                       }
> +
> +                       mapaddr = vmbus_map_resource(uio_res->maps[i].addr, fd,
> +                                                    uio_res->maps[i].offset,
> +                                                    uio_res->maps[i].size, 0);
> +                       /* fd is not needed in slave process, close it */
> +                       close(fd);
> +
> +                       if (mapaddr == uio_res->maps[i].addr)
> +                               continue;
> +
> +                       RTE_LOG(ERR, EAL,
> +                               "Cannot mmap device resource file %s to address: %p\n",
> +                               uio_res->maps[i].path,
> +                               uio_res->maps[i].addr);
> +
> +                       /* unmap addrs correctly mapped */
> +                       while (i != 0) {
> +                               --i;
> +                               vmbus_unmap_resource(uio_res->maps[i].addr,
> +                                                    uio_res->maps[i].size);
> +                       }
> +                       return -1;
> +
> +               }
> +               return 0;
> +       }
> +
> +       RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
> +       return 1;
> +}
> +
> +/* map the resources of a vmbus device in virtual memory */
> +int
> +rte_eal_vmbus_map_device(struct rte_vmbus_device *dev)
> +{
> +       struct mapped_vmbus_resource *uio_res;
> +       struct mapped_vmbus_res_list *uio_res_list =
> +               RTE_TAILQ_CAST(rte_vmbus_uio_tailq.head, mapped_vmbus_res_list);
> +       int i, ret, map_idx = 0;
> +
> +       dev->intr_handle.fd = -1;
> +       dev->intr_handle.uio_cfg_fd = -1;
> +       dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
> +
> +       /* secondary processes - use already recorded details */
> +       if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +               return vmbus_uio_map_secondary(dev);
> +
> +       /* allocate uio resource */
> +       uio_res = vmbus_uio_alloc_resource(dev);
> +       if (uio_res == NULL)
> +               return -1;
> +
> +       /* Map all BARs */
> +       for (i = 0; i != VMBUS_MAX_RESOURCE; i++) {
> +               uint64_t phaddr;
> +
> +               /* skip empty BAR */
> +               phaddr = dev->mem_resource[i].phys_addr;
> +               if (phaddr == 0)
> +                       continue;
> +
> +               ret = vmbus_uio_map_resource_by_index(dev, i,
> +                                                     uio_res, map_idx);
> +               if (ret)
> +                       goto error;
> +
> +               map_idx++;
> +       }
> +
> +       uio_res->nb_maps = map_idx;
> +
> +       TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
> +
> +       return 0;
> +error:
> +       for (i = 0; i < map_idx; i++) {
> +               vmbus_unmap_resource(uio_res->maps[i].addr,
> +                                    uio_res->maps[i].size);
> +               rte_free(uio_res->maps[i].path);
> +       }
> +       vmbus_uio_free_resource(dev, uio_res);
> +       return -1;
> +}
> +
> +/* Scan one vmbus sysfs entry, and fill the devices list from it. */
> +static int
> +vmbus_scan_one(const char *name)
> +{
> +       struct rte_vmbus_device *dev, *dev2;
> +       char filename[PATH_MAX];
> +       char dirname[PATH_MAX];
> +       unsigned long tmp;
> +
> +       dev = malloc(sizeof(*dev) + strlen(name) + 1);
> +       if (dev == NULL)
> +               return -1;
> +
> +       memset(dev, 0, sizeof(*dev));
> +       strcpy(dev->sysfs_name, name);
> +       if (dev->sysfs_name == NULL)
> +               goto error;
> +
> +       /* sysfs base directory
> +        *   /sys/bus/vmbus/devices/7a08391f-f5a0-4ac0-9802-d13fd964f8df
> +        * or on older kernel
> +        *   /sys/bus/vmbus/devices/vmbus_1
> +        */
> +       snprintf(dirname, sizeof(dirname), "%s/%s",
> +                SYSFS_VMBUS_DEVICES, name);
> +
> +       /* get device id */
> +       snprintf(filename, sizeof(filename), "%s/device_id", dirname);
> +       if (vmbus_get_sysfs_uuid(filename, dev->device_id) < 0)
> +               goto error;
> +
> +       /* get device class  */
> +       snprintf(filename, sizeof(filename), "%s/class_id", dirname);
> +       if (vmbus_get_sysfs_uuid(filename, dev->class_id) < 0)
> +               goto error;
> +
> +       /* get relid */
> +       snprintf(filename, sizeof(filename), "%s/id", dirname);
> +       if (eal_parse_sysfs_value(filename, &tmp) < 0)
> +               goto error;
> +       dev->relid = tmp;
> +
> +       /* get monitor id */
> +       snprintf(filename, sizeof(filename), "%s/monitor_id", dirname);
> +       if (eal_parse_sysfs_value(filename, &tmp) < 0)
> +               goto error;
> +       dev->monitor_id = tmp;
> +
> +       /* get numa node */
> +       snprintf(filename, sizeof(filename), "%s/numa_node",
> +                dirname);
> +       if (eal_parse_sysfs_value(filename, &tmp) < 0)
> +               /* if no NUMA support, set default to 0 */
> +               dev->device.numa_node = 0;
> +       else
> +               dev->device.numa_node = tmp;
> +
> +       /* device is valid, add in list (sorted) */
> +       RTE_LOG(DEBUG, EAL, "Adding vmbus device %s\n", name);
> +
> +       TAILQ_FOREACH(dev2, &vmbus_device_list, next) {
> +               int ret;
> +
> +               ret = uuid_compare(dev->device_id, dev->device_id);
> +               if (ret > 0)
> +                       continue;
> +
> +               if (ret < 0) {
> +                       TAILQ_INSERT_BEFORE(dev2, dev, next);
> +                       rte_eal_device_insert(&dev->device);
> +               } else { /* already registered */
> +                       memmove(dev2->mem_resource, dev->mem_resource,
> +                               sizeof(dev->mem_resource));
> +                       free(dev);
> +               }
> +               return 0;
> +       }
> +
> +       rte_eal_device_insert(&dev->device);
> +       TAILQ_INSERT_TAIL(&vmbus_device_list, dev, next);
> +
> +       return 0;
> +error:
> +       free(dev);
> +       return -1;
> +}
> +
> +/*
> + * Scan the content of the vmbus, and the devices in the devices list
> + */
> +static int
> +vmbus_scan(void)
> +{
> +       struct dirent *e;
> +       DIR *dir;
> +
> +       dir = opendir(SYSFS_VMBUS_DEVICES);
> +       if (dir == NULL) {
> +               if (errno == ENOENT)
> +                       return 0;
> +
> +               RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
> +                       __func__, strerror(errno));
> +               return -1;
> +       }
> +
> +       while ((e = readdir(dir)) != NULL) {
> +               if (e->d_name[0] == '.')
> +                       continue;
> +
> +               if (vmbus_scan_one(e->d_name) < 0)
> +                       goto error;
> +       }
> +       closedir(dir);
> +       return 0;
> +
> +error:
> +       closedir(dir);
> +       return -1;
> +}
> +
> +/* Init the VMBUS EAL subsystem */
> +int rte_eal_vmbus_init(void)
> +{
> +       /* VMBUS can be disabled */
> +       if (internal_config.no_vmbus)
> +               return 0;
> +
> +       if (vmbus_scan() < 0) {
> +               RTE_LOG(ERR, EAL, "%s(): Cannot scan vmbus\n", __func__);
> +               return -1;
> +       }
> +       return 0;
> +}
> +
> +/* Below is PROBE part of eal_vmbus library */
> +
> +/*
> + * If device ID match, call the devinit() function of the driver.
> + */
> +static int
> +rte_eal_vmbus_probe_one_driver(struct rte_vmbus_driver *dr,
> +                              struct rte_vmbus_device *dev)
> +{
> +       const uuid_t *id_table;
> +
> +       RTE_LOG(DEBUG, EAL, "  probe driver: %s\n", dr->driver.name);
> +
> +       for (id_table = dr->id_table; !uuid_is_null(*id_table); ++id_table) {
> +               struct rte_devargs *args;
> +               char guid[UUID_BUF_SZ];
> +               int ret;
> +
> +               /* skip devices not assocaited with this device class */
> +               if (uuid_compare(*id_table, dev->class_id) != 0)
> +                       continue;
> +
> +               uuid_unparse(dev->device_id, guid);
> +               RTE_LOG(INFO, EAL, "VMBUS device %s on NUMA socket %i\n",
> +                       guid, dev->device.numa_node);
> +
> +               /* no initialization when blacklisted, return without error */
> +               args = dev->device.devargs;
> +               if (args && args->type == RTE_DEVTYPE_BLACKLISTED_VMBUS) {
> +                       RTE_LOG(INFO, EAL, "  Device is blacklisted, not initializing\n");
> +                       return 1;
> +               }
> +
> +               RTE_LOG(INFO, EAL, "  probe driver: %s\n", dr->driver.name);
> +
> +               /* map resources for device */
> +               ret = rte_eal_vmbus_map_device(dev);
> +               if (ret != 0)
> +                       return ret;
> +
> +               /* reference driver structure */
> +               dev->driver = dr;
> +
> +               /* call the driver probe() function */
> +               ret = dr->probe(dr, dev);
> +               if (ret)
> +                       dev->driver = NULL;
> +
> +               return ret;
> +       }
> +
> +       /* return positive value if driver doesn't support this device */
> +       return 1;
> +}
> +
> +
> +/*
> + * If vendor/device ID match, call the remove() function of the
> + * driver.
> + */
> +static int
> +vmbus_detach_dev(struct rte_vmbus_driver *dr,
> +                struct rte_vmbus_device *dev)
> +{
> +       const uuid_t *id_table;
> +
> +       for (id_table = dr->id_table; !uuid_is_null(*id_table); ++id_table) {
> +               char guid[UUID_BUF_SZ];
> +
> +               /* skip devices not assocaited with this device class */
> +               if (uuid_compare(*id_table, dev->class_id) != 0)
> +                       continue;
> +
> +               uuid_unparse(dev->device_id, guid);
> +               RTE_LOG(INFO, EAL, "VMBUS device %s on NUMA socket %i\n",
> +                       guid, dev->device.numa_node);
> +
> +               RTE_LOG(DEBUG, EAL, "  remove driver: %s\n", dr->driver.name);
> +
> +               if (dr->remove && (dr->remove(dev) < 0))
> +                       return -1;      /* negative value is an error */
> +
> +               /* clear driver structure */
> +               dev->driver = NULL;
> +
> +               vmbus_uio_unmap_resource(dev);
> +               return 0;
> +       }
> +
> +       /* return positive value if driver doesn't support this device */
> +       return 1;
> +}
> +
> +/*
> + * call the devinit() function of all
> + * registered drivers for the vmbus device. Return -1 if no driver is
> + * found for this class of vmbus device.
> + * The present assumption is that we have drivers only for vmbus network
> + * devices. That's why we don't check driver's id_table now.
> + */
> +static int
> +vmbus_probe_all_drivers(struct rte_vmbus_device *dev)
> +{
> +       struct rte_vmbus_driver *dr = NULL;
> +       int ret;
> +
> +       TAILQ_FOREACH(dr, &vmbus_driver_list, next) {
> +               ret = rte_eal_vmbus_probe_one_driver(dr, dev);
> +               if (ret < 0) {
> +                       /* negative value is an error */
> +                       RTE_LOG(ERR, EAL, "Failed to probe driver %s\n",
> +                               dr->driver.name);
> +                       return -1;
> +               }
> +               /* positive value means driver doesn't support it */
> +               if (ret > 0)
> +                       continue;
> +
> +               return 0;
> +       }
> +
> +       return 1;
> +}
> +
> +
> +/*
> + * If device ID matches, call the remove() function of all
> + * registered driver for the given device. Return -1 if initialization
> + * failed, return 1 if no driver is found for this device.
> + */
> +static int
> +vmbus_detach_all_drivers(struct rte_vmbus_device *dev)
> +{
> +       struct rte_vmbus_driver *dr;
> +       int rc = 0;
> +
> +       if (dev == NULL)
> +               return -1;
> +
> +       TAILQ_FOREACH(dr, &vmbus_driver_list, next) {
> +               rc = vmbus_detach_dev(dr, dev);
> +               if (rc < 0)
> +                       /* negative value is an error */
> +                       return -1;
> +               if (rc > 0)
> +                       /* positive value means driver doesn't support it */
> +                       continue;
> +               return 0;
> +       }
> +       return 1;
> +}
> +
> +/* Detach device specified by its VMBUS id */
> +int
> +rte_eal_vmbus_detach(uuid_t device_id)
> +{
> +       struct rte_vmbus_device *dev;
> +       char ubuf[UUID_BUF_SZ];
> +
> +       TAILQ_FOREACH(dev, &vmbus_device_list, next) {
> +               if (uuid_compare(dev->device_id, device_id) != 0)
> +                       continue;
> +
> +               if (vmbus_detach_all_drivers(dev) < 0)
> +                       goto err_return;
> +
> +               TAILQ_REMOVE(&vmbus_device_list, dev, next);
> +               free(dev);
> +               return 0;
> +       }
> +       return -1;
> +
> +err_return:
> +       uuid_unparse(device_id, ubuf);
> +       RTE_LOG(WARNING, EAL, "Requested device %s cannot be used\n",
> +               ubuf);
> +       return -1;
> +}
> +
> +/*
> + * Scan the vmbus, and call the devinit() function for
> + * all registered drivers that have a matching entry in its id_table
> + * for discovered devices.
> + */
> +int
> +rte_eal_vmbus_probe(void)
> +{
> +       struct rte_vmbus_device *dev = NULL;
> +
> +       TAILQ_FOREACH(dev, &vmbus_device_list, next) {
> +               char ubuf[UUID_BUF_SZ];
> +
> +               uuid_unparse(dev->device_id, ubuf);
> +
> +               RTE_LOG(DEBUG, EAL, "Probing driver for device %s ...\n",
> +                       ubuf);
> +               vmbus_probe_all_drivers(dev);
> +       }
> +       return 0;
> +}
> +
> +/* register vmbus driver */
> +void
> +rte_eal_vmbus_register(struct rte_vmbus_driver *driver)
> +{
> +       TAILQ_INSERT_TAIL(&vmbus_driver_list, driver, next);
> +}
> +
> +/* unregister vmbus driver */
> +void
> +rte_eal_vmbus_unregister(struct rte_vmbus_driver *driver)
> +{
> +       TAILQ_REMOVE(&vmbus_driver_list, driver, next);
> +}
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 7c212096..b69af0f0 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -3334,3 +3334,93 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
>                                 -ENOTSUP);
>         return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
>  }
> +
> +
> +#ifdef RTE_LIBRTE_HV_PMD
> +int
> +rte_eth_dev_vmbus_probe(struct rte_vmbus_driver *vmbus_drv,
> +                       struct rte_vmbus_device *vmbus_dev)
> +{
> +       struct eth_driver  *eth_drv = (struct eth_driver *)vmbus_drv;
> +       struct rte_eth_dev *eth_dev;
> +       char ustr[UUID_BUF_SZ];
> +       int diag;
> +
> +       uuid_unparse(vmbus_dev->device_id, ustr);
> +
> +       eth_dev = rte_eth_dev_allocate(ustr);
> +       if (eth_dev == NULL)
> +               return -ENOMEM;
> +
> +       if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +               eth_dev->data->dev_private = rte_zmalloc("ethdev private structure",
> +                                 eth_drv->dev_private_size,
> +                                 RTE_CACHE_LINE_SIZE);
> +               if (eth_dev->data->dev_private == NULL)
> +                       rte_panic("Cannot allocate memzone for private port data\n");
> +       }
> +
> +       eth_dev->device = &vmbus_dev->device;
> +       eth_dev->driver = eth_drv;
> +       eth_dev->data->rx_mbuf_alloc_failed = 0;
> +
> +       /* init user callbacks */
> +       TAILQ_INIT(&(eth_dev->link_intr_cbs));
> +
> +       /*
> +        * Set the default maximum frame size.
> +        */
> +       eth_dev->data->mtu = ETHER_MTU;

Initialization of default values has moved into rte_eth_dev_allocate().

> +
> +       /* Invoke PMD device initialization function */
> +       diag = (*eth_drv->eth_dev_init)(eth_dev);
> +       if (diag == 0)
> +               return 0;
> +
> +       RTE_PMD_DEBUG_TRACE("driver %s: eth_dev_init(%s) failed\n",
> +                           vmbus_drv->driver.name, ustr);
> +
> +       if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +               rte_free(eth_dev->data->dev_private);
> +
> +       return diag;
> +}
> +
> +int
> +rte_eth_dev_vmbus_remove(struct rte_vmbus_device *vmbus_dev)
> +{
> +       const struct eth_driver *eth_drv;
> +       struct rte_eth_dev *eth_dev;
> +       char ustr[UUID_BUF_SZ];
> +       int ret;
> +
> +       if (vmbus_dev == NULL)
> +               return -EINVAL;
> +
> +       uuid_unparse(vmbus_dev->device_id, ustr);
> +       eth_dev = rte_eth_dev_allocated(ustr);
> +       if (eth_dev == NULL)
> +               return -ENODEV;
> +
> +       eth_drv = (const struct eth_driver *)vmbus_dev->driver;
> +
> +       /* Invoke PMD device uninit function */
> +       if (*eth_drv->eth_dev_uninit) {
> +               ret = (*eth_drv->eth_dev_uninit)(eth_dev);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       /* free ether device */
> +       rte_eth_dev_release_port(eth_dev);
> +
> +       if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> +               rte_free(eth_dev->data->dev_private);
> +
> +       eth_dev->device = NULL;
> +       eth_dev->driver = NULL;
> +       eth_dev->data = NULL;
> +
> +       return 0;
> +}
> +#endif
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 1a62a322..2a8c1eed 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -180,6 +180,9 @@ extern "C" {
>  #include <rte_log.h>
>  #include <rte_interrupts.h>
>  #include <rte_pci.h>
> +#ifdef RTE_LIBRTE_HV_PMD
> +#include <rte_vmbus.h>
> +#endif
>  #include <rte_dev.h>
>  #include <rte_devargs.h>
>  #include <rte_errno.h>
> @@ -1908,6 +1911,17 @@ struct rte_pci_eth_driver {
>         struct eth_driver       eth_drv;        /**< Ethernet driver. */
>  };
>
> +#ifdef RTE_LIBRTE_HV_PMD
> +/**
> + * @internal
> + * The structure associated with a PMD VMBUS Ethernet driver.
> + */
> +struct rte_vmbus_eth_driver {
> +       struct rte_vmbus_driver vmbus_drv;      /**< Underlying VMBUS driver. */
> +       struct eth_driver       eth_drv;        /**< Ethernet driver. */
> +};
> +#endif
> +
>  /**
>   * Convert a numerical speed in Mbps to a bitmap flag that can be used in
>   * the bitmap link_speeds of the struct rte_eth_conf
> @@ -4543,6 +4557,23 @@ int rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
>   */
>  int rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev);
>
> +#ifdef RTE_LIBRTE_HV_PMD
> +/**
> + * @internal
> + * Wrapper for use by vmbus drivers as a .probe function to attach to a ethdev
> + * interface.
> + */
> +int rte_eth_dev_vmbus_probe(struct rte_vmbus_driver *vmbus_drv,
> +                         struct rte_vmbus_device *vmbus_dev);
> +
> +/**
> + * @internal
> + * Wrapper for use by vmbus drivers as a .remove function to detach a ethdev
> + * interface.
> + */
> +int rte_eth_dev_vmbus_remove(struct rte_vmbus_device *vmbus_dev);
> +#endif

I don't think that replicating the PCI probe/remove wrappers is the
right thing to do. To me it looks like this should move into the
rte_vmbus_driver's probe function instead. That way the ethdev header
can decoupled from the low-level device implementations.


> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index f75f0e24..6b304084 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -130,6 +130,7 @@ ifeq ($(CONFIG_RTE_LIBRTE_VHOST),y)
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_VHOST)      += -lrte_pmd_vhost
>  endif # $(CONFIG_RTE_LIBRTE_VHOST)
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD)    += -lrte_pmd_vmxnet3_uio
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_HV_PMD)        += -luuid
>
>  ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y)
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB)    += -lrte_pmd_aesni_mb
> --
> 2.11.0
>

^ permalink raw reply

* Re: [PATCH v3 0/8] Add crypto PMD optimized for ARMv8
From: Zbigniew Bodek @ 2017-01-10 17:50 UTC (permalink / raw)
  To: De Lara Guarch, Pablo, dev@dpdk.org
  Cc: Doherty, Declan, jerin.jacob@caviumnetworks.com
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D897476C1810@IRSMSX108.ger.corp.intel.com>

Hello Pablo,

Please check my answers in-line below.

Kind regards
Zbigniew

On 10.01.2017 18:11, De Lara Guarch, Pablo wrote:
> Hi Zbigniew,
>
>
>> -----Original Message-----
>> From: zbigniew.bodek@caviumnetworks.com
>> [mailto:zbigniew.bodek@caviumnetworks.com]
>> Sent: Wednesday, January 04, 2017 5:33 PM
>> To: dev@dpdk.org
>> Cc: De Lara Guarch, Pablo; Doherty, Declan;
>> jerin.jacob@caviumnetworks.com; Zbigniew Bodek
>> Subject: [PATCH v3 0/8] Add crypto PMD optimized for ARMv8
>>
>> From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>
> ...
>
>>
>> Zbigniew Bodek (8):
>>   mk: fix build of assembly files for ARM64
>>   lib: add cryptodev type for the upcoming ARMv8 PMD
>>   crypto/armv8: add PMD optimized for ARMv8 processors
>>   mk/crypto/armv8: add PMD to the build system
>>   doc/armv8: update documentation about crypto PMD
>>   crypto/armv8: enable ARMv8 PMD in the configuration
>>   crypto/armv8: update MAINTAINERS entry for ARMv8 crypto
>>   app/test: add ARMv8 crypto tests and test vectors
>
> Thanks for this patchset.
>
> Could you change the titles of some of these patches?
> The prefix should be "mk:" and not "mk/crypto/armv8", for instance.
> The other ones that should be changed are "doc/armv8" -> "doc" and "crypto/armv8: update MAINTAINERS" to "MAINTAINERS:".
>
> I can do this for you, if you are OK with these changes.

I'm OK with the changes and I will appreciate changing those names if 
this is not an inconvenience for you.

>
> Apart from this, can anyone review these changes? I do not have access to an ARM board,
> so it is a bit difficult for me to review it.

I would like to add that I can help with the build and installation in 
case the documentation is not sufficient (this would also mean changing 
the documentation).

>
> Thanks,
> Pablo
>

^ permalink raw reply

* Re: [PATCH 2/8] i40e: don't refer to eth_dev->pci_dev
From: Stephen Hemminger @ 2017-01-10 17:57 UTC (permalink / raw)
  To: Jan Blunck; +Cc: dev, Stephen Hemminger
In-Reply-To: <CALe+Z01UWj4pTK-9ciyoYsdddHv3+hOGYOf7CWNODnTMSWe57A@mail.gmail.com>

On Tue, 10 Jan 2017 13:08:30 +0100
Jan Blunck <jblunck@infradead.org> wrote:

> On Sat, Jan 7, 2017 at 7:17 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > Later patches remove pci_dev from the ethernet device structure.
> > Fix the i40e code to just use it's own name when forming zone name.
> >
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> > ---
> >  drivers/net/i40e/i40e_fdir.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> > index 335bf15c..68a2523c 100644
> > --- a/drivers/net/i40e/i40e_fdir.c
> > +++ b/drivers/net/i40e/i40e_fdir.c
> > @@ -250,8 +250,7 @@ i40e_fdir_setup(struct i40e_pf *pf)
> >         }
> >
> >         /* reserve memory for the fdir programming packet */
> > -       snprintf(z_name, sizeof(z_name), "%s_%s_%d",
> > -                       eth_dev->driver->pci_drv.driver.name,
> > +       snprintf(z_name, sizeof(z_name), "i40e_%s_%d",  
> 
> The driver is called 'net_i40e'.

It really doesn't matter. The memory name is just so that primary and secondary
find the same resources.  Having net_ on the front doesn't change or help.

^ permalink raw reply

* Re: [PATCH 7/8] ethdev: break ethernet driver and pci_driver connection
From: Ferruh Yigit @ 2017-01-10 17:58 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Stephen Hemminger, Shreyansh Jain
In-Reply-To: <416d1526-f9d9-fa03-b04d-f63284a2df5d@intel.com>

On 1/10/2017 1:59 PM, Ferruh Yigit wrote:
> On 1/7/2017 6:17 PM, Stephen Hemminger wrote:
>> There are multiple buses and device types now. Therefore it no longer
>> makes sense that PCI driver information is part of the Ethernet driver
>> structure.
>>
>> This patch removes pci_driver from eth_driver and introduces a
>> new combined structure for use in all existing PMD's. The rationale
>> is that although all existing PCI drivers are Ethernet drivers,
>> it make sense that future projects may want to support PCI devices
>> that are not Ethernet.
>>
>> It also removes the requirement that driver is first element in
>> PCI driver structure.
>>
>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
>> ---
> 
> <...>
> 
>>  /**
>> + * @internal
>> + * The structure associated with a PMD PCI Ethernet driver.
>> + */
>> +struct rte_pci_eth_driver {
>> +	struct rte_pci_driver	pci_drv;	/**< Underlying PCI driver. */
>> +	struct eth_driver	eth_drv;	/**< Ethernet driver. */
>> +};
> 
> So do we need to add rte_vdev_eth_driver struct for virtual drivers, or
> need to add rte_pci_cryptodev_driver struct for pci crypto devices?
> 
> Can this be done in a more generic way? After Shreyansh's patches, there
> will be rte_device, rte_driver abstractions, can they be useful?

What do you think separating bus (pci) and functionality (eth/crypto)
driver structs, to make them less coupled. This makes combining bus /
function pairs easily.

I will send a patch as reply to this mail, it is not the complete patch,
but just to give the idea. It is based on Shreyansh's patchet.

> 
> <...>
> 

^ permalink raw reply

* Re: [PATCH 7/8] ethdev: break ethernet driver and pci_driver connection
From: Stephen Hemminger @ 2017-01-10 18:03 UTC (permalink / raw)
  To: Jan Blunck; +Cc: dev, Stephen Hemminger
In-Reply-To: <CALe+Z00Whxvzbhcqyopt4KBNnEHk2+XZQCr4vvWua9WWUZofLA@mail.gmail.com>

On Tue, 10 Jan 2017 17:11:15 +0100
Jan Blunck <jblunck@infradead.org> wrote:

> On Sat, Jan 7, 2017 at 7:17 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > There are multiple buses and device types now. Therefore it no longer
> > makes sense that PCI driver information is part of the Ethernet driver
> > structure.  
> 
> The Ethernet driver itself doesn't over alot of value from an
> abstraction point of view. Its questionable if there ever will be an
> Ethernet driver that is able to operate on different types of
> low-level devices. The virtual devices are anyway able to operate
> without an Ethernet driver structure. Most of that functionality
> should get moved either into the bus abstraction or the low-level
> device probe function.

I agree that that 'struct eth_driver' is not adding a lot now.
It should really be all folded back into 'struct rte_driver'.
The concept of init, uninit and private data are all generic and not
really specific to ethernet in anyway.

If we kill off eth_driver then PCI devices only have rte_pci_driver
and VMBUS can have rte_vmbus_driver.

^ permalink raw reply

* [PATCH 1/2] add rte_bus->probe
From: Ferruh Yigit @ 2017-01-10 18:02 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Shreyansh Jain, Jan Blunck, Ferruh Yigit
In-Reply-To: <bec2dfb0-d28f-a147-d9ea-1b06139e858c@intel.com>

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 lib/librte_eal/common/eal_common_bus.c  | 7 ++++---
 lib/librte_eal/common/include/rte_bus.h | 3 +++
 lib/librte_eal/linuxapp/eal/eal_pci.c   | 1 +
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index f8c2e03..e8d1143 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -145,6 +145,7 @@ rte_eal_bus_register(struct rte_bus *bus)
 	/* A bus should mandatorily have the scan and match implemented */
 	RTE_VERIFY(bus->scan);
 	RTE_VERIFY(bus->match);
+	RTE_VERIFY(bus->probe);
 
 	/* Initialize the driver and device list associated with the bus */
 	TAILQ_INIT(&(bus->driver_list));
@@ -195,19 +196,19 @@ rte_eal_bus_scan(void)
 }
 
 static int
-perform_probe(struct rte_bus *bus __rte_unused, struct rte_driver *driver,
+perform_probe(struct rte_bus *bus, struct rte_driver *driver,
 	      struct rte_device *device)
 {
 	int ret;
 
-	if (!driver->probe) {
+	if (!bus->probe) {
 		RTE_LOG(ERR, EAL, "Driver (%s) doesn't support probe.\n",
 			driver->name);
 		/* This is not an error - just a badly implemented PMD */
 		return 0;
 	}
 
-	ret = driver->probe(driver, device);
+	ret = bus->probe(driver, device);
 	if (ret < 0)
 		/* One of the probes failed */
 		RTE_LOG(ERR, EAL, "Probe failed for (%s).\n", driver->name);
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 07c30c4..ce1f56a 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -135,6 +135,8 @@ typedef int (*bus_scan_t)(struct rte_bus *bus);
  */
 typedef int (*bus_match_t)(struct rte_driver *drv, struct rte_device *dev);
 
+typedef int (*bus_probe_t)(struct rte_driver *drv, struct rte_device *dev);
+
 /**
  * A structure describing a generic bus.
  */
@@ -147,6 +149,7 @@ struct rte_bus {
 	const char *name;            /**< Name of the bus */
 	bus_scan_t scan;            /**< Scan for devices attached to bus */
 	bus_match_t match;
+	bus_probe_t probe;
 	/**< Match device with drivers associated with the bus */
 };
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 314effa..837adf6 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -726,6 +726,7 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
 struct rte_bus pci_bus = {
 	.scan = rte_eal_pci_scan,
 	.match = rte_eal_pci_match,
+	.probe = rte_eal_pci_probe,
 };
 
 RTE_REGISTER_BUS(pci, pci_bus);
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/2] separate bus and functionality driver structs
From: Ferruh Yigit @ 2017-01-10 18:02 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Shreyansh Jain, Jan Blunck, Ferruh Yigit
In-Reply-To: <20170110180250.10625-1-ferruh.yigit@intel.com>

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/bnx2x/bnx2x_ethdev.c        | 44 ++++++++++++++---------------
 drivers/net/cxgbe/cxgbe_ethdev.c        | 22 +++++++--------
 drivers/net/e1000/em_ethdev.c           | 26 ++++++++---------
 drivers/net/e1000/igb_ethdev.c          | 50 ++++++++++++++++-----------------
 drivers/net/ena/ena_ethdev.c            | 22 +++++++--------
 drivers/net/enic/enic_ethdev.c          | 22 +++++++--------
 drivers/net/fm10k/fm10k_ethdev.c        | 26 ++++++++---------
 drivers/net/i40e/i40e_ethdev.c          | 26 ++++++++---------
 drivers/net/i40e/i40e_ethdev_vf.c       | 24 ++++++++--------
 drivers/net/ixgbe/ixgbe_ethdev.c        | 50 ++++++++++++++++-----------------
 lib/librte_eal/common/eal_common_pci.c  |  4 +--
 lib/librte_eal/common/include/rte_pci.h |  3 +-
 lib/librte_ether/rte_ethdev.c           | 26 +++++++++++++----
 lib/librte_ether/rte_ethdev.h           |  7 +++--
 14 files changed, 183 insertions(+), 169 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 2dbd782..bb1937e 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -618,42 +618,42 @@ eth_bnx2xvf_dev_init(struct rte_eth_dev *eth_dev)
 	return bnx2x_common_dev_init(eth_dev, 1);
 }
 
-static struct eth_driver rte_bnx2x_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_bnx2x_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+static struct eth_driver rte_bnx2x_pmd_eth_drv = {
+	.eth_dev_init = eth_bnx2x_dev_init,
+	.dev_private_size = sizeof(struct bnx2x_softc),
+};
+
+static struct rte_pci_driver rte_bnx2x_pmd_pci_drv = {
+	.driver = {
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
-	.eth_dev_init = eth_bnx2x_dev_init,
-	.dev_private_size = sizeof(struct bnx2x_softc),
+	.id_table = pci_id_bnx2x_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+	.func_drv = &rte_bnx2x_pmd_eth_drv.driver,
 };
 
 /*
  * virtual function driver struct
  */
-static struct eth_driver rte_bnx2xvf_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_bnx2xvf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+static struct eth_driver rte_bnx2xvf_pmd_eth_drv = {
+	.eth_dev_init = eth_bnx2xvf_dev_init,
+	.dev_private_size = sizeof(struct bnx2x_softc),
+};
+
+static struct rte_pci_driver rte_bnx2xvf_pmd_pci_drv = {
+	.driver = {
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
-	.eth_dev_init = eth_bnx2xvf_dev_init,
-	.dev_private_size = sizeof(struct bnx2x_softc),
+	.id_table = pci_id_bnx2xvf_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+	.func_drv = &rte_bnx2xvf_pmd_eth_drv.driver,
 };
 
-RTE_PMD_REGISTER_PCI(net_bnx2x, rte_bnx2x_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_bnx2x, rte_bnx2x_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_bnx2x, pci_id_bnx2x_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_bnx2x, "* igb_uio | uio_pci_generic | vfio");
-RTE_PMD_REGISTER_PCI(net_bnx2xvf, rte_bnx2xvf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_bnx2xvf, rte_bnx2xvf_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_bnx2xvf, pci_id_bnx2xvf_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_bnx2xvf, "* igb_uio | vfio");
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 7718d02..df9a324 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -1039,21 +1039,21 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
 	return err;
 }
 
-static struct eth_driver rte_cxgbe_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = cxgb4_pci_tbl,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+static struct eth_driver rte_cxgbe_pmd_eth_drv = {
+	.eth_dev_init = eth_cxgbe_dev_init,
+	.dev_private_size = sizeof(struct port_info),
+};
+
+static struct rte_pci_driver rte_cxgbe_pmd_pci_drv = {
+	.driver = {
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
-	.eth_dev_init = eth_cxgbe_dev_init,
-	.dev_private_size = sizeof(struct port_info),
+	.id_table = cxgb4_pci_tbl,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+	.func_drv = &rte_cxgbe_pmd_eth_drv.driver,
 };
 
-RTE_PMD_REGISTER_PCI(net_cxgbe, rte_cxgbe_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_cxgbe, rte_cxgbe_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_cxgbe, cxgb4_pci_tbl);
 RTE_PMD_REGISTER_KMOD_DEP(net_cxgbe, "* igb_uio | uio_pci_generic | vfio");
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 8758aaa..fa5f650 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -415,23 +415,23 @@ eth_em_dev_uninit(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
-static struct eth_driver rte_em_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_em_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
-		.probe = rte_eth_dev_pci_probe,
-		.remove = rte_eth_dev_pci_remove,
-	},
+static struct eth_driver rte_em_pmd_eth_drv = {
 	.eth_dev_init = eth_em_dev_init,
 	.eth_dev_uninit = eth_em_dev_uninit,
 	.dev_private_size = sizeof(struct e1000_adapter),
 };
 
+static struct rte_pci_driver rte_em_pmd_pci_drv = {
+	.driver = {
+		.probe = rte_eth_dev_pci_probe,
+		.remove = rte_eth_dev_pci_remove,
+	},
+	.id_table = pci_id_em_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
+		RTE_PCI_DRV_DETACHABLE,
+	.func_drv = &rte_em_pmd_eth_drv.driver,
+};
+
 static int
 em_hw_init(struct e1000_hw *hw)
 {
@@ -1851,6 +1851,6 @@ eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
 	return 0;
 }
 
-RTE_PMD_REGISTER_PCI(net_e1000_em, rte_em_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_e1000_em, rte_em_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_e1000_em, pci_id_em_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_e1000_em, "* igb_uio | uio_pci_generic | vfio");
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 76d73cd..9563c46 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1082,42 +1082,42 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
-static struct eth_driver rte_igb_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_igb_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
-		.probe = rte_eth_dev_pci_probe,
-		.remove = rte_eth_dev_pci_remove,
-	},
+static struct eth_driver rte_igb_pmd_eth_drv = {
 	.eth_dev_init = eth_igb_dev_init,
 	.eth_dev_uninit = eth_igb_dev_uninit,
 	.dev_private_size = sizeof(struct e1000_adapter),
 };
 
-/*
- * virtual function driver struct
- */
-static struct eth_driver rte_igbvf_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_igbvf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+static struct rte_pci_driver rte_igb_pmd_pci_drv = {
+	.driver = {
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
+	.id_table = pci_id_igb_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
+		RTE_PCI_DRV_DETACHABLE,
+	.func_drv = &rte_igb_pmd_eth_drv.driver,
+};
+
+/*
+ * virtual function driver struct
+ */
+static struct eth_driver rte_igbvf_pmd_eth_drv = {
 	.eth_dev_init = eth_igbvf_dev_init,
 	.eth_dev_uninit = eth_igbvf_dev_uninit,
 	.dev_private_size = sizeof(struct e1000_adapter),
 };
 
+static struct rte_pci_driver rte_igbvf_pmd_pci_drv = {
+	.driver = {
+		.probe = rte_eth_dev_pci_probe,
+		.remove = rte_eth_dev_pci_remove,
+	},
+	.id_table = pci_id_igbvf_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+	.func_drv = &rte_igbvf_pmd_eth_drv.driver,
+};
+
 static void
 igb_vmdq_vlan_hw_filter_enable(struct rte_eth_dev *dev)
 {
@@ -5261,9 +5261,9 @@ eth_igb_configure_msix_intr(struct rte_eth_dev *dev)
 	E1000_WRITE_FLUSH(hw);
 }
 
-RTE_PMD_REGISTER_PCI(net_e1000_igb, rte_igb_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_e1000_igb, rte_igb_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_e1000_igb, pci_id_igb_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_e1000_igb, "* igb_uio | uio_pci_generic | vfio");
-RTE_PMD_REGISTER_PCI(net_e1000_igb_vf, rte_igbvf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_e1000_igb_vf, rte_igbvf_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_e1000_igb_vf, pci_id_igbvf_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_e1000_igb_vf, "* igb_uio | vfio");
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index ecdd015..ad5b0a9 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1756,21 +1756,21 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	return sent_idx;
 }
 
-static struct eth_driver rte_ena_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_ena_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+static struct eth_driver rte_ena_pmd_eth_drv = {
+	.eth_dev_init = eth_ena_dev_init,
+	.dev_private_size = sizeof(struct ena_adapter),
+};
+
+static struct rte_pci_driver rte_ena_pmd_pci_drv = {
+	.driver = {
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
-	.eth_dev_init = eth_ena_dev_init,
-	.dev_private_size = sizeof(struct ena_adapter),
+	.id_table = pci_id_ena_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+	.func_drv = &rte_ena_pmd_eth_drv.driver,
 };
 
-RTE_PMD_REGISTER_PCI(net_ena, rte_ena_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_ena, rte_ena_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_ena, pci_id_ena_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ena, "* igb_uio | uio_pci_generic | vfio");
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 00cf67b..0cb6400 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -634,21 +634,21 @@ static int eth_enicpmd_dev_init(struct rte_eth_dev *eth_dev)
 	return enic_probe(enic);
 }
 
-static struct eth_driver rte_enic_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_enic_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+static struct eth_driver rte_enic_pmd_eth_drv = {
+	.eth_dev_init = eth_enicpmd_dev_init,
+	.dev_private_size = sizeof(struct enic),
+};
+
+static struct rte_pci_driver rte_enic_pmd_pci_drv = {
+	.driver = {
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
-	.eth_dev_init = eth_enicpmd_dev_init,
-	.dev_private_size = sizeof(struct enic),
+	.id_table = pci_id_enic_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+	.func_drv = &rte_enic_pmd_eth_drv.driver,
 };
 
-RTE_PMD_REGISTER_PCI(net_enic, rte_enic_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_enic, rte_enic_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_enic, pci_id_enic_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_enic, "* igb_uio | uio_pci_generic | vfio");
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 9760fb7..4c84484 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -3077,23 +3077,23 @@ static const struct rte_pci_id pci_id_fm10k_map[] = {
 	{ .vendor_id = 0, /* sentinel */ },
 };
 
-static struct eth_driver rte_pmd_fm10k = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_fm10k_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
-		.probe = rte_eth_dev_pci_probe,
-		.remove = rte_eth_dev_pci_remove,
-	},
+static struct eth_driver rte_pmd_fm10k_eth_drv = {
 	.eth_dev_init = eth_fm10k_dev_init,
 	.eth_dev_uninit = eth_fm10k_dev_uninit,
 	.dev_private_size = sizeof(struct fm10k_adapter),
 };
 
-RTE_PMD_REGISTER_PCI(net_fm10k, rte_pmd_fm10k.pci_drv);
+static struct rte_pci_driver rte_pmd_fm10k_pci_drv = {
+	.driver = {
+		.probe = rte_eth_dev_pci_probe,
+		.remove = rte_eth_dev_pci_remove,
+	},
+	.id_table = pci_id_fm10k_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
+		RTE_PCI_DRV_DETACHABLE,
+	.func_drv = &rte_pmd_fm10k_eth_drv.driver,
+};
+
+RTE_PMD_REGISTER_PCI(net_fm10k, rte_pmd_fm10k_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_fm10k, pci_id_fm10k_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_fm10k, "* igb_uio | uio_pci_generic | vfio");
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 24683a9..9443c51 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -623,23 +623,23 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
 #define I40E_NB_TXQ_PRIO_XSTATS (sizeof(rte_i40e_txq_prio_strings) / \
 		sizeof(rte_i40e_txq_prio_strings[0]))
 
-static struct eth_driver rte_i40e_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_i40e_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
-		.probe = rte_eth_dev_pci_probe,
-		.remove = rte_eth_dev_pci_remove,
-	},
+static struct eth_driver rte_i40e_pmd_eth_drv = {
 	.eth_dev_init = eth_i40e_dev_init,
 	.eth_dev_uninit = eth_i40e_dev_uninit,
 	.dev_private_size = sizeof(struct i40e_adapter),
 };
 
+static struct rte_pci_driver rte_i40e_pmd_pci_drv = {
+	.driver = {
+		.probe = rte_eth_dev_pci_probe,
+		.remove = rte_eth_dev_pci_remove,
+	},
+	.id_table = pci_id_i40e_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
+		RTE_PCI_DRV_DETACHABLE,
+	.func_drv = &rte_i40e_pmd_eth_drv.driver,
+};
+
 static inline int
 rte_i40e_dev_atomic_read_link_status(struct rte_eth_dev *dev,
 				     struct rte_eth_link *link)
@@ -668,7 +668,7 @@ rte_i40e_dev_atomic_write_link_status(struct rte_eth_dev *dev,
 	return 0;
 }
 
-RTE_PMD_REGISTER_PCI(net_i40e, rte_i40e_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_i40e, rte_i40e_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_i40e, pci_id_i40e_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_i40e, "* igb_uio | uio_pci_generic | vfio");
 
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 7b97ed3..c548955 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1533,23 +1533,23 @@ i40evf_dev_uninit(struct rte_eth_dev *eth_dev)
 /*
  * virtual function driver struct
  */
-static struct eth_driver rte_i40evf_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_i40evf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
-		.probe = rte_eth_dev_pci_probe,
-		.remove = rte_eth_dev_pci_remove,
-	},
+static struct eth_driver rte_i40evf_pmd_eth_drv = {
 	.eth_dev_init = i40evf_dev_init,
 	.eth_dev_uninit = i40evf_dev_uninit,
 	.dev_private_size = sizeof(struct i40e_adapter),
 };
 
-RTE_PMD_REGISTER_PCI(net_i40e_vf, rte_i40evf_pmd.pci_drv);
+static struct rte_pci_driver rte_i40evf_pmd_pci_drv = {
+	.driver = {
+		.probe = rte_eth_dev_pci_probe,
+		.remove = rte_eth_dev_pci_remove,
+	},
+	.id_table = pci_id_i40evf_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+	.func_drv = &rte_i40evf_pmd_eth_drv.driver,
+};
+
+RTE_PMD_REGISTER_PCI(net_i40e_vf, rte_i40evf_pmd_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_i40e_vf, pci_id_i40evf_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_i40e_vf, "* igb_uio | vfio");
 
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index b17ed1a..520b2af 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1550,42 +1550,42 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 	return 0;
 }
 
-static struct eth_driver rte_ixgbe_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_ixgbe_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-			RTE_PCI_DRV_DETACHABLE,
-		.probe = rte_eth_dev_pci_probe,
-		.remove = rte_eth_dev_pci_remove,
-	},
+static struct eth_driver rte_ixgbe_eth_drv = {
 	.eth_dev_init = eth_ixgbe_dev_init,
 	.eth_dev_uninit = eth_ixgbe_dev_uninit,
 	.dev_private_size = sizeof(struct ixgbe_adapter),
 };
 
-/*
- * virtual function driver struct
- */
-static struct eth_driver rte_ixgbevf_pmd = {
-	.pci_drv = {
-		.driver = {
-			.probe = rte_eal_pci_probe,
-			.remove = rte_eal_pci_remove,
-		},
-		.id_table = pci_id_ixgbevf_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+static struct rte_pci_driver rte_ixgbe_pci_drv = {
+	.driver = {
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
 	},
+	.id_table = pci_id_ixgbe_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
+		RTE_PCI_DRV_DETACHABLE,
+	.func_drv = &rte_ixgbe_eth_drv.driver,
+};
+
+/*
+ * virtual function driver struct
+ */
+static struct eth_driver rte_ixgbevf_eth_drv = {
 	.eth_dev_init = eth_ixgbevf_dev_init,
 	.eth_dev_uninit = eth_ixgbevf_dev_uninit,
 	.dev_private_size = sizeof(struct ixgbe_adapter),
 };
 
+static struct rte_pci_driver rte_ixgbevf_pci_drv = {
+	.driver = {
+		.probe = rte_eth_dev_pci_probe,
+		.remove = rte_eth_dev_pci_remove,
+	},
+	.id_table = pci_id_ixgbevf_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+	.func_drv = &rte_ixgbevf_eth_drv.driver,
+};
+
 static int
 ixgbe_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 {
@@ -7695,9 +7695,9 @@ ixgbevf_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 	ixgbevf_dev_interrupt_action(dev);
 }
 
-RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic | vfio");
-RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pmd.pci_drv);
+RTE_PMD_REGISTER_PCI(net_ixgbe_vf, rte_ixgbevf_pci_drv);
 RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe_vf, pci_id_ixgbevf_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe_vf, "* igb_uio | vfio");
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 2d5a399..ea2f598 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -220,7 +220,7 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
 	dev->driver = dr;
 
 	/* call the driver probe() function */
-	ret = dr->probe(dr, dev);
+	ret = dr->driver.probe(&dr->driver, &dev->device);
 	if (ret) {
 		RTE_LOG(DEBUG, EAL, "Driver (%s) probe failed.\n",
 			dr->driver.name);
@@ -252,7 +252,7 @@ rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
 	RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
 			dev->id.device_id, dr->driver.name);
 
-	if (dr->remove && (dr->remove(dev) < 0))
+	if (dr->driver.remove && (dr->driver.remove(&dev->device) < 0))
 		return -1;	/* negative value is an error */
 
 	/* clear driver structure */
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 1647672..949ed3e 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -200,8 +200,7 @@ typedef int (pci_remove_t)(struct rte_pci_device *);
 struct rte_pci_driver {
 	TAILQ_ENTRY(rte_pci_driver) next;       /**< Next in list. */
 	struct rte_driver driver;               /**< Inherit core driver. */
-	pci_probe_t *probe;                     /**< Device Probe function. */
-	pci_remove_t *remove;                   /**< Device Remove function. */
+	struct rte_driver *func_drv;
 	const struct rte_pci_id *id_table;	/**< ID table, NULL terminated. */
 	uint32_t drv_flags;                     /**< Flags contolling handling of device. */
 };
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 917557a..3369864 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -236,16 +236,23 @@ rte_eth_dev_release_port(struct rte_eth_dev *eth_dev)
 }
 
 int
-rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
-		      struct rte_pci_device *pci_dev)
+rte_eth_dev_pci_probe(struct rte_driver *drv,
+		      struct rte_device *dev)
 {
-	struct eth_driver    *eth_drv;
+	struct rte_pci_driver *pci_drv;
+	struct rte_pci_device *pci_dev;
+	struct eth_driver *eth_drv;
 	struct rte_eth_dev *eth_dev;
 	char ethdev_name[RTE_ETH_NAME_MAX_LEN];
+	struct rte_driver *func_drv;
 
 	int diag;
 
-	eth_drv = (struct eth_driver *)pci_drv;
+	pci_drv = container_of(drv, struct rte_pci_driver, driver);
+	pci_dev = container_of(dev, struct rte_pci_device, device);
+
+	func_drv = pci_drv->func_drv;
+	eth_drv = container_of(func_drv, struct eth_driver, driver);
 
 	rte_eal_pci_device_name(&pci_dev->addr, ethdev_name,
 			sizeof(ethdev_name));
@@ -281,13 +288,19 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
 }
 
 int
-rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
+rte_eth_dev_pci_remove(struct rte_device *dev)
 {
+	struct rte_pci_device *pci_dev;
+	struct rte_pci_driver *pci_drv;
 	const struct eth_driver *eth_drv;
 	struct rte_eth_dev *eth_dev;
 	char ethdev_name[RTE_ETH_NAME_MAX_LEN];
+	struct rte_driver *func_drv;
 	int ret;
 
+	pci_dev = container_of(dev, struct rte_pci_device, device);
+	pci_drv = pci_dev->driver;
+
 	if (pci_dev == NULL)
 		return -EINVAL;
 
@@ -298,7 +311,8 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
 	if (eth_dev == NULL)
 		return -ENODEV;
 
-	eth_drv = (const struct eth_driver *)pci_dev->driver;
+	func_drv = pci_drv->func_drv;
+	eth_drv = container_of(func_drv, struct eth_driver, driver);
 
 	/* Invoke PMD device uninit function */
 	if (*eth_drv->eth_dev_uninit) {
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index ded43d7..203210d 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1862,6 +1862,7 @@ struct eth_driver {
 	eth_dev_init_t eth_dev_init;      /**< Device init function. */
 	eth_dev_uninit_t eth_dev_uninit;  /**< Device uninit function. */
 	unsigned int dev_private_size;    /**< Size of device private data. */
+	struct rte_driver driver;
 };
 
 /**
@@ -4382,15 +4383,15 @@ rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
  * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
  * interface.
  */
-int rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv,
-			  struct rte_pci_device *pci_dev);
+int rte_eth_dev_pci_probe(struct rte_driver *drv,
+			  struct rte_device *dev);
 
 /**
  * @internal
  * Wrapper for use by pci drivers as a .remove function to detach a ethdev
  * interface.
  */
-int rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev);
+int rte_eth_dev_pci_remove(struct rte_device *dev);
 
 #ifdef __cplusplus
 }
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH 8/8] eal: VMBUS infrastructure
From: Stephen Hemminger @ 2017-01-10 18:05 UTC (permalink / raw)
  To: Jan Blunck; +Cc: dev, Stephen Hemminger
In-Reply-To: <CALe+Z000WmS4WeL315otctGkYayPqspEB20o+RTxzgA8ieS8_g@mail.gmail.com>

On Tue, 10 Jan 2017 18:27:31 +0100
Jan Blunck <jblunck@infradead.org> wrote:

> > +#ifdef RTE_LIBRTE_HV_PMD
> > +/**
> > + * @internal
> > + * Wrapper for use by vmbus drivers as a .probe function to attach to a ethdev
> > + * interface.
> > + */
> > +int rte_eth_dev_vmbus_probe(struct rte_vmbus_driver *vmbus_drv,
> > +                         struct rte_vmbus_device *vmbus_dev);
> > +
> > +/**
> > + * @internal
> > + * Wrapper for use by vmbus drivers as a .remove function to detach a ethdev
> > + * interface.
> > + */
> > +int rte_eth_dev_vmbus_remove(struct rte_vmbus_device *vmbus_dev);
> > +#endif  
> 
> I don't think that replicating the PCI probe/remove wrappers is the
> right thing to do. To me it looks like this should move into the
> rte_vmbus_driver's probe function instead. That way the ethdev header
> can decoupled from the low-level device implementations.

With a real bus model. There would be registration of busses. And the probe would
be:
   foreach bus
       foreach device on bus
...

^ permalink raw reply

* [PATCH v1] ethdev: fix multi-process NULL dereference crashes
From: Remy Horton @ 2017-01-10 18:42 UTC (permalink / raw)
  To: dev, Thomas Monjalon

Even though only primary processes should setup PMDs, secondary
processes were also blanket zeroing ethernet device memory. The
result was NULL dereference crashes in multi-process setups.

Fixes: 7f95f78a8aea ("ethdev: clear data when allocating device")

Signed-off-by: Remy Horton <remy.horton@intel.com>
---
 doc/guides/rel_notes/release_17_02.rst | 6 ++++++
 lib/librte_ether/rte_ethdev.c          | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 180af82..20a4ced 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -72,6 +72,12 @@ Resolved Issues
 EAL
 ~~~
 
+* **ethdev: Fixed crash with multi-processing.**
+
+  Even though only primary processes should setup PMDs, secondary
+  processes were also blanket zeroing ethernet device memory. The
+  result was NULL dereference crashes in multi-process setups.
+
 
 Drivers
 ~~~~~~~
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 9dea1f1..a681982 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -212,7 +212,8 @@ rte_eth_dev_allocate(const char *name)
 
 	eth_dev = &rte_eth_devices[port_id];
 	eth_dev->data = &rte_eth_dev_data[port_id];
-	memset(eth_dev->data, 0, sizeof(*eth_dev->data));
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		memset(eth_dev->data, 0, sizeof(*eth_dev->data));
 	snprintf(eth_dev->data->name, sizeof(eth_dev->data->name), "%s", name);
 	eth_dev->data->port_id = port_id;
 	eth_dev->data->mtu = ETHER_MTU;
-- 
2.5.5

^ permalink raw reply related

* Re: [PATCH v8 18/25] app/testpmd: use VFD APIs on i40e
From: Ferruh Yigit @ 2017-01-10 20:08 UTC (permalink / raw)
  To: Wenzhuo Lu, dev; +Cc: Chen Jing D(Mark), Bernard Iremonger
In-Reply-To: <8dfea289-72da-8a84-4d30-368d6d4ea4d6@intel.com>

On 1/10/2017 11:29 AM, Ferruh Yigit wrote:
> On 1/10/2017 7:16 AM, Wenzhuo Lu wrote:
>> The new VF Daemon (VFD) APIs is implemented on i40e. Change
>> testpmd code to use them, including VF MAC anti-spoofing,
>> VF VLAN anti-spoofing, TX loopback, VF VLAN strip, VF VLAN
>> insert.
>>
>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
>> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
>> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
>> ---
> 
> <...>
> 
>> +#ifdef RTE_LIBRTE_IXGBE_PMD
> 
> Within this ifdef, there is following call:
> 
> ret = rte_pmd_ixgbe_set_all_queues_drop_en(res->port_id, is_on);
> 
> It is not safe to make that call directly, port should be verified first
> if it is ixgbe port, as done in other functions.

With a second thought, although this part is not correct, it is out of
this patches scope, because it concerns ixgbe related part. So below
comment also become invalid.

Wenzhuo,

What do you think not changing this patch, but fixing this in a separate
patch?

Thanks,
ferruh

> 
>>  /* all queues drop enable configuration */
>>  
>>  /* Common result structure for all queues drop enable */
>> @@ -11277,6 +11351,9 @@ struct cmd_all_queues_drop_en_result {
>>  	case -ENODEV:
>>  		printf("invalid port_id %d\n", res->port_id);
>>  		break;
>> +	case -ENOTSUP:
>> +		printf("function not implemented\n");
>> +		break;
>>  	default:
>>  		printf("programming error: (%s)\n", strerror(-ret));
>>  	}
>> @@ -11381,6 +11458,7 @@ struct cmd_vf_split_drop_en_result {
>>  		NULL,
>>  	},
>>  };
>> +#endif
>>  
> 
> <...>
> 
>> @@ -11619,20 +11711,20 @@ struct cmd_set_vf_mac_addr_result {
>>  	(cmdline_parse_inst_t *)&cmd_config_e_tag_forwarding_en_dis,
>>  	(cmdline_parse_inst_t *)&cmd_config_e_tag_filter_add,
>>  	(cmdline_parse_inst_t *)&cmd_config_e_tag_filter_del,
>> -#ifdef RTE_LIBRTE_IXGBE_PMD
>>  	(cmdline_parse_inst_t *)&cmd_set_vf_vlan_anti_spoof,
>>  	(cmdline_parse_inst_t *)&cmd_set_vf_mac_anti_spoof,
>>  	(cmdline_parse_inst_t *)&cmd_set_vf_vlan_stripq,
>>  	(cmdline_parse_inst_t *)&cmd_set_vf_vlan_insert,
>>  	(cmdline_parse_inst_t *)&cmd_set_tx_loopback,
>> +#ifdef RTE_LIBRTE_IXGBE_PMD
> 
> Overall, since port will be verified within the function, why not remove
> this ifdef completely?
> 
> So all functions will be visible to everyone, but only the PMDs support
> it will be called, others will return "not supported", as done in some
> functions supports bot i40e and ixgbe pmd specific APIs.
> 
>>  	(cmdline_parse_inst_t *)&cmd_set_all_queues_drop_en,
>>  	(cmdline_parse_inst_t *)&cmd_set_vf_split_drop_en,
>> -	(cmdline_parse_inst_t *)&cmd_set_vf_mac_addr,
>>  	(cmdline_parse_inst_t *)&cmd_set_vf_rxmode,
>>  	(cmdline_parse_inst_t *)&cmd_set_vf_traffic,
>>  	(cmdline_parse_inst_t *)&cmd_vf_rxvlan_filter,
>>  	(cmdline_parse_inst_t *)&cmd_vf_rate_limit,
>>  #endif
>> +	(cmdline_parse_inst_t *)&cmd_set_vf_mac_addr,
>>  	NULL,
>>  };
>>  
>>
> 

^ permalink raw reply

* Re: [PATCH v8 00/25] Support VFD on i40e
From: Vincent Jardin @ 2017-01-10 20:23 UTC (permalink / raw)
  To: Zhang, Helin, Lu, Wenzhuo, dev
  Cc: Lu, Wenzhuo, 'JOSHI, KAUSTUBH', 'DANIELS, EDWARD',
	'ZELEZNIAK, ALEX'
In-Reply-To: <F35DEAC7BCE34641BA9FAC6BCA4A12E71A98F145@SHSMSX103.ccr.corp.intel.com>

Nope. First one needs to assess if DPDK should be intensively used to 
become a PF knowing Linux can do the jobs. Linux kernel community does not 
like the forking of Kernel drivers, I tend to agree that we should not keep 
duplicating options that can be solved with the Linux kernel.

Best regards,
  Vincent

^ permalink raw reply

* Re: [PATCH v8 00/25] Support VFD on i40e
From: Ferruh Yigit @ 2017-01-10 21:32 UTC (permalink / raw)
  To: Vincent Jardin, Zhang, Helin, Lu, Wenzhuo, dev
  Cc: 'JOSHI, KAUSTUBH', 'DANIELS, EDWARD',
	'ZELEZNIAK, ALEX'
In-Reply-To: <1598a0c7f80.27fc.bb328046f2889bc8f44aafa891a44dd2@6wind.com>

Hi Vincent,

On 1/10/2017 8:23 PM, Vincent Jardin wrote:
> Nope. First one needs to assess if DPDK should be intensively used to 
> become a PF knowing Linux can do the jobs. Linux kernel community does not 
> like the forking of Kernel drivers, I tend to agree that we should not keep 
> duplicating options that can be solved with the Linux kernel.

This patchset split into two, mainly because of your comment, and this
one has nothing specific to Linux VF.

Is there any specific piece of this patchset that cause concern?

What do you think to continue high level DPDK PF discussion in mail
thread for other pathset? So that we can continue to work on this one.

Thanks,
ferruh

> 
> Best regards,
>   Vincent
> 
> 

^ permalink raw reply

* Re: i40e_aq_get_phy_capabilities() fails when using SFP+ with no link
From: Zhang, Helin @ 2017-01-11  1:08 UTC (permalink / raw)
  To: Olivier MATZ, Christos Ricudis, Rowden, Aaron F
  Cc: dev@dpdk.org, Wu, Jingjing
In-Reply-To: <20170110162849.2256dc6e@glumotte.dev.6wind.com>

Hi Aaron

Is the SFP+ (Finisar FTLX8571D3BCL) supported and validated by Intel?
It seems there is some PHY issue in this case.

Regards,
Helin

-----Original Message-----
From: Olivier MATZ [mailto:olivier.matz@6wind.com] 
Sent: Tuesday, January 10, 2017 11:29 PM
To: Christos Ricudis <ricudis.christos@gmail.com>
Cc: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
Subject: Re: [dpdk-dev] i40e_aq_get_phy_capabilities() fails when using SFP+ with no link

Hi Christos,

+CC i40e maintainers

On Tue, 10 Jan 2017 20:32:26 +0800, Christos Ricudis <ricudis.christos@gmail.com> wrote:
> Hello,
> 
> Using a X710 based 4-port 4x10Gbit NIC, I have came across the 
> following issue on the i40e PMD:
> 
> When an optical SFP+ (Finisar FTLX8571D3BCL) is used with no active 
> link partner on the other end of the link (or fiber completely 
> disconnected from the SFP+), i40e_aq_get_phy_capabilities() (called by 
> i40e_dev_sync_phy_type() on port initialization), fails with a
> 0x05 return value (EIO) on the AQ response structure. The struct 
> i40e_aq_get_phy_abilities_resp buffer passed to the Get Phy Abilities 
> command is unmodified upon return.
> 
> This prevents DPDK 16.11 from initializing the port, and ultimately 
> fails with the following error:
> 
> PMD: eth_i40e_dev_init(): Failed to sync phy type: -95
> 
> The change introducing this issue was
> http://dpdk.org/ml/archives/dev/2016-September/047663.html
> 
> Reading the X710 datasheet, I notice that no specific mention is given 
> on the meaning of EIO as a response to Get PHY Abilities command 
> (opcode 0x0600), whereas in most other commands, an explicit mention 
> of the meaning of the possible error status responses is given.
> 
> This behaviour is the same across the following NVM releases: 
> 
> FW 4.33 API 1.2 NVM 04.04.02 eetrack 800018a6 FW 4.40 API 1.4 NVM 
> 04.05.03 eetrack 80001cd8 FW 5.0 API 1.5 NVM 05.00.04 eetrack 800024da
> 
> I will try to get around the issue by falling back to PHY capabilities 
> detection using the device ID in the case
> i40e_aq_get_phy_capabilities() fails, but conceptually the 
> capabilities of the PHY should not be dependent on whether PHY detects 
> an active link or not.
> 
> I’d be happy to do more testing on this issue per your 
> recommendations.
> 
> Moreover, while trying to debug this issue, I managed to get both 3 
> NIC adapters on my test system on a state where the PHY has apparently 
> died - no link indication at all on any ports. A reboot solved this, 
> and I am now trying to replicate this behaviour under more controlled 
> conditions.
> 

I'm currently running into a similar issue (I think). I can reproduce it with testpmd with the following case:

  set link_check off
  port stop 0
  # don't wait between these 2 commands
  port start 0

I added some logs that are displayed after the port start:

  PMD: i40e_set_tx_function(): Vector tx finally be used.
  PMD: i40e_set_rx_function(): Vector rx enabled, please make sure RX
    burst size no less than 4 (port=0).
  PMD: i40e_dev_rx_queue_start():  >>
  PMD: i40e_dev_tx_queue_start():  >>
  PMD: i40e_dev_start(): applying link settings...
  PMD: i40e_apply_link_speed(): abilities = 38, speed = 0
  PMD: i40e_phy_conf_link(): i40e_aq_get_phy_capabilities failed -7
  PMD: i40e_dev_start(): Fail to apply link setting
  PMD: i40e_dev_clear_queues():  >>

The -7 corresponds to I40E_ERR_UNKNOWN_PHY. This happens in
i40e_aq_get_phy_capabilities() in the following code, which makes me think it's the same problem than yours:

	if (hw->aq.asq_last_status == I40E_AQ_RC_EIO)
		status = I40E_ERR_UNKNOWN_PHY;

A workaround in my usecase is to wait a bit between the stop and the start.

Any help is welcome.

Regards,
Olivier



^ permalink raw reply

* Re: [PATCH v8 18/25] app/testpmd: use VFD APIs on i40e
From: Lu, Wenzhuo @ 2017-01-11  1:27 UTC (permalink / raw)
  To: Yigit, Ferruh, dev@dpdk.org; +Cc: Chen, Jing D, Iremonger, Bernard
In-Reply-To: <598bd9db-073a-b40c-b60e-1d755b8f95ce@intel.com>

Hi Ferruh,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 4:09 AM
> To: Lu, Wenzhuo; dev@dpdk.org
> Cc: Chen, Jing D; Iremonger, Bernard
> Subject: Re: [dpdk-dev] [PATCH v8 18/25] app/testpmd: use VFD APIs on i40e
> 
> On 1/10/2017 11:29 AM, Ferruh Yigit wrote:
> > On 1/10/2017 7:16 AM, Wenzhuo Lu wrote:
> >> The new VF Daemon (VFD) APIs is implemented on i40e. Change testpmd
> >> code to use them, including VF MAC anti-spoofing, VF VLAN
> >> anti-spoofing, TX loopback, VF VLAN strip, VF VLAN insert.
> >>
> >> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> >> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> >> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> >> ---
> >
> > <...>
> >
> >> +#ifdef RTE_LIBRTE_IXGBE_PMD
> >
> > Within this ifdef, there is following call:
> >
> > ret = rte_pmd_ixgbe_set_all_queues_drop_en(res->port_id, is_on);
> >
> > It is not safe to make that call directly, port should be verified
> > first if it is ixgbe port, as done in other functions.
> 
> With a second thought, although this part is not correct, it is out of this
> patches scope, because it concerns ixgbe related part. So below comment also
> become invalid.
> 
> Wenzhuo,
> 
> What do you think not changing this patch, but fixing this in a separate patch?
Agree. I'll not change this patch but fix this issue with a separate patch.

> 
> Thanks,
> ferruh

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox