* [PATCH v2 08/31] net/i40e/base: add clause22 and clause45 implementation
From: Jingjing Wu @ 2016-12-09 14:39 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1481294364-83505-1-git-send-email-jingjing.wu@intel.com>
Some external PHYs require Clause22 and Clause45 method for
accessing registers. Mostly used for X722 support.
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/base/i40e_common.c | 245 +++++++++++++++++++++++++++------
drivers/net/i40e/base/i40e_prototype.h | 16 ++-
drivers/net/i40e/base/i40e_type.h | 17 ++-
3 files changed, 226 insertions(+), 52 deletions(-)
diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
index 7eea189..85c1c11 100644
--- a/drivers/net/i40e/base/i40e_common.c
+++ b/drivers/net/i40e/base/i40e_common.c
@@ -6022,7 +6022,92 @@ enum i40e_status_code i40e_aq_configure_partition_bw(struct i40e_hw *hw,
}
/**
- * i40e_read_phy_register
+ * i40e_read_phy_register_clause22
+ * @hw: pointer to the HW structure
+ * @reg: register address in the page
+ * @phy_adr: PHY address on MDIO interface
+ * @value: PHY register value
+ *
+ * Reads specified PHY register value
+ **/
+enum i40e_status_code i40e_read_phy_register_clause22(struct i40e_hw *hw,
+ u16 reg, u8 phy_addr, u16 *value)
+{
+ enum i40e_status_code status = I40E_ERR_TIMEOUT;
+ u8 port_num = (u8)hw->func_caps.mdio_port_num;
+ u32 command = 0;
+ u16 retry = 1000;
+
+ command = (reg << I40E_GLGEN_MSCA_DEVADD_SHIFT) |
+ (phy_addr << I40E_GLGEN_MSCA_PHYADD_SHIFT) |
+ (I40E_MDIO_CLAUSE22_OPCODE_READ_MASK) |
+ (I40E_MDIO_CLAUSE22_STCODE_MASK) |
+ (I40E_GLGEN_MSCA_MDICMD_MASK);
+ wr32(hw, I40E_GLGEN_MSCA(port_num), command);
+ do {
+ command = rd32(hw, I40E_GLGEN_MSCA(port_num));
+ if (!(command & I40E_GLGEN_MSCA_MDICMD_MASK)) {
+ status = I40E_SUCCESS;
+ break;
+ }
+ i40e_usec_delay(10);
+ retry--;
+ } while (retry);
+
+ if (status) {
+ i40e_debug(hw, I40E_DEBUG_PHY,
+ "PHY: Can't write command to external PHY.\n");
+ } else {
+ command = rd32(hw, I40E_GLGEN_MSRWD(port_num));
+ *value = (command & I40E_GLGEN_MSRWD_MDIRDDATA_MASK) >>
+ I40E_GLGEN_MSRWD_MDIRDDATA_SHIFT;
+ }
+
+ return status;
+}
+
+/**
+ * i40e_write_phy_register_clause22
+ * @hw: pointer to the HW structure
+ * @reg: register address in the page
+ * @phy_adr: PHY address on MDIO interface
+ * @value: PHY register value
+ *
+ * Writes specified PHY register value
+ **/
+enum i40e_status_code i40e_write_phy_register_clause22(struct i40e_hw *hw,
+ u16 reg, u8 phy_addr, u16 value)
+{
+ enum i40e_status_code status = I40E_ERR_TIMEOUT;
+ u8 port_num = (u8)hw->func_caps.mdio_port_num;
+ u32 command = 0;
+ u16 retry = 1000;
+
+ command = value << I40E_GLGEN_MSRWD_MDIWRDATA_SHIFT;
+ wr32(hw, I40E_GLGEN_MSRWD(port_num), command);
+
+ command = (reg << I40E_GLGEN_MSCA_DEVADD_SHIFT) |
+ (phy_addr << I40E_GLGEN_MSCA_PHYADD_SHIFT) |
+ (I40E_MDIO_CLAUSE22_OPCODE_WRITE_MASK) |
+ (I40E_MDIO_CLAUSE22_STCODE_MASK) |
+ (I40E_GLGEN_MSCA_MDICMD_MASK);
+
+ wr32(hw, I40E_GLGEN_MSCA(port_num), command);
+ do {
+ command = rd32(hw, I40E_GLGEN_MSCA(port_num));
+ if (!(command & I40E_GLGEN_MSCA_MDICMD_MASK)) {
+ status = I40E_SUCCESS;
+ break;
+ }
+ i40e_usec_delay(10);
+ retry--;
+ } while (retry);
+
+ return status;
+}
+
+/**
+ * i40e_read_phy_register_clause45
* @hw: pointer to the HW structure
* @page: registers page number
* @reg: register address in the page
@@ -6031,9 +6116,8 @@ enum i40e_status_code i40e_aq_configure_partition_bw(struct i40e_hw *hw,
*
* Reads specified PHY register value
**/
-enum i40e_status_code i40e_read_phy_register(struct i40e_hw *hw,
- u8 page, u16 reg, u8 phy_addr,
- u16 *value)
+enum i40e_status_code i40e_read_phy_register_clause45(struct i40e_hw *hw,
+ u8 page, u16 reg, u8 phy_addr, u16 *value)
{
enum i40e_status_code status = I40E_ERR_TIMEOUT;
u32 command = 0;
@@ -6043,8 +6127,8 @@ enum i40e_status_code i40e_read_phy_register(struct i40e_hw *hw,
command = (reg << I40E_GLGEN_MSCA_MDIADD_SHIFT) |
(page << I40E_GLGEN_MSCA_DEVADD_SHIFT) |
(phy_addr << I40E_GLGEN_MSCA_PHYADD_SHIFT) |
- (I40E_MDIO_OPCODE_ADDRESS) |
- (I40E_MDIO_STCODE) |
+ (I40E_MDIO_CLAUSE45_OPCODE_ADDRESS_MASK) |
+ (I40E_MDIO_CLAUSE45_STCODE_MASK) |
(I40E_GLGEN_MSCA_MDICMD_MASK) |
(I40E_GLGEN_MSCA_MDIINPROGEN_MASK);
wr32(hw, I40E_GLGEN_MSCA(port_num), command);
@@ -6066,8 +6150,8 @@ enum i40e_status_code i40e_read_phy_register(struct i40e_hw *hw,
command = (page << I40E_GLGEN_MSCA_DEVADD_SHIFT) |
(phy_addr << I40E_GLGEN_MSCA_PHYADD_SHIFT) |
- (I40E_MDIO_OPCODE_READ) |
- (I40E_MDIO_STCODE) |
+ (I40E_MDIO_CLAUSE45_OPCODE_READ_MASK) |
+ (I40E_MDIO_CLAUSE45_STCODE_MASK) |
(I40E_GLGEN_MSCA_MDICMD_MASK) |
(I40E_GLGEN_MSCA_MDIINPROGEN_MASK);
status = I40E_ERR_TIMEOUT;
@@ -6097,7 +6181,7 @@ enum i40e_status_code i40e_read_phy_register(struct i40e_hw *hw,
}
/**
- * i40e_write_phy_register
+ * i40e_write_phy_register_clause45
* @hw: pointer to the HW structure
* @page: registers page number
* @reg: register address in the page
@@ -6106,9 +6190,8 @@ enum i40e_status_code i40e_read_phy_register(struct i40e_hw *hw,
*
* Writes value to specified PHY register
**/
-enum i40e_status_code i40e_write_phy_register(struct i40e_hw *hw,
- u8 page, u16 reg, u8 phy_addr,
- u16 value)
+enum i40e_status_code i40e_write_phy_register_clause45(struct i40e_hw *hw,
+ u8 page, u16 reg, u8 phy_addr, u16 value)
{
enum i40e_status_code status = I40E_ERR_TIMEOUT;
u32 command = 0;
@@ -6118,8 +6201,8 @@ enum i40e_status_code i40e_write_phy_register(struct i40e_hw *hw,
command = (reg << I40E_GLGEN_MSCA_MDIADD_SHIFT) |
(page << I40E_GLGEN_MSCA_DEVADD_SHIFT) |
(phy_addr << I40E_GLGEN_MSCA_PHYADD_SHIFT) |
- (I40E_MDIO_OPCODE_ADDRESS) |
- (I40E_MDIO_STCODE) |
+ (I40E_MDIO_CLAUSE45_OPCODE_ADDRESS_MASK) |
+ (I40E_MDIO_CLAUSE45_STCODE_MASK) |
(I40E_GLGEN_MSCA_MDICMD_MASK) |
(I40E_GLGEN_MSCA_MDIINPROGEN_MASK);
wr32(hw, I40E_GLGEN_MSCA(port_num), command);
@@ -6143,8 +6226,8 @@ enum i40e_status_code i40e_write_phy_register(struct i40e_hw *hw,
command = (page << I40E_GLGEN_MSCA_DEVADD_SHIFT) |
(phy_addr << I40E_GLGEN_MSCA_PHYADD_SHIFT) |
- (I40E_MDIO_OPCODE_WRITE) |
- (I40E_MDIO_STCODE) |
+ (I40E_MDIO_CLAUSE45_OPCODE_WRITE_MASK) |
+ (I40E_MDIO_CLAUSE45_STCODE_MASK) |
(I40E_GLGEN_MSCA_MDICMD_MASK) |
(I40E_GLGEN_MSCA_MDIINPROGEN_MASK);
status = I40E_ERR_TIMEOUT;
@@ -6165,6 +6248,78 @@ enum i40e_status_code i40e_write_phy_register(struct i40e_hw *hw,
}
/**
+ * i40e_write_phy_register
+ * @hw: pointer to the HW structure
+ * @page: registers page number
+ * @reg: register address in the page
+ * @phy_adr: PHY address on MDIO interface
+ * @value: PHY register value
+ *
+ * Writes value to specified PHY register
+ **/
+enum i40e_status_code i40e_write_phy_register(struct i40e_hw *hw,
+ u8 page, u16 reg, u8 phy_addr, u16 value)
+{
+ enum i40e_status_code status;
+
+ switch (hw->device_id) {
+ case I40E_DEV_ID_1G_BASE_T_X722:
+ status = i40e_write_phy_register_clause22(hw,
+ reg, phy_addr, value);
+ break;
+ case I40E_DEV_ID_10G_BASE_T:
+ case I40E_DEV_ID_10G_BASE_T4:
+ case I40E_DEV_ID_10G_BASE_T_X722:
+ case I40E_DEV_ID_25G_B:
+ case I40E_DEV_ID_25G_SFP28:
+ status = i40e_write_phy_register_clause45(hw,
+ page, reg, phy_addr, value);
+ break;
+ default:
+ status = I40E_ERR_UNKNOWN_PHY;
+ break;
+ }
+
+ return status;
+}
+
+/**
+ * i40e_read_phy_register
+ * @hw: pointer to the HW structure
+ * @page: registers page number
+ * @reg: register address in the page
+ * @phy_adr: PHY address on MDIO interface
+ * @value: PHY register value
+ *
+ * Reads specified PHY register value
+ **/
+enum i40e_status_code i40e_read_phy_register(struct i40e_hw *hw,
+ u8 page, u16 reg, u8 phy_addr, u16 *value)
+{
+ enum i40e_status_code status;
+
+ switch (hw->device_id) {
+ case I40E_DEV_ID_1G_BASE_T_X722:
+ status = i40e_read_phy_register_clause22(hw, reg, phy_addr,
+ value);
+ break;
+ case I40E_DEV_ID_10G_BASE_T:
+ case I40E_DEV_ID_10G_BASE_T4:
+ case I40E_DEV_ID_10G_BASE_T_X722:
+ case I40E_DEV_ID_25G_B:
+ case I40E_DEV_ID_25G_SFP28:
+ status = i40e_read_phy_register_clause45(hw, page, reg,
+ phy_addr, value);
+ break;
+ default:
+ status = I40E_ERR_UNKNOWN_PHY;
+ break;
+ }
+
+ return status;
+}
+
+/**
* i40e_get_phy_address
* @hw: pointer to the HW structure
* @dev_num: PHY port num that address we want
@@ -6206,14 +6361,16 @@ enum i40e_status_code i40e_blink_phy_link_led(struct i40e_hw *hw,
for (gpio_led_port = 0; gpio_led_port < 3; gpio_led_port++,
led_addr++) {
- status = i40e_read_phy_register(hw, I40E_PHY_COM_REG_PAGE,
- led_addr, phy_addr, &led_reg);
+ status = i40e_read_phy_register_clause45(hw,
+ I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr,
+ &led_reg);
if (status)
goto phy_blinking_end;
led_ctl = led_reg;
if (led_reg & I40E_PHY_LED_LINK_MODE_MASK) {
led_reg = 0;
- status = i40e_write_phy_register(hw,
+ status = i40e_write_phy_register_clause45(hw,
I40E_PHY_COM_REG_PAGE,
led_addr, phy_addr,
led_reg);
@@ -6225,20 +6382,18 @@ enum i40e_status_code i40e_blink_phy_link_led(struct i40e_hw *hw,
if (time > 0 && interval > 0) {
for (i = 0; i < time * 1000; i += interval) {
- status = i40e_read_phy_register(hw,
- I40E_PHY_COM_REG_PAGE,
- led_addr, phy_addr,
- &led_reg);
+ status = i40e_read_phy_register_clause45(hw,
+ I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr, &led_reg);
if (status)
goto restore_config;
if (led_reg & I40E_PHY_LED_MANUAL_ON)
led_reg = 0;
else
led_reg = I40E_PHY_LED_MANUAL_ON;
- status = i40e_write_phy_register(hw,
- I40E_PHY_COM_REG_PAGE,
- led_addr, phy_addr,
- led_reg);
+ status = i40e_write_phy_register_clause45(hw,
+ I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr, led_reg);
if (status)
goto restore_config;
i40e_msec_delay(interval);
@@ -6246,8 +6401,9 @@ enum i40e_status_code i40e_blink_phy_link_led(struct i40e_hw *hw,
}
restore_config:
- status = i40e_write_phy_register(hw, I40E_PHY_COM_REG_PAGE, led_addr,
- phy_addr, led_ctl);
+ status = i40e_write_phy_register_clause45(hw,
+ I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr, led_ctl);
phy_blinking_end:
return status;
@@ -6278,8 +6434,10 @@ enum i40e_status_code i40e_led_get_phy(struct i40e_hw *hw, u16 *led_addr,
for (gpio_led_port = 0; gpio_led_port < 3; gpio_led_port++,
temp_addr++) {
- status = i40e_read_phy_register(hw, I40E_PHY_COM_REG_PAGE,
- temp_addr, phy_addr, ®_val);
+ status = i40e_read_phy_register_clause45(hw,
+ I40E_PHY_COM_REG_PAGE,
+ temp_addr, phy_addr,
+ ®_val);
if (status)
return status;
*val = reg_val;
@@ -6312,41 +6470,42 @@ enum i40e_status_code i40e_led_set_phy(struct i40e_hw *hw, bool on,
i = rd32(hw, I40E_PFGEN_PORTNUM);
port_num = (u8)(i & I40E_PFGEN_PORTNUM_PORT_NUM_MASK);
phy_addr = i40e_get_phy_address(hw, port_num);
-
- status = i40e_read_phy_register(hw, I40E_PHY_COM_REG_PAGE, led_addr,
- phy_addr, &led_reg);
+ status = i40e_read_phy_register_clause45(hw, I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr, &led_reg);
if (status)
return status;
led_ctl = led_reg;
if (led_reg & I40E_PHY_LED_LINK_MODE_MASK) {
led_reg = 0;
- status = i40e_write_phy_register(hw, I40E_PHY_COM_REG_PAGE,
- led_addr, phy_addr, led_reg);
+ status = i40e_write_phy_register_clause45(hw,
+ I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr,
+ led_reg);
if (status)
return status;
}
- status = i40e_read_phy_register(hw, I40E_PHY_COM_REG_PAGE,
- led_addr, phy_addr, &led_reg);
+ status = i40e_read_phy_register_clause45(hw, I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr, &led_reg);
if (status)
goto restore_config;
if (on)
led_reg = I40E_PHY_LED_MANUAL_ON;
else
led_reg = 0;
- status = i40e_write_phy_register(hw, I40E_PHY_COM_REG_PAGE,
- led_addr, phy_addr, led_reg);
+ status = i40e_write_phy_register_clause45(hw, I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr, led_reg);
if (status)
goto restore_config;
if (mode & I40E_PHY_LED_MODE_ORIG) {
led_ctl = (mode & I40E_PHY_LED_MODE_MASK);
- status = i40e_write_phy_register(hw,
+ status = i40e_write_phy_register_clause45(hw,
I40E_PHY_COM_REG_PAGE,
led_addr, phy_addr, led_ctl);
}
return status;
restore_config:
- status = i40e_write_phy_register(hw, I40E_PHY_COM_REG_PAGE, led_addr,
- phy_addr, led_ctl);
+ status = i40e_write_phy_register_clause45(hw, I40E_PHY_COM_REG_PAGE,
+ led_addr, phy_addr, led_ctl);
return status;
}
#endif /* PF_DRIVER */
diff --git a/drivers/net/i40e/base/i40e_prototype.h b/drivers/net/i40e/base/i40e_prototype.h
index 3aab5ca..9109cfc 100644
--- a/drivers/net/i40e/base/i40e_prototype.h
+++ b/drivers/net/i40e/base/i40e_prototype.h
@@ -538,10 +538,18 @@ enum i40e_status_code i40e_aq_get_wake_event_reason(struct i40e_hw *hw,
u16 *wake_reason,
struct i40e_asq_cmd_details *cmd_details);
#endif
-enum i40e_status_code i40e_read_phy_register(struct i40e_hw *hw, u8 page,
- u16 reg, u8 phy_addr, u16 *value);
-enum i40e_status_code i40e_write_phy_register(struct i40e_hw *hw, u8 page,
- u16 reg, u8 phy_addr, u16 value);
+enum i40e_status_code i40e_read_phy_register_clause22(struct i40e_hw *hw,
+ u16 reg, u8 phy_addr, u16 *value);
+enum i40e_status_code i40e_write_phy_register_clause22(struct i40e_hw *hw,
+ u16 reg, u8 phy_addr, u16 value);
+enum i40e_status_code i40e_read_phy_register_clause45(struct i40e_hw *hw,
+ u8 page, u16 reg, u8 phy_addr, u16 *value);
+enum i40e_status_code i40e_write_phy_register_clause45(struct i40e_hw *hw,
+ u8 page, u16 reg, u8 phy_addr, u16 value);
+enum i40e_status_code i40e_read_phy_register(struct i40e_hw *hw,
+ u8 page, u16 reg, u8 phy_addr, u16 *value);
+enum i40e_status_code i40e_write_phy_register(struct i40e_hw *hw,
+ u8 page, u16 reg, u8 phy_addr, u16 value);
u8 i40e_get_phy_address(struct i40e_hw *hw, u8 dev_num);
enum i40e_status_code i40e_blink_phy_link_led(struct i40e_hw *hw,
u32 time, u32 interval);
diff --git a/drivers/net/i40e/base/i40e_type.h b/drivers/net/i40e/base/i40e_type.h
index b5f72c3..5a59ce2 100644
--- a/drivers/net/i40e/base/i40e_type.h
+++ b/drivers/net/i40e/base/i40e_type.h
@@ -157,15 +157,22 @@ enum i40e_debug_mask {
#define I40E_PCI_LINK_SPEED_5000 0x2
#define I40E_PCI_LINK_SPEED_8000 0x3
-#define I40E_MDIO_STCODE I40E_MASK(0, \
+#define I40E_MDIO_CLAUSE22_STCODE_MASK I40E_MASK(1, \
I40E_GLGEN_MSCA_STCODE_SHIFT)
-#define I40E_MDIO_OPCODE_ADDRESS I40E_MASK(0, \
+#define I40E_MDIO_CLAUSE22_OPCODE_WRITE_MASK I40E_MASK(1, \
I40E_GLGEN_MSCA_OPCODE_SHIFT)
-#define I40E_MDIO_OPCODE_WRITE I40E_MASK(1, \
+#define I40E_MDIO_CLAUSE22_OPCODE_READ_MASK I40E_MASK(2, \
I40E_GLGEN_MSCA_OPCODE_SHIFT)
-#define I40E_MDIO_OPCODE_READ_INC_ADDR I40E_MASK(2, \
+
+#define I40E_MDIO_CLAUSE45_STCODE_MASK I40E_MASK(0, \
+ I40E_GLGEN_MSCA_STCODE_SHIFT)
+#define I40E_MDIO_CLAUSE45_OPCODE_ADDRESS_MASK I40E_MASK(0, \
+ I40E_GLGEN_MSCA_OPCODE_SHIFT)
+#define I40E_MDIO_CLAUSE45_OPCODE_WRITE_MASK I40E_MASK(1, \
+ I40E_GLGEN_MSCA_OPCODE_SHIFT)
+#define I40E_MDIO_CLAUSE45_OPCODE_READ_INC_ADDR_MASK I40E_MASK(2, \
I40E_GLGEN_MSCA_OPCODE_SHIFT)
-#define I40E_MDIO_OPCODE_READ I40E_MASK(3, \
+#define I40E_MDIO_CLAUSE45_OPCODE_READ_MASK I40E_MASK(3, \
I40E_GLGEN_MSCA_OPCODE_SHIFT)
#define I40E_PHY_COM_REG_PAGE 0x1E
--
2.4.11
^ permalink raw reply related
* [PATCH v2 07/31] net/i40e/base: add media type detection for 25G link
From: Jingjing Wu @ 2016-12-09 14:39 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1481294364-83505-1-git-send-email-jingjing.wu@intel.com>
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/base/i40e_common.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
index 2ad9448..7eea189 100644
--- a/drivers/net/i40e/base/i40e_common.c
+++ b/drivers/net/i40e/base/i40e_common.c
@@ -1288,6 +1288,8 @@ STATIC enum i40e_media_type i40e_get_media_type(struct i40e_hw *hw)
case I40E_PHY_TYPE_1000BASE_LX:
case I40E_PHY_TYPE_40GBASE_SR4:
case I40E_PHY_TYPE_40GBASE_LR4:
+ case I40E_PHY_TYPE_25GBASE_LR:
+ case I40E_PHY_TYPE_25GBASE_SR:
media = I40E_MEDIA_TYPE_FIBER;
break;
case I40E_PHY_TYPE_100BASE_TX:
@@ -1302,6 +1304,7 @@ STATIC enum i40e_media_type i40e_get_media_type(struct i40e_hw *hw)
case I40E_PHY_TYPE_10GBASE_SFPP_CU:
case I40E_PHY_TYPE_40GBASE_AOC:
case I40E_PHY_TYPE_10GBASE_AOC:
+ case I40E_PHY_TYPE_25GBASE_CR:
media = I40E_MEDIA_TYPE_DA;
break;
case I40E_PHY_TYPE_1000BASE_KX:
@@ -1309,6 +1312,7 @@ STATIC enum i40e_media_type i40e_get_media_type(struct i40e_hw *hw)
case I40E_PHY_TYPE_10GBASE_KR:
case I40E_PHY_TYPE_40GBASE_KR4:
case I40E_PHY_TYPE_20GBASE_KR2:
+ case I40E_PHY_TYPE_25GBASE_KR:
media = I40E_MEDIA_TYPE_BACKPLANE;
break;
case I40E_PHY_TYPE_SGMII:
--
2.4.11
^ permalink raw reply related
* [PATCH v2 06/31] net/i40e/base: fix long link down notification time
From: Jingjing Wu @ 2016-12-09 14:38 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1481294364-83505-1-git-send-email-jingjing.wu@intel.com>
This patch fixes a problem where it could take a very
long time (>100 msec) to print the link down notification.
This problem is fixed by changing how often we update link
info from fw, when link is down. Without this patch, it can
take over 100msec to notify user link is down.
Fixes: e6691b428eb1 ("i40e/base: fix PHY NVM interaction")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/base/i40e_common.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
index a2661cf..2ad9448 100644
--- a/drivers/net/i40e/base/i40e_common.c
+++ b/drivers/net/i40e/base/i40e_common.c
@@ -2746,7 +2746,10 @@ enum i40e_status_code i40e_update_link_info(struct i40e_hw *hw)
if (status)
return status;
- if (hw->phy.link_info.link_info & I40E_AQ_MEDIA_AVAILABLE) {
+ /* extra checking needed to ensure link info to user is timely */
+ if ((hw->phy.link_info.link_info & I40E_AQ_MEDIA_AVAILABLE) &&
+ ((hw->phy.link_info.link_info & I40E_AQ_LINK_UP) ||
+ !(hw->phy.link_info_old.link_info & I40E_AQ_LINK_UP))) {
status = i40e_aq_get_phy_capabilities(hw, false, false,
&abilities, NULL);
if (status)
--
2.4.11
^ permalink raw reply related
* [PATCH v2 05/31] net/i40e/base: group base mode VF offload flags
From: Jingjing Wu @ 2016-12-09 14:38 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1481294364-83505-1-git-send-email-jingjing.wu@intel.com>
Group together the minimum set of offload capabilities that are always
supported by VF in base mode. This define would be used by PF to make
sure VF in base mode gets minimum of base capabilities.
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/base/i40e_virtchnl.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/i40e/base/i40e_virtchnl.h b/drivers/net/i40e/base/i40e_virtchnl.h
index 07e7472..8fba608 100644
--- a/drivers/net/i40e/base/i40e_virtchnl.h
+++ b/drivers/net/i40e/base/i40e_virtchnl.h
@@ -172,6 +172,10 @@ struct i40e_virtchnl_vsi_resource {
#define I40E_VIRTCHNL_VF_OFFLOAD_RSS_PF 0X00080000
#define I40E_VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM 0X00100000
+#define I40E_VF_BASE_MODE_OFFLOADS (I40E_VIRTCHNL_VF_OFFLOAD_L2 | \
+ I40E_VIRTCHNL_VF_OFFLOAD_VLAN | \
+ I40E_VIRTCHNL_VF_OFFLOAD_RSS_PF)
+
struct i40e_virtchnl_vf_resource {
u16 num_vsis;
u16 num_queue_pairs;
--
2.4.11
^ permalink raw reply related
* [PATCH v2 04/31] net/i40e/base: fix bit test mask
From: Jingjing Wu @ 2016-12-09 14:38 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1481294364-83505-1-git-send-email-jingjing.wu@intel.com>
Incorrect bit mask was used for testing "get link status" response.
Instead of I40E_AQ_LSE_ENABLE (which is actually 0x03) it should
be I40E_AQ_LSE_IS_ENABLED (which is defined as 0x01).
Fixes: 8db9e2a1b232 ("i40e: base driver")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/base/i40e_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
index aa346d1..a2661cf 100644
--- a/drivers/net/i40e/base/i40e_common.c
+++ b/drivers/net/i40e/base/i40e_common.c
@@ -1975,7 +1975,7 @@ enum i40e_status_code i40e_aq_get_link_info(struct i40e_hw *hw,
else
hw_link_info->crc_enable = false;
- if (resp->command_flags & CPU_TO_LE16(I40E_AQ_LSE_ENABLE))
+ if (resp->command_flags & CPU_TO_LE16(I40E_AQ_LSE_IS_ENABLED))
hw_link_info->lse_enable = true;
else
hw_link_info->lse_enable = false;
--
2.4.11
^ permalink raw reply related
* [PATCH v2 03/31] net/i40e/base: remove unnecessary code
From: Jingjing Wu @ 2016-12-09 14:38 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1481294364-83505-1-git-send-email-jingjing.wu@intel.com>
This patch changes some assignments and removing the unnecessary
code to avoid error reported by static analysis tools.
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/base/i40e_common.c | 4 ----
drivers/net/i40e/base/i40e_lan_hmc.c | 5 -----
2 files changed, 9 deletions(-)
diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
index d67ad90..aa346d1 100644
--- a/drivers/net/i40e/base/i40e_common.c
+++ b/drivers/net/i40e/base/i40e_common.c
@@ -3833,7 +3833,6 @@ STATIC void i40e_parse_discover_capabilities(struct i40e_hw *hw, void *buff,
else
p->acpi_prog_method = I40E_ACPI_PROGRAMMING_METHOD_HW_FVL;
p->proxy_support = (phys_id & I40E_PROXY_SUPPORT_MASK) ? 1 : 0;
- p->proxy_support = p->proxy_support;
i40e_debug(hw, I40E_DEBUG_INIT,
"HW Capability: WOL proxy filters = %d\n",
hw->num_wol_proxy_filters);
@@ -6008,9 +6007,6 @@ enum i40e_status_code i40e_aq_configure_partition_bw(struct i40e_hw *hw,
desc.flags |= CPU_TO_LE16((u16)I40E_AQ_FLAG_BUF);
desc.flags |= CPU_TO_LE16((u16)I40E_AQ_FLAG_RD);
- if (bwd_size > I40E_AQ_LARGE_BUF)
- desc.flags |= CPU_TO_LE16((u16)I40E_AQ_FLAG_LB);
-
desc.datalen = CPU_TO_LE16(bwd_size);
status = i40e_asq_send_command(hw, &desc, bw_data, bwd_size, cmd_details);
diff --git a/drivers/net/i40e/base/i40e_lan_hmc.c b/drivers/net/i40e/base/i40e_lan_hmc.c
index 2260648..f03f381 100644
--- a/drivers/net/i40e/base/i40e_lan_hmc.c
+++ b/drivers/net/i40e/base/i40e_lan_hmc.c
@@ -1239,11 +1239,6 @@ enum i40e_status_code i40e_hmc_get_object_va(struct i40e_hw *hw,
u64 obj_offset_in_fpm;
u32 sd_idx, sd_lmt;
- if (NULL == hmc_info) {
- ret_code = I40E_ERR_BAD_PTR;
- DEBUGOUT("i40e_hmc_get_object_va: bad hmc_info ptr\n");
- goto exit;
- }
if (NULL == hmc_info->hmc_obj) {
ret_code = I40E_ERR_BAD_PTR;
DEBUGOUT("i40e_hmc_get_object_va: bad hmc_info->hmc_obj ptr\n");
--
2.4.11
^ permalink raw reply related
* [PATCH v2 02/31] net/i40e/base: preserve extended PHY type field
From: Jingjing Wu @ 2016-12-09 14:38 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1481294364-83505-1-git-send-email-jingjing.wu@intel.com>
Prevents 25G PHY types from being disabled when setting
the flow control modes.
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/base/i40e_common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/i40e/base/i40e_common.c b/drivers/net/i40e/base/i40e_common.c
index 9a6b3ed..d67ad90 100644
--- a/drivers/net/i40e/base/i40e_common.c
+++ b/drivers/net/i40e/base/i40e_common.c
@@ -1789,6 +1789,7 @@ enum i40e_status_code i40e_set_fc(struct i40e_hw *hw, u8 *aq_failures,
config.abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
/* Copy over all the old settings */
config.phy_type = abilities.phy_type;
+ config.phy_type_ext = abilities.phy_type_ext;
config.link_speed = abilities.link_speed;
config.eee_capability = abilities.eee_capability;
config.eeer = abilities.eeer_val;
--
2.4.11
^ permalink raw reply related
* [PATCH v2 01/31] net/i40e/base: add encap csum VF offload flag
From: Jingjing Wu @ 2016-12-09 14:38 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1481294364-83505-1-git-send-email-jingjing.wu@intel.com>
Add ENCAP_CSUM offload negotiation flag. Currently VF assumes checksum
offload for encapsulated packets is supported by default. Going forward,
this feature needs to be negotiated with PF before advertising to the
stack. Hence, we need a flag to control it.
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
drivers/net/i40e/base/i40e_virtchnl.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/i40e/base/i40e_virtchnl.h b/drivers/net/i40e/base/i40e_virtchnl.h
index fd51ec3..07e7472 100644
--- a/drivers/net/i40e/base/i40e_virtchnl.h
+++ b/drivers/net/i40e/base/i40e_virtchnl.h
@@ -170,6 +170,7 @@ struct i40e_virtchnl_vsi_resource {
#define I40E_VIRTCHNL_VF_OFFLOAD_RX_POLLING 0x00020000
#define I40E_VIRTCHNL_VF_OFFLOAD_RSS_PCTYPE_V2 0x00040000
#define I40E_VIRTCHNL_VF_OFFLOAD_RSS_PF 0X00080000
+#define I40E_VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM 0X00100000
struct i40e_virtchnl_vf_resource {
u16 num_vsis;
--
2.4.11
^ permalink raw reply related
* [PATCH v2 00/31] net/i40e: base code update
From: Jingjing Wu @ 2016-12-09 14:38 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1480727953-92137-1-git-send-email-jingjing.wu@intel.com>
i40e base code upate. The main changes are:
- add clause22 and clause45 implementation for PHY registers accessing
- replace existing legacy memcpy() calls with i40e_memcpy() calls.
- use BIT() macro instead of bit fields
- add clear all WoL filters implementation
- add ERROR state for NVM update state machine
- add broadcast promiscuous control per VLAN
- remove unused X722_SUPPORT and I40E_NDIS_SUPPORT MARCOs
v2 changes:
- comments rework
- complie issue fix
- rebase to dpdk-next-net
Jingjing Wu (31):
net/i40e/base: add encap csum VF offload flag
net/i40e/base: preserve extended PHY type field
net/i40e/base: remove unnecessary code
net/i40e/base: fix bit test mask
net/i40e/base: group base mode VF offload flags
net/i40e/base: fix long link down notification time
net/i40e/base: add media type detection for 25G link
net/i40e/base: add clause22 and clause45 implementation
net/i40e/base: add bus number info
net/i40e/base: add protocols when discover capabilities
net/i40e/base: pass unknown PHY type for unknown PHYs
net/i40e/base: replace memcpy
net/i40e/base: deprecating unused macro
net/i40e/base: remove FPK HyperV VF device ID
net/i40e/base: add FEC bits to PHY capabilities
net/i40e/base: use BIT() macro instead of bit fields
net/i40e/base: adjust 25G PHY type values
net/i40e/base: implement clear all WoL filters
net/i40e/base: implement set VSI full promisc mode
net/i40e/base: add defines for new aq command
net/i40e/base: save link FEC info from link up event
net/i40e/base: acquire NVM lock before reads on all devices
net/i40e/base: change shift values to hex
net/i40e/base: comment that udp port must be in Host order
net/i40e/base: remove duplicate definitions
net/i40e/base: add ERROR state for NVM update state machine
net/i40e/base: add broadcast promiscuous control per VLAN
net/i40e/base: avoid division by zero
net/i40e/base: fix byte order
net/i40e/base: remove unused macro
net/i40e: remove unused macro from PMD
drivers/net/i40e/Makefile | 2 +-
drivers/net/i40e/base/i40e_adminq.c | 4 +-
drivers/net/i40e/base/i40e_adminq_cmd.h | 51 ++--
drivers/net/i40e/base/i40e_common.c | 425 ++++++++++++++++++++++++++------
drivers/net/i40e/base/i40e_devids.h | 3 -
drivers/net/i40e/base/i40e_lan_hmc.c | 5 -
drivers/net/i40e/base/i40e_nvm.c | 52 ++--
drivers/net/i40e/base/i40e_prototype.h | 30 ++-
drivers/net/i40e/base/i40e_register.h | 2 -
drivers/net/i40e/base/i40e_type.h | 94 +++----
drivers/net/i40e/base/i40e_virtchnl.h | 5 +
drivers/net/i40e/i40e_ethdev.c | 42 +---
drivers/net/i40e/i40e_ethdev_vf.c | 1 -
13 files changed, 468 insertions(+), 248 deletions(-)
--
2.4.11
^ permalink raw reply
* [dev] hi all,can memnic pmd be used in linux container
From: zheng jie @ 2016-12-09 14:24 UTC (permalink / raw)
To: dev@dpdk.org
^ permalink raw reply
* Re: [PATCH v1] net/i40e: set no drop for traffic class
From: Sexton, Rory @ 2016-12-09 14:02 UTC (permalink / raw)
To: Wu, Jingjing; +Cc: dev@dpdk.org, Marjanovic, Nemanja, Mcnamara, John
In-Reply-To: <9BB6961774997848B5B42BEC655768F80E2C29D0@SHSMSX103.ccr.corp.intel.com>
Hi Jingjing,
Yes PRTDCB_TC2PFC is used to control pfc for each TC however we have noticed other advantages of using the register.
By setting the register explicitly by doing the "I40E_WRITE_REG(hw, 0x1c0980, 0xff);" it allows for packets to be temporarily stored on the NICs RX SRAM until there is space for them on SW descriptor ring versus dropping them when the SW ring becomes full. This also allows for larger burst handling. It also means SW doesn't have to be as quick to empty the DRAM based descriptor rings, allowing more processing on cores.
I have tested using the ETH_DCB_PFC_SUPPORT flag in rte_eth_conf.dcb_capability_en and rte_eth_dcb_rx_conf.nb_tcs.
This results in the NIC's RX SRAM not being used and if there is no space on SW descriptor ring for packet it is dropped.
The advantages of using the PRTDCB_TC2PFC explicitly is that there will be no packet loss and descriptor rings do not need to be modified (can be left at 128 for rx and 512 for tx as default settings for apps). Enabling via this register allows Burst handling to be within the NIC Rx Buffer and SW rings combined.
At the moment for tests the rx and tx descriptor rings have to be increased to 2048 to eliminate packet loss.
Ideally it would be an optional setting as using it may increase the max latency.
Regards,
Rory/Nemanja
-----Original Message-----
From: Wu, Jingjing
Sent: Monday, December 5, 2016 8:45 AM
To: Sexton, Rory <rory.sexton@intel.com>
Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>; Mcnamara, John <john.mcnamara@intel.com>
Subject: RE: [PATCH v1] net/i40e: set no drop for traffic class
-----Original Message-----
From: Sexton, Rory
Sent: Sunday, December 4, 2016 9:55 PM
To: Wu, Jingjing <jingjing.wu@intel.com>
Cc: dev@dpdk.org; Marjanovic, Nemanja <nemanja.marjanovic@intel.com>; Mcnamara, John <john.mcnamara@intel.com>; Sexton, Rory <rory.sexton@intel.com>
Subject: [PATCH v1] net/i40e: set no drop for traffic class
From: John McNamara <john.mcnamara@intel.com>
The default traffic class in i40e is set to drop versus on ixgbe it isset to no drop. This means when packets build up in the RX SRAM on the NIC, they are dropped, and they do this when the SW descriptor rings fill up.
This patch changes this behaviour and our testing shows there are no drops as a result.
Signed-off-by: Rory Sexton <rory.sexton@intel.com>
Signed-off-by: Nemanja Marjanovic <nemanja.marjanovic@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 1 +
drivers/net/i40e/i40e_rxtx.c | 12 ++++++++++++
drivers/net/i40e/i40e_rxtx.h | 1 +
lib/librte_ether/rte_ethdev.h | 24 ++++++++++++++++++++++++
4 files changed, 38 insertions(+)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 67778ba..9702acb 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -553,6 +553,7 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
.get_eeprom = i40e_get_eeprom,
.mac_addr_set = i40e_set_default_mac_addr,
.mtu_set = i40e_dev_mtu_set,
+ .set_no_drop = i40e_set_no_drop,
};
/* store statistics names and its offset in stats structure */ diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index 7ae7d9f..02aeff4 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -783,6 +783,18 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
return nb_rx;
}
+uint32_t
+i40e_set_no_drop(struct rte_eth_dev *dev, uint16_t rx_queue_id) {
+ struct i40e_rx_queue *rxq = dev->data->rx_queues[rx_queue_id];
+ struct i40e_hw *hw = I40E_VSI_TO_HW(rxq->vsi);
+
+ /* Set No Drop Traffic Class. */
+ I40E_WRITE_REG(hw, 0x1c0980, 0xff);
+
+ return 1;
+}
0x1c0980 is the register (PRTDCB_TC2PFC) which is used to control pfc for each TC.
We already have ETH_DCB_PFC_SUPPORT flag in rte_eth_conf.dcb_capability_en to Control if PFC is enabled. And rte_eth_dcb_rx_conf.nb_tcs identified number of TCs It supports.
"I40E_WRITE_REG(hw, 0x1c0980, 0xff);" can be achieved by enabling DCB and PFC For all TCs.
Why do we need such a new API?
Thanks
Jingjing
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.
^ permalink raw reply
* Re: dpdk/vpp and cross-version migration for vhost
From: Maxime Coquelin @ 2016-12-09 13:35 UTC (permalink / raw)
To: Yuanhan Liu, Michael S. Tsirkin, Daniel P. Berrange
Cc: dev, Stephen Hemminger, qemu-devel, libvir-list, vpp-dev,
Marc-André Lureau
In-Reply-To: <20161124063129.GE5048@yliu-dev.sh.intel.com>
++Daniel for libvirt
On 11/24/2016 07:31 AM, Yuanhan Liu wrote:
>>>>>>>> As version here is an opaque string for libvirt and qemu,
>>>>>>>> > > > > > >>anything can be used - but I suggest either a list
>>>>>>>> > > > > > >>of values defining the interface, e.g.
>>>>>>>> > > > > > >>any_layout=on,max_ring=256
>>>>>>>> > > > > > >>or a version including the name and vendor of the backend,
>>>>>>>> > > > > > >>e.g. "org.dpdk.v4.5.6".
>>> > >
>>> > > The version scheme may not be ideal here. Assume a QEMU is supposed
>>> > > to work with a specific DPDK version, however, user may disable some
>>> > > newer features through qemu command line, that it also could work with
>>> > > an elder DPDK version. Using the version scheme will not allow us doing
>>> > > such migration to an elder DPDK version. The MTU is a lively example
>>> > > here? (when MTU feature is provided by QEMU but is actually disabled
>>> > > by user, that it could also work with an elder DPDK without MTU support).
>>> > >
>>> > > --yliu
>> >
>> > OK, so does a list of values look better to you then?
> Yes, if there are no better way.
>
> And I think it may be better to not list all those features, literally.
> But instead, using the number should be better, say, features=0xdeadbeef.
>
> Listing the feature names means we have to come to an agreement in all
> components involved here (QEMU, libvirt, DPDK, VPP, and maybe more
> backends), that we have to use the exact same feature names. Though it
> may not be a big deal, it lacks some flexibility.
>
> A feature bits will not have this issue.
I initially thought having key/value pairs would be more flexible, and
could allow migrating to another application if compatible (i.e. from
OVS to VPP, and vice versa...) without needing synchronization between
the applications.
But Daniel pointed me out that it would add a lot of complexity on
management tool side, as it would need to know how to interpret these
key/value pairs. I think his argument is very valid.
So maybe the best way would be the version string, letting the
application (OVS-DPDK/VPP/...) specify which version it is
compatible with.
For the downsides, as soon as a new feature is supported in vhost-user
application, the new version will not be advertised as compatible with
the previous one, even if the user disables the feature in Qemu (as
pointed out by Yuanhan).
The question is are we ready to add complexity on management tool side
to permit more migration cases, or do we prefer keeping it simple but
sometimes prevent migration even if technically possible?
-- Maxime
^ permalink raw reply
* Re: [PATCH 1/8] net/qede: fix to get vendor/device id info
From: Ferruh Yigit @ 2016-12-09 13:35 UTC (permalink / raw)
To: Rasesh Mody, dev; +Cc: Dept-EngDPDKDev
In-Reply-To: <1480746909-25686-1-git-send-email-Rasesh.Mody@cavium.com>
Hi Rasesh,
On 12/3/2016 6:35 AM, Rasesh Mody wrote:
> Fixes: ec94dbc5 ("qede: add base driver")
>
> Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
Related to the commit logs of this patchset. Many people, including me,
won't know technical details of your driver as you do.
>From below patch, I also can see vendor_id and device_id get, which were
not there before. But I have no clue why? Or what happens when you don't
have them, or what works fine when you have them.
Overall a little context helps a lot to understand what is really fixed,
and what happens if not fixed.
For example in patch 8/8, it is a one line easy modification :), but
there is no way that I can understand what really it does, but commit
log comes to help there, and describes it really fixes VF over legacy
PF, by VF asking a FW overwrite instead of failing on PF reject msg. So
patch subject can be here: fix VF over legacy PF.
Please trying to provide more context on fixes.
Also you need to add CC:stable@dpdk.org to commit log, so that git
send-email ensures this fixes also sent to stable trees.
Thanks,
> ---
> drivers/net/qede/qede_ethdev.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index 9c2a5eae..b9a325df 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -2071,6 +2071,10 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
>
> rte_eth_copy_pci_info(eth_dev, pci_dev);
>
> + /* @DPDK */
> + edev->vendor_id = pci_dev->id.vendor_id;
> + edev->device_id = pci_dev->id.device_id;
> +
> qed_ops = qed_get_eth_ops();
> if (!qed_ops) {
> DP_ERR(edev, "Failed to get qed_eth_ops_pass\n");
>
^ permalink raw reply
* [PATCH] net/mlx5: fix Tx doorbell
From: Nelio Laranjeiro @ 2016-12-09 13:27 UTC (permalink / raw)
To: dev
Too much data is uselessly written to the Tx doorbell.
Fixes: 1d88ba171942 ("net/mlx5: refactor Tx data path")
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
This patch should be applied on top of [1] or [2].
[1] http://dpdk.org/ml/archives/dev/2016-November/050716.html
[2] http://dpdk.org/dev/patchwork/patch/17254/
---
drivers/net/mlx5/mlx5_rxtx.c | 26 ++++++++++++--------------
drivers/net/mlx5/mlx5_rxtx.h | 2 --
drivers/net/mlx5/mlx5_txq.c | 2 --
3 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 22b22ac..029d872 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -82,7 +82,8 @@ txq_mp2mr(struct txq *txq, struct rte_mempool *mp)
__attribute__((always_inline));
static inline void
-mlx5_tx_dbrec(struct txq *txq) __attribute__((always_inline));
+mlx5_tx_dbrec(struct txq *txq, volatile struct mlx5_wqe *wqe)
+ __attribute__((always_inline));
static inline uint32_t
rxq_cq_to_pkt_type(volatile struct mlx5_cqe *cqe)
@@ -326,23 +327,20 @@ txq_mp2mr(struct txq *txq, struct rte_mempool *mp)
*
* @param txq
* Pointer to TX queue structure.
+ * @param wqe
+ * Pointer to the last WQE posted in the NIC.
*/
static inline void
-mlx5_tx_dbrec(struct txq *txq)
+mlx5_tx_dbrec(struct txq *txq, volatile struct mlx5_wqe *wqe)
{
- uint8_t *dst = (uint8_t *)((uintptr_t)txq->bf_reg + txq->bf_offset);
- uint32_t data[4] = {
- htonl((txq->wqe_ci << 8) | MLX5_OPCODE_SEND),
- htonl(txq->qp_num_8s),
- 0,
- 0,
- };
+ uint64_t *dst = (uint64_t *)((uintptr_t)txq->bf_reg);
+ volatile uint64_t *src = ((volatile uint64_t *)wqe);
+
rte_wmb();
*txq->qp_db = htonl(txq->wqe_ci);
/* Ensure ordering between DB record and BF copy. */
rte_wmb();
- memcpy(dst, (uint8_t *)data, 16);
- txq->bf_offset ^= (1 << txq->bf_buf_size);
+ *dst = *src;
}
/**
@@ -609,7 +607,7 @@ mlx5_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n)
txq->stats.opackets += i;
#endif
/* Ring QP doorbell. */
- mlx5_tx_dbrec(txq);
+ mlx5_tx_dbrec(txq, (volatile struct mlx5_wqe *)wqe);
txq->elts_head = elts_head;
return i;
}
@@ -816,7 +814,7 @@ mlx5_tx_burst_mpw(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n)
/* Ring QP doorbell. */
if (mpw.state == MLX5_MPW_STATE_OPENED)
mlx5_mpw_close(txq, &mpw);
- mlx5_tx_dbrec(txq);
+ mlx5_tx_dbrec(txq, mpw.wqe);
txq->elts_head = elts_head;
return i;
}
@@ -1084,7 +1082,7 @@ mlx5_tx_burst_mpw_inline(void *dpdk_txq, struct rte_mbuf **pkts,
mlx5_mpw_inline_close(txq, &mpw);
else if (mpw.state == MLX5_MPW_STATE_OPENED)
mlx5_mpw_close(txq, &mpw);
- mlx5_tx_dbrec(txq);
+ mlx5_tx_dbrec(txq, mpw.wqe);
txq->elts_head = elts_head;
return i;
}
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 6dcd35d..e244c48 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -251,8 +251,6 @@ struct txq {
uint16_t elts_n:4; /* (*elts)[] length (in log2). */
uint16_t cqe_n:4; /* Number of CQ elements (in log2). */
uint16_t wqe_n:4; /* Number of of WQ elements (in log2). */
- uint16_t bf_buf_size:4; /* Log2 Blueflame size. */
- uint16_t bf_offset; /* Blueflame offset. */
uint16_t max_inline; /* Multiple of RTE_CACHE_LINE_SIZE to inline. */
uint32_t qp_num_8s; /* QP number shifted by 8. */
volatile struct mlx5_cqe (*cqes)[]; /* Completion queue. */
diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index f4c6682..ad39ddc 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -220,8 +220,6 @@ txq_setup(struct txq_ctrl *tmpl, struct txq_ctrl *txq_ctrl)
tmpl->txq.wqe_n = log2above(qp->sq.wqe_cnt);
tmpl->txq.qp_db = &qp->gen_data.db[MLX5_SND_DBR];
tmpl->txq.bf_reg = qp->gen_data.bf->reg;
- tmpl->txq.bf_offset = qp->gen_data.bf->offset;
- tmpl->txq.bf_buf_size = log2above(qp->gen_data.bf->buf_size);
tmpl->txq.cq_db = cq->dbrec;
tmpl->txq.cqes =
(volatile struct mlx5_cqe (*)[])
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v12 1/6] ethdev: add Tx preparation
From: Kulasek, TomaszX @ 2016-12-09 13:25 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev@dpdk.org, Ananyev, Konstantin, olivier.matz@6wind.com,
Richardson, Bruce
In-Reply-To: <4969291.OX96oIJoy2@xps13>
Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, December 2, 2016 00:51
> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>
> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> olivier.matz@6wind.com; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v12 1/6] ethdev: add Tx preparation
>
> 2016-12-01 22:31, Kulasek, TomaszX:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2016-12-01 19:20, Kulasek, TomaszX:
> > > > Hi Thomas,
> > > >
> > > > Sorry, I have answered for this question in another thread and I
> > > > missed
> > > about this one. Detailed answer is below.
> > >
> > > Yes you already gave this answer.
> > > And I will continue asking the question until you understand it.
> > >
> > > > > 2016-11-28 11:54, Thomas Monjalon:
> > > > > > Hi,
> > > > > >
> > > > > > 2016-11-23 18:36, Tomasz Kulasek:
> > > > > > > --- a/config/common_base
> > > > > > > +++ b/config/common_base
> > > > > > > @@ -120,6 +120,7 @@ CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> > > > > > > CONFIG_RTE_LIBRTE_IEEE1588=n
> > > > > > > CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > > > > > > CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y
> > > > > > > +CONFIG_RTE_ETHDEV_TX_PREPARE=y
> > > > > >
> > > > > > Please, remind me why is there a configuration here.
> > > > > > It should be the responsibility of the application to call
> > > > > > tx_prepare or not. If the application choose to use this new
> > > > > > API but it is disabled, then the packets won't be prepared and
> > > > > > there is
> > > no error code:
> > > > > >
> > > > > > > +#else
> > > > > > > +
> > > > > > > +static inline uint16_t
> > > > > > > +rte_eth_tx_prepare(__rte_unused uint8_t port_id,
> > > > > > > +__rte_unused
> > > > > uint16_t queue_id,
> > > > > > > + __rte_unused struct rte_mbuf **tx_pkts,
> > > > > > > +uint16_t
> > > > > > > +nb_pkts) {
> > > > > > > + return nb_pkts;
> > > > > > > +}
> > > > > > > +
> > > > > > > +#endif
> > > > > >
> > > > > > So the application is not aware of the issue and it will not
> > > > > > use any fallback.
> > > >
> > > > tx_prepare mechanism can be turned off by compilation flag (as
> > > > discussed
> > > with Jerin in http://dpdk.org/dev/patchwork/patch/15770/) to provide
> > > real NOOP functionality (e.g. for low-end CPUs, where even
> > > unnecessary memory dereference and check can have significant impact
> on performance).
> > > >
> > > > Jerin observed that on some architectures (e.g. low-end ARM with
> > > embedded NIC), just reading and comparing 'dev->tx_pkt_prepare' may
> > > cause significant performance drop, so he proposed to introduce this
> > > configuration flag to provide real NOOP when tx_prepare
> > > functionality is not required, and can be turned on based on the
> _target_ configuration.
> > > >
> > > > For other cases, when this flag is turned on (by default), and
> > > tx_prepare is not implemented, functional NOOP is used based on
> > > comparison (dev->tx_pkt_prepare == NULL).
> > >
> > > So if the application call this function and if it is disabled, it
> > > simply won't work. Packets won't be prepared, checksum won't be
> computed.
> > >
> > > I give up, I just NACK.
> >
> > It is not to be turned on/off whatever someone wants, but only and only
> for the case, when platform developer knows, that his platform doesn't
> need this callback, so, he may turn off it and then save some performance
> (this option is per target).
>
> How may he know? There is no comment in the config file, no documentation.
>
> > For this case, the behavior of tx_prepare will be exactly the same when
> it is turned on or off. If is not the same, there's no sense to turn it
> off. There were long topic, where we've tried to convince you, that it
> should be turned on for all devices.
>
> Really? You tried to convince me to turn it on?
> No you were trying to convince Jerin.
> I think it is a wrong idea to allow disabling this function.
> I didn't comment in first discussion because Jerin told it was really
> important for small hardware with fixed NIC, and I thought it would be
> implemented in a way the application cannot be misleaded.
>
> The only solution I see here is to add some comments in the configuration
> file, below the #else and in the doc.
> Have you checked doc/guides/prog_guide/poll_mode_drv.rst?
I can change the name of CONFIG_RTE_ETHDEV_TX_PREPARE=y to something like CONFIG_RTE_ETHDEV_TX_PREPARE_NOOP=n to made it less confusing, and add comments to describe why it is introduced and how it use safely.
I can also remove it at all if you don't like it.
As for doc/guides/prog_guide/poll_mode_drv.rst, do you mean, to add new section describing this feature?
Tomasz
^ permalink raw reply
* Re: [PATCH v1 0/5] net/ixgbe: move set VF functions.
From: Iremonger, Bernard @ 2016-12-09 13:23 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Yigit, Ferruh, dev@dpdk.org
In-Reply-To: <29626520.OD9qkZZgkn@xps13>
Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, December 9, 2016 1:05 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 0/5] net/ixgbe: move set VF functions.
>
> 2016-12-09 12:00, Iremonger, Bernard:
> > From: Yigit, Ferruh
> > > Why this patchset doesn't remove ethdev updates for these functions?
> > >
> > > ixgbe is the only user for these eth-dev_ops, since code moved to
> > > ixgbe driver, they and relevant rte_eth_xx functions (and
> > > deprecation notice) can be removed in this patchset. Most probably
> > > after testpmd updated to prevent compilation errors.
> >
> > My understanding is that the functions should be copied and reworked
> before being removed from the ethdev, and that the removal should be
> done in a separate patch set.
> >
> > Hi Thomas,
> >
> > Could you clarify please.
>
> It was announced that these functions will be removed, so they can be safely
> removed in 17.02, right?
Yes, it was announced in 16.11 that these functions will be removed so I believe they can be safely removed in17.02.
> In this case, it is a code move: copy, rework and delete in the same patch.
> And yes, the deprecation notice must be removed at the same time.
I will add the deletes in a v2 of the patchset.
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v1 0/5] net/ixgbe: move set VF functions.
From: Thomas Monjalon @ 2016-12-09 13:04 UTC (permalink / raw)
To: Iremonger, Bernard; +Cc: Yigit, Ferruh, dev
In-Reply-To: <8CEF83825BEC744B83065625E567D7C224D0601D@IRSMSX108.ger.corp.intel.com>
2016-12-09 12:00, Iremonger, Bernard:
> From: Yigit, Ferruh
> > Why this patchset doesn't remove ethdev updates for these functions?
> >
> > ixgbe is the only user for these eth-dev_ops, since code moved to ixgbe
> > driver, they and relevant rte_eth_xx functions (and deprecation notice) can
> > be removed in this patchset. Most probably after testpmd updated to
> > prevent compilation errors.
>
> My understanding is that the functions should be copied and reworked before being removed from the ethdev, and that the removal should be done in a separate patch set.
>
> Hi Thomas,
>
> Could you clarify please.
It was announced that these functions will be removed,
so they can be safely removed in 17.02, right?
In this case, it is a code move: copy, rework and delete in the same patch.
And yes, the deprecation notice must be removed at the same time.
^ permalink raw reply
* Re: [PATCH 01/22] ethdev: introduce generic flow API
From: Chandran, Sugesh @ 2016-12-09 12:18 UTC (permalink / raw)
To: Adrien Mazarguil
Cc: Kevin Traynor, dev@dpdk.org, Thomas Monjalon,
De Lara Guarch, Pablo, Olivier Matz, sugesh.chandran@intel.comn
In-Reply-To: <20161208150908.GJ10340@6wind.com>
Hi Adrien,
Thank you for your comments,
Please see the reply below.
Regards
_Sugesh
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Thursday, December 8, 2016 3:09 PM
> To: Chandran, Sugesh <sugesh.chandran@intel.com>
> Cc: Kevin Traynor <ktraynor@redhat.com>; dev@dpdk.org; Thomas
> Monjalon <thomas.monjalon@6wind.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Olivier Matz <olivier.matz@6wind.com>;
> sugesh.chandran@intel.comn
> Subject: Re: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API
>
> Hi Sugesh,
>
> On Tue, Dec 06, 2016 at 06:11:38PM +0000, Chandran, Sugesh wrote:
> [...]
> > > >>> +int
> > > >>> +rte_flow_validate(uint8_t port_id,
> > > >>> + const struct rte_flow_attr *attr,
> > > >>> + const struct rte_flow_item pattern[],
> > > >>> + const struct rte_flow_action actions[],
> > > >>> + struct rte_flow_error *error);
> > > >>
> > > >> Why not just use rte_flow_create() and get an error? Is it less
> > > >> disruptive to do a validate and find the rule cannot be created,
> > > >> than using a create directly?
> > > >
> > > > The rationale can be found in the original RFC, which I'll convert
> > > > to actual documentation in v2. In short:
> > > >
> > > > - Calling rte_flow_validate() before rte_flow_create() is useless since
> > > > rte_flow_create() also performs validation.
> > > >
> > > > - We cannot possibly express a full static set of allowed flow rules, even
> > > > if we could, it usually depends on the current hardware configuration
> > > > therefore would not be static.
> > > >
> > > > - rte_flow_validate() is thus provided as a replacement for capability
> > > > flags. It can be used to determine during initialization if the underlying
> > > > device can support the typical flow rules an application might want to
> > > > provide later and do something useful with that information (e.g.
> always
> > > > use software fallback due to HW limitations).
> > > >
> > > > - rte_flow_validate() being a subset of rte_flow_create(), it is
> essentially
> > > > free to expose.
> > >
> > > make sense now, thanks.
> > [Sugesh] : We had this discussion earlier at the design stage about
> > the time taken for programming the hardware, and how to make it
> > deterministic. How about having a timeout parameter as well for the
> > rte_flow_* If the hardware flow insert is timed out, error out than
> > waiting indefinitely, so that application have some control over The
> > time to program the flow. It can be another set of APIs something
> > like, rte_flow_create_timeout()
>
> Yes as discussed the existing API does not provide any timing constraints to
> PMDs, validate() and create() may take forever to complete, although PMDs
> are strongly encouraged to take as little time as possible.
>
> Like you suggested, this could be done through distinct API calls. The
> validate() function would also have its _timeout() counterpart since the set
> of possible rules could be restricted in that mode.
[Sugesh] Thanks!. Looking forward to see an api set with that implementation as well
in the future :). I feel it's a must from the user application point of view.
>
> > Are you going to provide any control over the initialization of NIC
> > to define the capability matrices For eg; To operate in a L3 router mode,
> software wanted to initialize the NIC port only to consider the L2 and L3
> fields.
> > I assume the initialization is done based on the first rules that are
> programmed into the NIC.?
>
> Precisely, PMDs are supposed to determine the most appropriate device
> mode to use in order to handle the requested rules. They may even switch
> to another mode if necessary assuming this does not break existing
> constraints.
>
> I think we've discussed an atomic (commit-based) mode of operation
> through separate functions as well, where the application would attempt to
> create a bunch of rules at once, possibly making it easier for PMDs to
> determine the most appropriate mode of operation for the device.
>
> All of these may be added later according to users feedback once the basic
> API has settled.
[Sugesh] Yes , we discussed about this before. However I feel that, it make sense
to provide some flexibility to the user/application to define a profile/mode of the device.
This way the complexity of determining the mode by itself will be taken away from PMD.
Looking at the P4 enablement patches in OVS, the mode definition APIs can be used in conjunction
P4 behavioral model.
For eg: A P4 model for a L2 switch operate OVS as a L2 switch. Using the mode definition APIs
Its possible to impose the same behavioral model in the hardware too.
This way its simple, clean and very predictive though it needs to define an additional profile_define APIs.
I am sorry to provide the comment at this stage, However looking at the adoption of ebpf, P4 make me
to think this way.
What do you think?
>
> --
> Adrien Mazarguil
> 6WIND
^ permalink raw reply
* Re: [PATCH v2 02/12] lib: add cryptodev type for the upcoming ARMv8 PMD
From: Declan Doherty @ 2016-12-09 12:06 UTC (permalink / raw)
To: Thomas Monjalon, Zbigniew Bodek
Cc: dev, zbigniew.bodek, pablo.de.lara.guarch, jerin.jacob
In-Reply-To: <1683862.k80B36XYkt@xps13>
On 07/12/16 20:09, Thomas Monjalon wrote:
> 2016-12-07 20:04, Zbigniew Bodek:
>> On 06.12.2016 21:27, Thomas Monjalon wrote:
>>> 2016-12-06 18:32, zbigniew.bodek@caviumnetworks.com:
>>>> From: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>>>>
>>>> Add type and name for ARMv8 crypto PMD
>>>>
>>>> Signed-off-by: Zbigniew Bodek <zbigniew.bodek@caviumnetworks.com>
>>> [...]
>>>> --- a/lib/librte_cryptodev/rte_cryptodev.h
>>>> +++ b/lib/librte_cryptodev/rte_cryptodev.h
>>>> @@ -66,6 +66,8 @@
>>>> /**< KASUMI PMD device name */
>>>> #define CRYPTODEV_NAME_ZUC_PMD crypto_zuc
>>>> /**< KASUMI PMD device name */
>>>> +#define CRYPTODEV_NAME_ARMV8_PMD crypto_armv8
>>>> +/**< ARMv8 CM device name */
>>>>
>>>> /** Crypto device type */
>>>> enum rte_cryptodev_type {
>>>> @@ -77,6 +79,7 @@ enum rte_cryptodev_type {
>>>> RTE_CRYPTODEV_KASUMI_PMD, /**< KASUMI PMD */
>>>> RTE_CRYPTODEV_ZUC_PMD, /**< ZUC PMD */
>>>> RTE_CRYPTODEV_OPENSSL_PMD, /**< OpenSSL PMD */
>>>> + RTE_CRYPTODEV_ARMV8_PMD, /**< ARMv8 crypto PMD */
>>>> };
>>>
>>> Can we remove all these types and names in the generic crypto API?
>>>
>>
>> Hello Thomas,
>>
>> I added another PMD type and therefore we need new, unique number for
>> it. I'm not sure if I understand correctly what you mean here, so please
>> elaborate.
>
> My comment is not specific to your PMD.
> I think there is something wrong in the design of cryptodev if we need
> to update rte_cryptodev.h each time a new driver is added.
> There is no such thing in ethdev.
>
Hey Thomas, I've been meaning to have a look at removing this enum, I
just haven't had the time as yet, I think since there is now a standard
naming convention for all pmds, the use for this is redundant.
This change will require a ABI/API deprecation notice, so I'll put that
into 17.02 and then do the patches to remove for 17.05
Declan
^ permalink raw reply
* Re: [PATCH v1 3/5] app/testpmd: add command for set VF VLAN filter
From: Iremonger, Bernard @ 2016-12-09 12:10 UTC (permalink / raw)
To: Yigit, Ferruh, thomas.monjalon@6wind.com, dev@dpdk.org
In-Reply-To: <b1e6668c-8528-7cdd-c3ab-292a39f8722d@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, December 9, 2016 12:03 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>;
> thomas.monjalon@6wind.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 3/5] app/testpmd: add command for set
> VF VLAN filter
>
> On 12/9/2016 11:27 AM, Bernard Iremonger wrote:
> > Add the following command to testpmd:
> > set vf vlan filter <port_id> <vlan_id> <vf_mask> <on|off>
> >
> > Add command to the testpmd user guide.
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
>
> <...>
>
> > +
> > +cmdline_parse_inst_t cmd_set_vf_vlan_filter = {
> > + .f = cmd_set_vf_vlan_filter_parsed,
> > + .data = NULL,
> > + .help_str = "set vf vlan filter <port_id> <vlan_id> <vf_mask>
> > +<on_off>",
>
> s/<on_off>/on|off
>
> Only variables should be wrapped with "<>", fixed string with options can be
> used as a|b|c .
>
> similar issues in other testpmd patches.
>
I will fix in a v2.
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v1 2/5] app/testpmd: use ixgbe public functions
From: Iremonger, Bernard @ 2016-12-09 12:08 UTC (permalink / raw)
To: Yigit, Ferruh, thomas.monjalon@6wind.com, dev@dpdk.org
In-Reply-To: <bb9b767a-591e-7f1b-e39b-1f44605108d2@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, December 9, 2016 11:57 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>;
> thomas.monjalon@6wind.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 2/5] app/testpmd: use ixgbe public
> functions
>
> On 12/9/2016 11:27 AM, Bernard Iremonger wrote:
> > Use the the following ixgbe public functions:
> >
> > rte_pmd_ixgbe_set_vf_rate_limit
> > rte_pmd_ixgbe_set_vf_rx
> > rte_pmd_ixgbe_set_vf_rxmode
> > rte_pmd_ixgbe_set_vf_tx
> > rte_pmd_ixgbe_set_vf_vlan_filter
> >
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
>
> <...>
>
> > @@ -2388,23 +2391,11 @@ int
> > set_vf_rate_limit(portid_t port_id, uint16_t vf, uint16_t rate,
> > uint64_t q_msk) {
> > int diag;
> > - struct rte_eth_link link;
> > -
> > - if (q_msk == 0)
> > - return 0;
> >
> > - if (port_id_is_invalid(port_id, ENABLED_WARN))
> > - return 1;
> > - rte_eth_link_get_nowait(port_id, &link);
> > - if (rate > link.link_speed) {
> > - printf("Invalid rate value:%u bigger than link speed: %u\n",
> > - rate, link.link_speed);
> > - return 1;
> > - }
>
> Why these changes required? Isn't only change is location and naming of the
> ...set_vf_rate_limit() ?
This change is a cleanup as the parameter checking is now done in rte_pmd_ixgbe_set_vf_rate_limit().
It was not possible to test the parameter checking in rte_pmd_ixgbe_set_vf_rate_limit() if the parameters were checked in the set_vf_rate_limit() function.
>
> > - diag = rte_eth_set_vf_rate_limit(port_id, vf, rate, q_msk);
> > + diag = rte_pmd_ixgbe_set_vf_rate_limit(port_id, vf, rate, q_msk);
> > if (diag == 0)
> > return diag;
> > - printf("rte_eth_set_vf_rate_limit for port_id=%d failed diag=%d\n",
> > + printf("rte_pmd_ixgbe_set_vf_rate_limit for port_id=%d failed
> > +diag=%d\n",
> > port_id, diag);
> > return diag;
> > }
> >
Regards,
Bernard.
^ permalink raw reply
* Re: [PATCH v2] cryptodev: fix crash on null dereference
From: De Lara Guarch, Pablo @ 2016-12-09 12:06 UTC (permalink / raw)
To: Jerin Jacob, dev@dpdk.org; +Cc: Doherty, Declan, stable@dpdk.org
In-Reply-To: <1480790041-6398-1-git-send-email-jerin.jacob@caviumnetworks.com>
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Saturday, December 03, 2016 6:34 PM
> To: dev@dpdk.org
> Cc: Doherty, Declan; De Lara Guarch, Pablo; Jerin Jacob; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] cryptodev: fix crash on null dereference
>
> crypodev->data->name will be null when
> rte_cryptodev_get_dev_id() invoked without a valid
> crypto device instance.
>
> Fixes: d11b0f30df88 ("cryptodev: introduce API and framework for crypto
> devices")
>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Acked-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> CC: stable@dpdk.org
Applied to dpdk-next-crypto.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case
From: De Lara Guarch, Pablo @ 2016-12-09 12:05 UTC (permalink / raw)
To: Trahe, Fiona, dev@dpdk.org; +Cc: Griffin, John, Kusztal, ArkadiuszX
In-Reply-To: <1479986267-2836-1-git-send-email-fiona.trahe@intel.com>
Hi Fiona,
> -----Original Message-----
> From: Trahe, Fiona
> Sent: Thursday, November 24, 2016 11:18 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Trahe, Fiona; Griffin, John; Kusztal, ArkadiuszX
> Subject: [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case
>
> In out-of-place operation, data is DMAed from source mbuf
> to destination mbuf. To avoid header data in dest mbuf being
> overwritten, the minimal data-set should be DMAed.
>
> Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for
> performance")
>
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
Should this be in the stable tree? If so, could you add: "CC: stable@dpdk.org" in the commit message?
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH v1 3/5] app/testpmd: add command for set VF VLAN filter
From: Ferruh Yigit @ 2016-12-09 12:02 UTC (permalink / raw)
To: Bernard Iremonger, thomas.monjalon, dev
In-Reply-To: <1481282878-26176-4-git-send-email-bernard.iremonger@intel.com>
On 12/9/2016 11:27 AM, Bernard Iremonger wrote:
> Add the following command to testpmd:
> set vf vlan filter <port_id> <vlan_id> <vf_mask> <on|off>
>
> Add command to the testpmd user guide.
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
<...>
> +
> +cmdline_parse_inst_t cmd_set_vf_vlan_filter = {
> + .f = cmd_set_vf_vlan_filter_parsed,
> + .data = NULL,
> + .help_str = "set vf vlan filter <port_id> <vlan_id> <vf_mask> <on_off>",
s/<on_off>/on|off
Only variables should be wrapped with "<>", fixed string with options
can be used as a|b|c .
similar issues in other testpmd patches.
^ permalink raw reply
* Re: [PATCH v1 0/5] net/ixgbe: move set VF functions.
From: Iremonger, Bernard @ 2016-12-09 12:00 UTC (permalink / raw)
To: Yigit, Ferruh, thomas.monjalon@6wind.com, dev@dpdk.org; +Cc: Iremonger, Bernard
In-Reply-To: <c6b63a88-bf65-8096-5bae-5cfdff161dc2@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, December 9, 2016 11:54 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>;
> thomas.monjalon@6wind.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 0/5] net/ixgbe: move set VF functions.
>
> On 12/9/2016 11:27 AM, Bernard Iremonger wrote:
> > This patchset implements the following deprecation notice:
> > [PATCH v1] doc: announce API and ABI change for librte_ether
> >
> > The following functions from eth_dev_ops have been moved to the ixgbe
> > PMD and renamed:
> >
> > ixgbe_set_pool_rx_mode
> > ixgbe_set_pool_rx
> > ixgbe_set_pool_tx
> > ixgbe_set_pool_vlan_filter
> > ixgbe_set_vf_rate_limit
> >
> > Renamed the functions to the following:
> >
> > rte_pmd_ixgbe_set_vf_rxmode
> > rte_pmd_ixgbe_set_vf_rx
> > rte_pmd_ixgbe_set_vf_tx
> > rte_pmd_ixgbe_set_vf_vlan_filter
> > rte_pmd_ixgbe_set_vf_rate_limit
> >
> > Testpmd has been modified to use the following functions:
> > rte_pmd_ixgbe_set_vf_rxmode
> > rte_pmd_ixgbe_set_vf_rate_limit
> >
> > New testpmd commands have been added to test the following functions:
> > rte_pmd_ixgbe_set_vf_rx
> > rte_pmd_ixgbe_set_vf_tx
> > rte_pmd_ixgbe_set_vf_vlan_filter
> >
> > The testpmd user guide has been updated for the new commands.
> >
> > Bernard Iremonger (5):
> > net/ixgbe: move set VF functions from the ethdev
> > app/testpmd: use ixgbe public functions
> > app/testpmd: add command for set VF VLAN filter
> > app/testpmd: add command for set VF receive
> > app/testpmd: add command for set VF transmit
> >
> > app/test-pmd/cmdline.c | 270
> +++++++++++++++++++++++++++-
> > app/test-pmd/config.c | 31 ++--
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 21 +++
> > drivers/net/ixgbe/ixgbe_ethdev.c | 263
> +++++++++++++++++++++++++++
> > drivers/net/ixgbe/rte_pmd_ixgbe.h | 104 +++++++++++
> > drivers/net/ixgbe/rte_pmd_ixgbe_version.map | 10 ++
> > 6 files changed, 678 insertions(+), 21 deletions(-)
> >
>
> Why this patchset doesn't remove ethdev updates for these functions?
>
> ixgbe is the only user for these eth-dev_ops, since code moved to ixgbe
> driver, they and relevant rte_eth_xx functions (and deprecation notice) can
> be removed in this patchset. Most probably after testpmd updated to
> prevent compilation errors.
My understanding is that the functions should be copied and reworked before being removed from the ethdev, and that the removal should be done in a separate patch set.
Hi Thomas,
Could you clarify please.
Regards,
Bernard.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox