[dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Zijie Pan]

Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
It supports the following i40e debug lldp commands:
    - start/stop of the LLDP agent.
    - get local/remote of the LLDP MIB (Management Information Base).

Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
---
 drivers/net/i40e/i40e_ethdev.h            |   19 +++++++++
 drivers/net/i40e/rte_pmd_i40e.c           |   64 +++++++++++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e.h           |   22 ++++++++++
 drivers/net/i40e/rte_pmd_i40e_version.map |    8 +++-
 4 files changed, 112 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 55c8875..459b3ff 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -267,6 +267,25 @@ enum i40e_flxpld_layer_idx {
 struct i40e_adapter;
 
 /**
+ * LLDP command type
+ */
+enum i40_lldp_cmd_type {
+	I40E_LLDP_CMD_START = 0,
+	I40E_LLDP_CMD_STOP,
+	I40E_LLDP_CMD_GET_LOCAL,
+	I40E_LLDP_CMD_GET_REMOTE,
+	I40E_LLDP_CMD_UNKNOWN,
+};
+
+/**
+ * LLDP command structure
+ */
+struct i40e_lldp_cmd {
+	const char *name;
+	enum i40_lldp_cmd_type cmd;
+};
+
+/**
  * MAC filter structure
  */
 struct i40e_mac_filter_info {
diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i40e.c
index 7aa1a75..307c597 100644
--- a/drivers/net/i40e/rte_pmd_i40e.c
+++ b/drivers/net/i40e/rte_pmd_i40e.c
@@ -12,6 +12,14 @@
 #include "i40e_rxtx.h"
 #include "rte_pmd_i40e.h"
 
+static const struct i40e_lldp_cmd lldp_cmds_table[] = {
+	{"start", I40E_LLDP_CMD_START},
+	{"stop", I40E_LLDP_CMD_STOP},
+	{"get local", I40E_LLDP_CMD_GET_LOCAL},
+	{"get remote", I40E_LLDP_CMD_GET_REMOTE},
+	{"", I40E_LLDP_CMD_UNKNOWN},
+};
+
 int
 rte_pmd_i40e_ping_vfs(uint16_t port, uint16_t vf)
 {
@@ -3192,3 +3200,59 @@ int rte_pmd_i40e_flow_add_del_packet_template(
 	I40E_WRITE_FLUSH(hw);
 	return 0;
 }
+
+int __rte_experimental
+rte_pmd_i40e_set_lldp_cmd(uint16_t port, const char *cmd, void *lldpmib)
+{
+	struct rte_eth_dev *dev;
+	struct i40e_hw *hw;
+	uint8_t br_type;
+	int i, ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+
+	if (!is_i40e_supported(dev))
+		return -ENOTSUP;
+
+	hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	for (i = 0; i < I40E_LLDP_CMD_UNKNOWN; i++) {
+		if (!strcmp(cmd, lldp_cmds_table[i].name))
+			break;
+	}
+
+	if (lldp_cmds_table[i].cmd == I40E_LLDP_CMD_UNKNOWN) {
+		PMD_DRV_LOG(ERR, "Unknown LLDP command\n");
+		return -EINVAL;
+	}
+
+	switch (lldp_cmds_table[i].cmd) {
+	case I40E_LLDP_CMD_START:
+		ret = i40e_aq_start_lldp(hw, NULL);
+		break;
+	case I40E_LLDP_CMD_STOP:
+		ret = i40e_aq_stop_lldp(hw, true, NULL);
+		break;
+	case I40E_LLDP_CMD_GET_LOCAL:
+		ret = i40e_aq_get_lldp_mib(hw, 0,
+					   I40E_AQ_LLDP_MIB_LOCAL,
+					   lldpmib, I40E_LLDPDU_SIZE,
+					   NULL, NULL, NULL);
+		break;
+	case I40E_LLDP_CMD_GET_REMOTE:
+		br_type = I40E_AQ_LLDP_BRIDGE_TYPE_NEAREST_BRIDGE;
+		ret = i40e_aq_get_lldp_mib(hw, br_type,
+					   I40E_AQ_LLDP_MIB_REMOTE,
+					   lldpmib, I40E_LLDPDU_SIZE,
+					   NULL, NULL, NULL);
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "Unknown LLDP command\n");
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index be4a602..59b2eee 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -1061,4 +1061,26 @@ int rte_pmd_i40e_inset_set(uint16_t port, uint8_t pctype,
 	return 0;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Process the LLDP commands, including "lldp start", "lldp stop",
+ * "lldp get local" and "lldp get remote".
+ *
+ * @param port
+ *    The port identifier of the Ethernet device.
+ * @param cmd
+ *    The LLDP command.
+ * @param lldpmib
+ *    The pointer to return the LLDP MIB.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if *cmd* invalid.
+ *   - (-ENOTSUP) not supported by firmware.
+ */
+int __rte_experimental
+rte_pmd_i40e_set_lldp_cmd(uint16_t port, const char *cmd, void *lldpmib);
+
 #endif /* _PMD_I40E_H_ */
diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
index cccd576..1ffa02a 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -64,4 +64,10 @@ DPDK_18.02 {
 
 	rte_pmd_i40e_inset_get;
 	rte_pmd_i40e_inset_set;
-} DPDK_17.11;
\ No newline at end of file
+} DPDK_17.11;
+
+EXPERIMENTAL {
+	global:
+
+	rte_pmd_i40e_set_lldp_cmd;
+};
-- 
1.7.10.4

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Thomas Monjalon]

18/05/2018 12:14, Zijie Pan:
> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> It supports the following i40e debug lldp commands:
>     - start/stop of the LLDP agent.
>     - get local/remote of the LLDP MIB (Management Information Base).
> 
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>

Does it mean that this feature is enabled by default in i40e devices?
Should it be disabled by default?

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Zijie Pan]

> > Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> > It supports the following i40e debug lldp commands:
> >     - start/stop of the LLDP agent.
> >     - get local/remote of the LLDP MIB (Management Information Base).
> >
> > Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> > Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
>
> Does it mean that this feature is enabled by default in i40e devices?
> Should it be disabled by default?

The new API is just an interface to control the LLDP agent. No functions of i40e call it by default. It is only used to debug LLDP of i40e devices. For example we can add a new command for test-pmd to debug lldp of i40e by calling rte_pmd_i40e_set_lldp_cmd().

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Thomas Monjalon]

21/05/2018 05:35, Zijie Pan:
> > > Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> > > It supports the following i40e debug lldp commands:
> > >     - start/stop of the LLDP agent.
> > >     - get local/remote of the LLDP MIB (Management Information Base).
> > >
> > > Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> > > Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
> >
> > Does it mean that this feature is enabled by default in i40e devices?
> > Should it be disabled by default?
> 
> The new API is just an interface to control the LLDP agent. No functions of i40e call it by default. It is only used to debug LLDP of i40e devices. For example we can add a new command for test-pmd to debug lldp of i40e by calling rte_pmd_i40e_set_lldp_cmd().

There is a command to stop the LLDP agent.
My question is to know whether this agent is running by default
and should be disabled?
I mean i40e is generating LLDP packets by default?

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Laurent Hardy]

Hello Thomas,

On 05/21/2018 10:39 AM, Thomas Monjalon wrote:
> 21/05/2018 05:35, Zijie Pan:
>>>> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
>>>> It supports the following i40e debug lldp commands:
>>>>      - start/stop of the LLDP agent.
>>>>      - get local/remote of the LLDP MIB (Management Information Base).
>>>>
>>>> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
>>>> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
>>> Does it mean that this feature is enabled by default in i40e devices?
>>> Should it be disabled by default?
>> The new API is just an interface to control the LLDP agent. No functions of i40e call it by default. It is only used to debug LLDP of i40e devices. For example we can add a new command for test-pmd to debug lldp of i40e by calling rte_pmd_i40e_set_lldp_cmd().
> There is a command to stop the LLDP agent.
> My question is to know whether this agent is running by default
> and should be disabled?
This agent is running by default.
Beside, disabling LLDP agent when using old fw (NVM 4.53 or 5.x) could 
have some side effects.

https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/xl710-10-40-controller-spec-update.pdf

Extracted from x710 intel spec:

"70. LLDP Disable Can Result in Incorrect Configuration of the Receive Packet Buffer

Problem:
---------
The default operation of the device is LLDP enabled.
LLDP can be disabled by an AQ command or by a custom NVM obtained from Intel.
Disabling LLDP when using NVM 4.53 or 5.x can result in the device firmware not configuring the Receive Packet Buffer according to the link mode and flow control settings.
Implication:
------------
If flow control is disabled, a hang of the receive data path could occur wherein all received packets are dropped.
If flow control is enabled, the incorrect settings could cause packet drops despite the flow control, making the flow control ineffective.
Also, throughput might be sub-optimal in some cases.
Workaround:
-----------
When using NVM 4.53 or 5.x, do one of the following:
•  Do not disable LLDP or DCBx. Leave the device in default operation mode.
•  Use the Set Local LLDP MIB admin queue command to apply the (default) DCB configuration during driver initialization and following any link-up event.
When using NVM 6.01 or later, the receive data path does not hang even if the FW LLDP is disabled."

> I mean i40e is generating LLDP packets by default?
>
>

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Thomas Monjalon]

21/05/2018 10:58, Laurent Hardy:
> Hello Thomas,
> 
> On 05/21/2018 10:39 AM, Thomas Monjalon wrote:
> > 21/05/2018 05:35, Zijie Pan:
> >>>> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> >>>> It supports the following i40e debug lldp commands:
> >>>>      - start/stop of the LLDP agent.
> >>>>      - get local/remote of the LLDP MIB (Management Information Base).
> >>>>
> >>>> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> >>>> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
> >>> Does it mean that this feature is enabled by default in i40e devices?
> >>> Should it be disabled by default?
> >> The new API is just an interface to control the LLDP agent. No functions of i40e call it by default. It is only used to debug LLDP of i40e devices. For example we can add a new command for test-pmd to debug lldp of i40e by calling rte_pmd_i40e_set_lldp_cmd().
> > There is a command to stop the LLDP agent.
> > My question is to know whether this agent is running by default
> > and should be disabled?
> This agent is running by default.
> Beside, disabling LLDP agent when using old fw (NVM 4.53 or 5.x) could 
> have some side effects.
> 
> https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/xl710-10-40-controller-spec-update.pdf
> 
> Extracted from x710 intel spec:
> 
> "70. LLDP Disable Can Result in Incorrect Configuration of the Receive Packet Buffer
> 
> Problem:
> ---------
> The default operation of the device is LLDP enabled.
> LLDP can be disabled by an AQ command or by a custom NVM obtained from Intel.
> Disabling LLDP when using NVM 4.53 or 5.x can result in the device firmware not configuring the Receive Packet Buffer according to the link mode and flow control settings.
> Implication:
> ------------
> If flow control is disabled, a hang of the receive data path could occur wherein all received packets are dropped.
> If flow control is enabled, the incorrect settings could cause packet drops despite the flow control, making the flow control ineffective.
> Also, throughput might be sub-optimal in some cases.
> Workaround:
> -----------
> When using NVM 4.53 or 5.x, do one of the following:
> •  Do not disable LLDP or DCBx. Leave the device in default operation mode.
> •  Use the Set Local LLDP MIB admin queue command to apply the (default) DCB configuration during driver initialization and following any link-up event.
> When using NVM 6.01 or later, the receive data path does not hang even if the FW LLDP is disabled."

OK, interesting, thank you.

I think you should add a comment in the code to explain why it is not disabled.
And later, we could consider disabling it with new firmwares.


> > I mean i40e is generating LLDP packets by default?

So the answer is yes, i40e generates packets silently :)

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Zhang, Qi Z]

Hi Zijie:

> -----Original Message-----
> From: Zijie Pan [mailto:zijie.pan@6wind.com]
> Sent: Friday, May 18, 2018 6:14 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Laurent Hardy <laurent.hardy@6wind.com>
> Subject: [PATCH] net/i40e: add a specific API to control the LLDP agent
> 
> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> It supports the following i40e debug lldp commands:

I'm not very sure if it is necessary to expose more APIs just for debug?
As you know we have the patch Fixes: 4861cde46116 ("i40e: new poll mode driver") for the knowing issue.
Now, if new firmware fix this, we properly can add LLDP disable back with properly firmware check branch (of cause with enough test first)

Regards
Qi


>     - start/stop of the LLDP agent.
>     - get local/remote of the LLDP MIB (Management Information Base).
> 
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
> ---
>  drivers/net/i40e/i40e_ethdev.h            |   19 +++++++++
>  drivers/net/i40e/rte_pmd_i40e.c           |   64
> +++++++++++++++++++++++++++++
>  drivers/net/i40e/rte_pmd_i40e.h           |   22 ++++++++++
>  drivers/net/i40e/rte_pmd_i40e_version.map |    8 +++-
>  4 files changed, 112 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 55c8875..459b3ff 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -267,6 +267,25 @@ enum i40e_flxpld_layer_idx {  struct i40e_adapter;
> 
>  /**
> + * LLDP command type
> + */
> +enum i40_lldp_cmd_type {
> +	I40E_LLDP_CMD_START = 0,
> +	I40E_LLDP_CMD_STOP,
> +	I40E_LLDP_CMD_GET_LOCAL,
> +	I40E_LLDP_CMD_GET_REMOTE,
> +	I40E_LLDP_CMD_UNKNOWN,
> +};
> +
> +/**
> + * LLDP command structure
> + */
> +struct i40e_lldp_cmd {
> +	const char *name;
> +	enum i40_lldp_cmd_type cmd;
> +};
> +
> +/**
>   * MAC filter structure
>   */
>  struct i40e_mac_filter_info {
> diff --git a/drivers/net/i40e/rte_pmd_i40e.c
> b/drivers/net/i40e/rte_pmd_i40e.c index 7aa1a75..307c597 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.c
> +++ b/drivers/net/i40e/rte_pmd_i40e.c
> @@ -12,6 +12,14 @@
>  #include "i40e_rxtx.h"
>  #include "rte_pmd_i40e.h"
> 
> +static const struct i40e_lldp_cmd lldp_cmds_table[] = {
> +	{"start", I40E_LLDP_CMD_START},
> +	{"stop", I40E_LLDP_CMD_STOP},
> +	{"get local", I40E_LLDP_CMD_GET_LOCAL},
> +	{"get remote", I40E_LLDP_CMD_GET_REMOTE},
> +	{"", I40E_LLDP_CMD_UNKNOWN},
> +};
> +
>  int
>  rte_pmd_i40e_ping_vfs(uint16_t port, uint16_t vf)  { @@ -3192,3 +3200,59
> @@ int rte_pmd_i40e_flow_add_del_packet_template(
>  	I40E_WRITE_FLUSH(hw);
>  	return 0;
>  }
> +
> +int __rte_experimental
> +rte_pmd_i40e_set_lldp_cmd(uint16_t port, const char *cmd, void
> +*lldpmib) {
> +	struct rte_eth_dev *dev;
> +	struct i40e_hw *hw;
> +	uint8_t br_type;
> +	int i, ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> +	dev = &rte_eth_devices[port];
> +
> +	if (!is_i40e_supported(dev))
> +		return -ENOTSUP;
> +
> +	hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +	for (i = 0; i < I40E_LLDP_CMD_UNKNOWN; i++) {
> +		if (!strcmp(cmd, lldp_cmds_table[i].name))
> +			break;
> +	}
> +
> +	if (lldp_cmds_table[i].cmd == I40E_LLDP_CMD_UNKNOWN) {
> +		PMD_DRV_LOG(ERR, "Unknown LLDP command\n");
> +		return -EINVAL;
> +	}
> +
> +	switch (lldp_cmds_table[i].cmd) {
> +	case I40E_LLDP_CMD_START:
> +		ret = i40e_aq_start_lldp(hw, NULL);
> +		break;
> +	case I40E_LLDP_CMD_STOP:
> +		ret = i40e_aq_stop_lldp(hw, true, NULL);
> +		break;
> +	case I40E_LLDP_CMD_GET_LOCAL:
> +		ret = i40e_aq_get_lldp_mib(hw, 0,
> +					   I40E_AQ_LLDP_MIB_LOCAL,
> +					   lldpmib, I40E_LLDPDU_SIZE,
> +					   NULL, NULL, NULL);
> +		break;
> +	case I40E_LLDP_CMD_GET_REMOTE:
> +		br_type = I40E_AQ_LLDP_BRIDGE_TYPE_NEAREST_BRIDGE;
> +		ret = i40e_aq_get_lldp_mib(hw, br_type,
> +					   I40E_AQ_LLDP_MIB_REMOTE,
> +					   lldpmib, I40E_LLDPDU_SIZE,
> +					   NULL, NULL, NULL);
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "Unknown LLDP command\n");
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> diff --git a/drivers/net/i40e/rte_pmd_i40e.h
> b/drivers/net/i40e/rte_pmd_i40e.h index be4a602..59b2eee 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.h
> +++ b/drivers/net/i40e/rte_pmd_i40e.h
> @@ -1061,4 +1061,26 @@ int rte_pmd_i40e_inset_set(uint16_t port, uint8_t
> pctype,
>  	return 0;
>  }
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Process the LLDP commands, including "lldp start", "lldp stop",
> + * "lldp get local" and "lldp get remote".
> + *
> + * @param port
> + *    The port identifier of the Ethernet device.
> + * @param cmd
> + *    The LLDP command.
> + * @param lldpmib
> + *    The pointer to return the LLDP MIB.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port* invalid.
> + *   - (-EINVAL) if *cmd* invalid.
> + *   - (-ENOTSUP) not supported by firmware.
> + */
> +int __rte_experimental
> +rte_pmd_i40e_set_lldp_cmd(uint16_t port, const char *cmd, void
> +*lldpmib);
> +
>  #endif /* _PMD_I40E_H_ */
> diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map
> b/drivers/net/i40e/rte_pmd_i40e_version.map
> index cccd576..1ffa02a 100644
> --- a/drivers/net/i40e/rte_pmd_i40e_version.map
> +++ b/drivers/net/i40e/rte_pmd_i40e_version.map
> @@ -64,4 +64,10 @@ DPDK_18.02 {
> 
>  	rte_pmd_i40e_inset_get;
>  	rte_pmd_i40e_inset_set;
> -} DPDK_17.11;
> \ No newline at end of file
> +} DPDK_17.11;
> +
> +EXPERIMENTAL {
> +	global:
> +
> +	rte_pmd_i40e_set_lldp_cmd;
> +};
> --
> 1.7.10.4

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Laurent Hardy]

Hi Qi,


On 05/21/2018 03:11 PM, Zhang, Qi Z wrote:
> Hi Zijie:
>
>> -----Original Message-----
>> From: Zijie Pan [mailto:zijie.pan@6wind.com]
>> Sent: Friday, May 18, 2018 6:14 PM
>> To: dev@dpdk.org
>> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
>> Laurent Hardy <laurent.hardy@6wind.com>
>> Subject: [PATCH] net/i40e: add a specific API to control the LLDP agent
>>
>> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
>> It supports the following i40e debug lldp commands:
> I'm not very sure if it is necessary to expose more APIs just for debug?
> As you know we have the patch Fixes: 4861cde46116 ("i40e: new poll mode driver") for the knowing issue.
> Now, if new firmware fix this, we properly can add LLDP disable back with properly firmware check branch (of cause with enough test first)
There is actually a real need (from customers) to have some control on 
the LLDP agent.
In kernel driver, this feature exists as it is currently possible to 
control the LLDP agent through a sysfs entry 
(/sys/kernel/debug/i40e/[INTERFACE]/cmd).
The purpose of this debug API is to provide same level of support via dpdk.

best regards,
Laurent
> Regards
> Qi
>
>
>>      - start/stop of the LLDP agent.
>>      - get local/remote of the LLDP MIB (Management Information Base).
>>
>> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
>> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
>> ---
>>   drivers/net/i40e/i40e_ethdev.h            |   19 +++++++++
>>   drivers/net/i40e/rte_pmd_i40e.c           |   64
>> +++++++++++++++++++++++++++++
>>   drivers/net/i40e/rte_pmd_i40e.h           |   22 ++++++++++
>>   drivers/net/i40e/rte_pmd_i40e_version.map |    8 +++-
>>   4 files changed, 112 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
>> index 55c8875..459b3ff 100644
>> --- a/drivers/net/i40e/i40e_ethdev.h
>> +++ b/drivers/net/i40e/i40e_ethdev.h
>> @@ -267,6 +267,25 @@ enum i40e_flxpld_layer_idx {  struct i40e_adapter;
>>
>>   /**
>> + * LLDP command type
>> + */
>> +enum i40_lldp_cmd_type {
>> +	I40E_LLDP_CMD_START = 0,
>> +	I40E_LLDP_CMD_STOP,
>> +	I40E_LLDP_CMD_GET_LOCAL,
>> +	I40E_LLDP_CMD_GET_REMOTE,
>> +	I40E_LLDP_CMD_UNKNOWN,
>> +};
>> +
>> +/**
>> + * LLDP command structure
>> + */
>> +struct i40e_lldp_cmd {
>> +	const char *name;
>> +	enum i40_lldp_cmd_type cmd;
>> +};
>> +
>> +/**
>>    * MAC filter structure
>>    */
>>   struct i40e_mac_filter_info {
>> diff --git a/drivers/net/i40e/rte_pmd_i40e.c
>> b/drivers/net/i40e/rte_pmd_i40e.c index 7aa1a75..307c597 100644
>> --- a/drivers/net/i40e/rte_pmd_i40e.c
>> +++ b/drivers/net/i40e/rte_pmd_i40e.c
>> @@ -12,6 +12,14 @@
>>   #include "i40e_rxtx.h"
>>   #include "rte_pmd_i40e.h"
>>
>> +static const struct i40e_lldp_cmd lldp_cmds_table[] = {
>> +	{"start", I40E_LLDP_CMD_START},
>> +	{"stop", I40E_LLDP_CMD_STOP},
>> +	{"get local", I40E_LLDP_CMD_GET_LOCAL},
>> +	{"get remote", I40E_LLDP_CMD_GET_REMOTE},
>> +	{"", I40E_LLDP_CMD_UNKNOWN},
>> +};
>> +
>>   int
>>   rte_pmd_i40e_ping_vfs(uint16_t port, uint16_t vf)  { @@ -3192,3 +3200,59
>> @@ int rte_pmd_i40e_flow_add_del_packet_template(
>>   	I40E_WRITE_FLUSH(hw);
>>   	return 0;
>>   }
>> +
>> +int __rte_experimental
>> +rte_pmd_i40e_set_lldp_cmd(uint16_t port, const char *cmd, void
>> +*lldpmib) {
>> +	struct rte_eth_dev *dev;
>> +	struct i40e_hw *hw;
>> +	uint8_t br_type;
>> +	int i, ret;
>> +
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
>> +
>> +	dev = &rte_eth_devices[port];
>> +
>> +	if (!is_i40e_supported(dev))
>> +		return -ENOTSUP;
>> +
>> +	hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>> +
>> +	for (i = 0; i < I40E_LLDP_CMD_UNKNOWN; i++) {
>> +		if (!strcmp(cmd, lldp_cmds_table[i].name))
>> +			break;
>> +	}
>> +
>> +	if (lldp_cmds_table[i].cmd == I40E_LLDP_CMD_UNKNOWN) {
>> +		PMD_DRV_LOG(ERR, "Unknown LLDP command\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	switch (lldp_cmds_table[i].cmd) {
>> +	case I40E_LLDP_CMD_START:
>> +		ret = i40e_aq_start_lldp(hw, NULL);
>> +		break;
>> +	case I40E_LLDP_CMD_STOP:
>> +		ret = i40e_aq_stop_lldp(hw, true, NULL);
>> +		break;
>> +	case I40E_LLDP_CMD_GET_LOCAL:
>> +		ret = i40e_aq_get_lldp_mib(hw, 0,
>> +					   I40E_AQ_LLDP_MIB_LOCAL,
>> +					   lldpmib, I40E_LLDPDU_SIZE,
>> +					   NULL, NULL, NULL);
>> +		break;
>> +	case I40E_LLDP_CMD_GET_REMOTE:
>> +		br_type = I40E_AQ_LLDP_BRIDGE_TYPE_NEAREST_BRIDGE;
>> +		ret = i40e_aq_get_lldp_mib(hw, br_type,
>> +					   I40E_AQ_LLDP_MIB_REMOTE,
>> +					   lldpmib, I40E_LLDPDU_SIZE,
>> +					   NULL, NULL, NULL);
>> +		break;
>> +	default:
>> +		PMD_DRV_LOG(ERR, "Unknown LLDP command\n");
>> +		ret = -EINVAL;
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> diff --git a/drivers/net/i40e/rte_pmd_i40e.h
>> b/drivers/net/i40e/rte_pmd_i40e.h index be4a602..59b2eee 100644
>> --- a/drivers/net/i40e/rte_pmd_i40e.h
>> +++ b/drivers/net/i40e/rte_pmd_i40e.h
>> @@ -1061,4 +1061,26 @@ int rte_pmd_i40e_inset_set(uint16_t port, uint8_t
>> pctype,
>>   	return 0;
>>   }
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Process the LLDP commands, including "lldp start", "lldp stop",
>> + * "lldp get local" and "lldp get remote".
>> + *
>> + * @param port
>> + *    The port identifier of the Ethernet device.
>> + * @param cmd
>> + *    The LLDP command.
>> + * @param lldpmib
>> + *    The pointer to return the LLDP MIB.
>> + * @return
>> + *   - (0) if successful.
>> + *   - (-ENODEV) if *port* invalid.
>> + *   - (-EINVAL) if *cmd* invalid.
>> + *   - (-ENOTSUP) not supported by firmware.
>> + */
>> +int __rte_experimental
>> +rte_pmd_i40e_set_lldp_cmd(uint16_t port, const char *cmd, void
>> +*lldpmib);
>> +
>>   #endif /* _PMD_I40E_H_ */
>> diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map
>> b/drivers/net/i40e/rte_pmd_i40e_version.map
>> index cccd576..1ffa02a 100644
>> --- a/drivers/net/i40e/rte_pmd_i40e_version.map
>> +++ b/drivers/net/i40e/rte_pmd_i40e_version.map
>> @@ -64,4 +64,10 @@ DPDK_18.02 {
>>
>>   	rte_pmd_i40e_inset_get;
>>   	rte_pmd_i40e_inset_set;
>> -} DPDK_17.11;
>> \ No newline at end of file
>> +} DPDK_17.11;
>> +
>> +EXPERIMENTAL {
>> +	global:
>> +
>> +	rte_pmd_i40e_set_lldp_cmd;
>> +};
>> --
>> 1.7.10.4

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Zhang, Qi Z]

> -----Original Message-----
> From: Zijie Pan [mailto:zijie.pan@6wind.com]
> Sent: Friday, May 18, 2018 6:14 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Laurent Hardy <laurent.hardy@6wind.com>
> Subject: [PATCH] net/i40e: add a specific API to control the LLDP agent
> 
> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> It supports the following i40e debug lldp commands:
>     - start/stop of the LLDP agent.
>     - get local/remote of the LLDP MIB (Management Information Base).
> 
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

[dpdk-dev] [PATCH v2] net/i40e: add a specific API to control the LLDP agent

[Zijie Pan]

Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
It supports the following i40e debug lldp commands:
    - start/stop of the LLDP agent.
    - get local/remote of the LLDP MIB (Management Information Base).

Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
Acked-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.h            |   19 ++++++++
 drivers/net/i40e/rte_pmd_i40e.c           |   69 +++++++++++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e.h           |   22 +++++++++
 drivers/net/i40e/rte_pmd_i40e_version.map |    8 +++-
 4 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 55c8875..459b3ff 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -267,6 +267,25 @@ enum i40e_flxpld_layer_idx {
 struct i40e_adapter;
 
 /**
+ * LLDP command type
+ */
+enum i40_lldp_cmd_type {
+	I40E_LLDP_CMD_START = 0,
+	I40E_LLDP_CMD_STOP,
+	I40E_LLDP_CMD_GET_LOCAL,
+	I40E_LLDP_CMD_GET_REMOTE,
+	I40E_LLDP_CMD_UNKNOWN,
+};
+
+/**
+ * LLDP command structure
+ */
+struct i40e_lldp_cmd {
+	const char *name;
+	enum i40_lldp_cmd_type cmd;
+};
+
+/**
  * MAC filter structure
  */
 struct i40e_mac_filter_info {
diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i40e.c
index 7aa1a75..612eb6d 100644
--- a/drivers/net/i40e/rte_pmd_i40e.c
+++ b/drivers/net/i40e/rte_pmd_i40e.c
@@ -12,6 +12,14 @@
 #include "i40e_rxtx.h"
 #include "rte_pmd_i40e.h"
 
+static const struct i40e_lldp_cmd lldp_cmds_table[] = {
+	{"start", I40E_LLDP_CMD_START},
+	{"stop", I40E_LLDP_CMD_STOP},
+	{"get local", I40E_LLDP_CMD_GET_LOCAL},
+	{"get remote", I40E_LLDP_CMD_GET_REMOTE},
+	{"", I40E_LLDP_CMD_UNKNOWN},
+};
+
 int
 rte_pmd_i40e_ping_vfs(uint16_t port, uint16_t vf)
 {
@@ -3192,3 +3200,64 @@ int rte_pmd_i40e_flow_add_del_packet_template(
 	I40E_WRITE_FLUSH(hw);
 	return 0;
 }
+
+/**
+ * LLDP is enabled by default. Disabling LLDP when using NVM 4.53 or 5.x can
+ * result in the device firmware not configuring the Receive Packet Buffer
+ * according to the link mode and flow control settings.
+ */
+int __rte_experimental
+rte_pmd_i40e_set_lldp_cmd(uint16_t port, const char *cmd, void *lldpmib)
+{
+	struct rte_eth_dev *dev;
+	struct i40e_hw *hw;
+	uint8_t br_type;
+	int i, ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+
+	if (!is_i40e_supported(dev))
+		return -ENOTSUP;
+
+	hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	for (i = 0; i < I40E_LLDP_CMD_UNKNOWN; i++) {
+		if (!strcmp(cmd, lldp_cmds_table[i].name))
+			break;
+	}
+
+	if (lldp_cmds_table[i].cmd == I40E_LLDP_CMD_UNKNOWN) {
+		PMD_DRV_LOG(ERR, "Unknown LLDP command\n");
+		return -EINVAL;
+	}
+
+	switch (lldp_cmds_table[i].cmd) {
+	case I40E_LLDP_CMD_START:
+		ret = i40e_aq_start_lldp(hw, NULL);
+		break;
+	case I40E_LLDP_CMD_STOP:
+		ret = i40e_aq_stop_lldp(hw, true, NULL);
+		break;
+	case I40E_LLDP_CMD_GET_LOCAL:
+		ret = i40e_aq_get_lldp_mib(hw, 0,
+					   I40E_AQ_LLDP_MIB_LOCAL,
+					   lldpmib, I40E_LLDPDU_SIZE,
+					   NULL, NULL, NULL);
+		break;
+	case I40E_LLDP_CMD_GET_REMOTE:
+		br_type = I40E_AQ_LLDP_BRIDGE_TYPE_NEAREST_BRIDGE;
+		ret = i40e_aq_get_lldp_mib(hw, br_type,
+					   I40E_AQ_LLDP_MIB_REMOTE,
+					   lldpmib, I40E_LLDPDU_SIZE,
+					   NULL, NULL, NULL);
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "Unknown LLDP command\n");
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index be4a602..59b2eee 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -1061,4 +1061,26 @@ int rte_pmd_i40e_inset_set(uint16_t port, uint8_t pctype,
 	return 0;
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Process the LLDP commands, including "lldp start", "lldp stop",
+ * "lldp get local" and "lldp get remote".
+ *
+ * @param port
+ *    The port identifier of the Ethernet device.
+ * @param cmd
+ *    The LLDP command.
+ * @param lldpmib
+ *    The pointer to return the LLDP MIB.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if *cmd* invalid.
+ *   - (-ENOTSUP) not supported by firmware.
+ */
+int __rte_experimental
+rte_pmd_i40e_set_lldp_cmd(uint16_t port, const char *cmd, void *lldpmib);
+
 #endif /* _PMD_I40E_H_ */
diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
index cccd576..1ffa02a 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -64,4 +64,10 @@ DPDK_18.02 {
 
 	rte_pmd_i40e_inset_get;
 	rte_pmd_i40e_inset_set;
-} DPDK_17.11;
\ No newline at end of file
+} DPDK_17.11;
+
+EXPERIMENTAL {
+	global:
+
+	rte_pmd_i40e_set_lldp_cmd;
+};
-- 
1.7.10.4

Re: [dpdk-dev] [PATCH v2] net/i40e: add a specific API to control the LLDP agent

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zijie Pan
> Sent: Thursday, May 24, 2018 9:33 AM
> To: dev@dpdk.org
> Cc: Xing, Beilei; Zhang, Qi Z; thomas@monjalon.net; Laurent Hardy
> Subject: [dpdk-dev] [PATCH v2] net/i40e: add a specific API to control the LLDP
> agent
> 
> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> It supports the following i40e debug lldp commands:
>     - start/stop of the LLDP agent.
>     - get local/remote of the LLDP MIB (Management Information Base).
> 
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Applied to dpdk-next-net-intel, thanks!

/Helin

Re: [dpdk-dev] [PATCH v2] net/i40e: add a specific API to control the LLDP agent

[Ferruh Yigit]

On 5/24/2018 2:32 AM, Zijie Pan wrote:
> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> It supports the following i40e debug lldp commands:
>     - start/stop of the LLDP agent.
>     - get local/remote of the LLDP MIB (Management Information Base).
> 
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Hi Zijie, Qi,

What do you think adding a testpmd command to use this API?

First it enables testing this API and second using API gives better chance to
API to survive from possible refactoring.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] net/i40e: add a specific API to control the LLDP agent

[Zijie Pan]

Thanks for the review.
I added a comment to explain why lldp is disabled in the code, and sent the v2 patch.

Thanks,
Zijie
 
 
------------------ Original ------------------
From:  "Zhang, Qi Z"<qi.z.zhang@intel.com>;
Date:  Tue, May 22, 2018 09:00 PM
To:  "Zijie Pan"<zijie.pan@6wind.com>; "dev@dpdk.org"<dev@dpdk.org>; 
Cc:  "Xing, Beilei"<beilei.xing@intel.com>; "Laurent Hardy"<laurent.hardy@6wind.com>; 
Subject:  RE: [PATCH] net/i40e: add a specific API to control the LLDP agent

 
> -----Original Message-----
> From: Zijie Pan [mailto:zijie.pan@6wind.com]
> Sent: Friday, May 18, 2018 6:14 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Laurent Hardy <laurent.hardy@6wind.com>
> Subject: [PATCH] net/i40e: add a specific API to control the LLDP agent
> 
> Add a new API rte_pmd_i40e_set_lldp_cmd to control LLDP agent for i40e.
> It supports the following i40e debug lldp commands:
>     - start/stop of the LLDP agent.
>     - get local/remote of the LLDP MIB (Management Information Base).
> 
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> Signed-off-by: Zijie Pan <zijie.pan@6wind.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>