[Intel-wired-lan] [PATCH net-next 0/3] ice: Implement LLDP MIB Pending change

[Intel-wired-lan] [PATCH net-next 0/3] ice: Implement LLDP MIB Pending change

[Anatolii Gerasymenko]

If the RDMA traffic class is removed, then our Control QP (RDMA Admin QP)
may not receive completions and would hang RDMA processing. Which would
translate into requests for reset on the card after receiving such
changes from the switch.

To solve above problem FW will no longer remove TC nodes for the
applicable TC instead allowing RDMA to destroy QP markers. After RDMA
finish outstanding operations notification needs to be sent to FW to
finish DCB configuration change.

Anatolii Gerasymenko (2):
  ice: Get DCBX config from LLDP MIB change event
  ice: Handle LLDP MIB Pending change

Tsotne Chakhvadze (1):
  ice: Add 'Execute Pending LLDP MIB' Admin Queue command

 .../net/ethernet/intel/ice/ice_adminq_cmd.h   | 18 ++++++-
 drivers/net/ethernet/intel/ice/ice_common.c   | 13 +++++
 drivers/net/ethernet/intel/ice/ice_common.h   |  1 +
 drivers/net/ethernet/intel/ice/ice_dcb.c      | 39 ++++++++++++++
 drivers/net/ethernet/intel/ice/ice_dcb.h      |  2 +
 drivers/net/ethernet/intel/ice/ice_dcb_lib.c  | 53 +++++++++++--------
 6 files changed, 101 insertions(+), 25 deletions(-)

-- 
2.25.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next 1/3] ice: Add 'Execute Pending LLDP MIB' Admin Queue command

[Anatolii Gerasymenko]

From: Tsotne Chakhvadze <tsotne.chakhvadze@intel.com>

In DCB Willing Mode (FW managed LLDP), when the link partner changes
configuration which requires fewer TCs, the TCs that are no longer
needed are suspended by EMP FW, removed, and never resumed. This occurs
before a MIB change event is indicated to SW. The permanent suspension and
removal of these TC nodes in the scheduler prevents RDMA from being able
to destroy QPs associated with this TC, requiring a CORE reset to recover.

A new DCBX configuration change flow is defined to allow SW driver and
other SW components (RDMA) to properly adjust to the configuration
changes before they are taking effect in HW. This flow includes a
two-way handshake between EMP FW<->LAN SW<->RDMA SW.

List of changes:
- Add 'Execute Pending LLDP MIB' AQC.
- Add 'Pending Event Enable' bit.
- Add additional logic to ignore Pending Event Enable' request
  while 'LLDP MIB Chnage' event is disabled.
- Add 'Execute Pending LLDP MIB' AQC sending function to FW,
  which is needed to take place MIB Event change.

Signed-off-by: Tsotne Chakhvadze <tsotne.chakhvadze@intel.com>
Co-developed-by: Karen Sornek <karen.sornek@intel.com>
Signed-off-by: Karen Sornek <karen.sornek@intel.com>
Co-developed-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
---
 .../net/ethernet/intel/ice/ice_adminq_cmd.h    | 18 ++++++++++++++++--
 drivers/net/ethernet/intel/ice/ice_common.c    | 13 +++++++++++++
 drivers/net/ethernet/intel/ice/ice_common.h    |  1 +
 drivers/net/ethernet/intel/ice/ice_dcb.c       |  3 +++
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 9939238573a4..506f24f9277a 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1603,14 +1603,24 @@ struct ice_aqc_lldp_get_mib {
 #define ICE_AQ_LLDP_TX_ACTIVE			0
 #define ICE_AQ_LLDP_TX_SUSPENDED		1
 #define ICE_AQ_LLDP_TX_FLUSHED			3
+/* DCBX mode */
+#define ICE_AQ_LLDP_DCBX_M			GENMASK(7, 6)
+#define ICE_AQ_LLDP_DCBX_NA			0
+#define ICE_AQ_LLDP_DCBX_IEEE			1
+#define ICE_AQ_LLDP_DCBX_CEE			2
+
+	u8 state;
+#define ICE_AQ_LLDP_MIB_CHANGE_STATE_M		BIT(0)
+#define ICE_AQ_LLDP_MIB_CHANGE_EXECUTED		0
+#define ICE_AQ_LLDP_MIB_CHANGE_PENDING		1
+
 /* The following bytes are reserved for the Get LLDP MIB command (0x0A00)
  * and in the LLDP MIB Change Event (0x0A01). They are valid for the
  * Get LLDP MIB (0x0A00) response only.
  */
-	u8 reserved1;
 	__le16 local_len;
 	__le16 remote_len;
-	u8 reserved2[2];
+	u8 reserved[2];
 	__le32 addr_high;
 	__le32 addr_low;
 };
@@ -1621,6 +1631,9 @@ struct ice_aqc_lldp_set_mib_change {
 	u8 command;
 #define ICE_AQ_LLDP_MIB_UPDATE_ENABLE		0x0
 #define ICE_AQ_LLDP_MIB_UPDATE_DIS		0x1
+#define ICE_AQ_LLDP_MIB_PENDING_M		BIT(1)
+#define ICE_AQ_LLDP_MIB_PENDING_DISABLE		0
+#define ICE_AQ_LLDP_MIB_PENDING_ENABLE		1
 	u8 reserved[15];
 };
 
@@ -2269,6 +2282,7 @@ enum ice_adminq_opc {
 	ice_aqc_opc_lldp_set_local_mib			= 0x0A08,
 	ice_aqc_opc_lldp_stop_start_specific_agent	= 0x0A09,
 	ice_aqc_opc_lldp_filter_ctrl			= 0x0A0A,
+	ice_aqc_opc_lldp_execute_pending_mib		= 0x0A0B,
 
 	/* RSS commands */
 	ice_aqc_opc_set_rss_key				= 0x0B02,
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 27d0cbbd29da..30b428d27044 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -5217,6 +5217,19 @@ ice_lldp_fltr_add_remove(struct ice_hw *hw, u16 vsi_num, bool add)
 	return ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
 }
 
+/**
+ * ice_lldp_execute_pending_mib - execute LLDP pending MIB request
+ * @hw: pointer to HW struct
+ */
+int ice_lldp_execute_pending_mib(struct ice_hw *hw)
+{
+	struct ice_aq_desc desc;
+
+	ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_lldp_execute_pending_mib);
+
+	return ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
+}
+
 /**
  * ice_fw_supports_report_dflt_cfg
  * @hw: pointer to the hardware structure
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 61b7c60db689..1a8153edb751 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -210,6 +210,7 @@ ice_aq_set_lldp_mib(struct ice_hw *hw, u8 mib_type, void *buf, u16 buf_size,
 bool ice_fw_supports_lldp_fltr_ctrl(struct ice_hw *hw);
 int
 ice_lldp_fltr_add_remove(struct ice_hw *hw, u16 vsi_num, bool add);
+int ice_lldp_execute_pending_mib(struct ice_hw *hw);
 int
 ice_aq_read_i2c(struct ice_hw *hw, struct ice_aqc_link_topo_addr topo_addr,
 		u16 bus_addr, __le16 addr, u8 params, u8 *data,
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb.c b/drivers/net/ethernet/intel/ice/ice_dcb.c
index 0b146a0d4205..ca95f8753cee 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb.c
@@ -73,6 +73,9 @@ ice_aq_cfg_lldp_mib_change(struct ice_hw *hw, bool ena_update,
 
 	if (!ena_update)
 		cmd->command |= ICE_AQ_LLDP_MIB_UPDATE_DIS;
+	else
+		cmd->command |= FIELD_PREP(ICE_AQ_LLDP_MIB_PENDING_M,
+					   ICE_AQ_LLDP_MIB_PENDING_ENABLE);
 
 	return ice_aq_send_cmd(hw, &desc, NULL, 0, cd);
 }
-- 
2.25.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next 2/3] ice: Get DCBX config from LLDP MIB change event

[Anatolii Gerasymenko]

LLDP MIB Change Event (opcode 0x0A01) already contains MIB, which has
been changed. Add ice_dcb_process_lldp_set_mib_change() function, which
will set local/remote DCBX config from LLDP MIB Change Event's buffer.

This will save redundant Get LLDP MIB (opcode 0x0A00) AQ calls.

Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_dcb.c     | 36 ++++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_dcb.h     |  2 ++
 drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 30 +++++-----------
 3 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dcb.c b/drivers/net/ethernet/intel/ice/ice_dcb.c
index ca95f8753cee..f5d9533e61cc 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb.c
@@ -966,6 +966,42 @@ int ice_get_dcb_cfg(struct ice_port_info *pi)
 	return ret;
 }
 
+/**
+ * ice_get_dcb_cfg_from_mib_change
+ * @pi: port information structure
+ * @event: pointer to the admin queue receive event
+ *
+ * Set DCB configuration from received MIB Change event
+ */
+void ice_get_dcb_cfg_from_mib_change(struct ice_port_info *pi,
+				     struct ice_rq_event_info *event)
+{
+	struct ice_dcbx_cfg *dcbx_cfg = &pi->qos_cfg.local_dcbx_cfg;
+	struct ice_aqc_lldp_get_mib *mib;
+	u8 change_type, dcbx_mode;
+
+	mib = (struct ice_aqc_lldp_get_mib *)&event->desc.params.raw;
+
+	change_type = FIELD_GET(ICE_AQ_LLDP_MIB_TYPE_M,  mib->type);
+	if (change_type == ICE_AQ_LLDP_MIB_REMOTE)
+		dcbx_cfg = &pi->qos_cfg.remote_dcbx_cfg;
+
+	dcbx_mode = FIELD_GET(ICE_AQ_LLDP_DCBX_M, mib->type);
+
+	switch (dcbx_mode) {
+	case ICE_AQ_LLDP_DCBX_IEEE:
+		dcbx_cfg->dcbx_mode = ICE_DCBX_MODE_IEEE;
+		ice_lldp_to_dcb_cfg(event->msg_buf, dcbx_cfg);
+		break;
+
+	case ICE_AQ_LLDP_DCBX_CEE:
+		pi->qos_cfg.desired_dcbx_cfg = pi->qos_cfg.local_dcbx_cfg;
+		ice_cee_to_dcb_cfg((struct ice_aqc_get_cee_dcb_cfg_resp *)
+				   event->msg_buf, pi);
+		break;
+	}
+}
+
 /**
  * ice_init_dcb
  * @hw: pointer to the HW struct
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb.h b/drivers/net/ethernet/intel/ice/ice_dcb.h
index 6abf28a14291..be34650a77d5 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb.h
+++ b/drivers/net/ethernet/intel/ice/ice_dcb.h
@@ -144,6 +144,8 @@ ice_aq_get_dcb_cfg(struct ice_hw *hw, u8 mib_type, u8 bridgetype,
 		   struct ice_dcbx_cfg *dcbcfg);
 int ice_get_dcb_cfg(struct ice_port_info *pi);
 int ice_set_dcb_cfg(struct ice_port_info *pi);
+void ice_get_dcb_cfg_from_mib_change(struct ice_port_info *pi,
+				     struct ice_rq_event_info *event);
 int ice_init_dcb(struct ice_hw *hw, bool enable_mib_change);
 int
 ice_query_port_ets(struct ice_port_info *pi,
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index add90e75f05c..1c54eb643b6f 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -963,41 +963,27 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
 	pi = pf->hw.port_info;
 	mib = (struct ice_aqc_lldp_get_mib *)&event->desc.params.raw;
 	/* Ignore if event is not for Nearest Bridge */
-	mib_type = ((mib->type >> ICE_AQ_LLDP_BRID_TYPE_S) &
-		    ICE_AQ_LLDP_BRID_TYPE_M);
+	mib_type = FIELD_GET(ICE_AQ_LLDP_BRID_TYPE_M, mib->type);
 	dev_dbg(dev, "LLDP event MIB bridge type 0x%x\n", mib_type);
 	if (mib_type != ICE_AQ_LLDP_BRID_TYPE_NEAREST_BRID)
 		return;
 
 	/* Check MIB Type and return if event for Remote MIB update */
-	mib_type = mib->type & ICE_AQ_LLDP_MIB_TYPE_M;
+	mib_type = FIELD_GET(ICE_AQ_LLDP_MIB_TYPE_M, mib->type);
 	dev_dbg(dev, "LLDP event mib type %s\n", mib_type ? "remote" : "local");
 	if (mib_type == ICE_AQ_LLDP_MIB_REMOTE) {
 		/* Update the remote cached instance and return */
-		ret = ice_aq_get_dcb_cfg(pi->hw, ICE_AQ_LLDP_MIB_REMOTE,
-					 ICE_AQ_LLDP_BRID_TYPE_NEAREST_BRID,
-					 &pi->qos_cfg.remote_dcbx_cfg);
-		if (ret) {
-			dev_err(dev, "Failed to get remote DCB config\n");
-			return;
-		}
+		ice_get_dcb_cfg_from_mib_change(pi, event);
+		return;
 	}
 
 	mutex_lock(&pf->tc_mutex);
 
 	/* store the old configuration */
-	tmp_dcbx_cfg = pf->hw.port_info->qos_cfg.local_dcbx_cfg;
-
-	/* Reset the old DCBX configuration data */
-	memset(&pi->qos_cfg.local_dcbx_cfg, 0,
-	       sizeof(pi->qos_cfg.local_dcbx_cfg));
+	tmp_dcbx_cfg = pi->qos_cfg.local_dcbx_cfg;
 
-	/* Get updated DCBX data from firmware */
-	ret = ice_get_dcb_cfg(pf->hw.port_info);
-	if (ret) {
-		dev_err(dev, "Failed to get DCB config\n");
-		goto out;
-	}
+	/* Update DCBX data from MIB Change event's buffer */
+	ice_get_dcb_cfg_from_mib_change(pi, event);
 
 	/* No change detected in DCBX configs */
 	if (!memcmp(&tmp_dcbx_cfg, &pi->qos_cfg.local_dcbx_cfg,
@@ -1027,7 +1013,7 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
 	/* disable VSIs affected by DCB changes */
 	ice_dcb_ena_dis_vsi(pf, false, true);
 
-	ret = ice_query_port_ets(pf->hw.port_info, &buf, sizeof(buf), NULL);
+	ret = ice_query_port_ets(pi, &buf, sizeof(buf), NULL);
 	if (ret) {
 		dev_err(dev, "Query Port ETS failed\n");
 		goto unlock_rtnl;
-- 
2.25.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH net-next 3/3] ice: Handle LLDP MIB Pending change

[Anatolii Gerasymenko]

If the number of Traffic Classes (TC) is decreased, the FW will no
longer remove TC nodes, but will send a pending change notification. This
will allow RDMA to destroy corresponding Control QP markers. After RDMA
finishes outstanding operations, the ice driver will send an execute MIB
Pending change admin queue command to FW to finish DCB configuration
change.

The FW will buffer all incoming Pending changes, so there can be only
one active Pending change.

RDMA driver guarantees to remove Control QP markers within 5000 ms.
Hence, LLDP response timeout txTTL (default 30 sec) will be met.

Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 25 ++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
index 1c54eb643b6f..f16e4c54409f 100644
--- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
@@ -933,6 +933,16 @@ ice_tx_prepare_vlan_flags_dcb(struct ice_tx_ring *tx_ring,
 	}
 }
 
+/**
+ * ice_dcb_is_mib_change_pending - Check if MIB change is pending
+ * @state: MIB change state
+ */
+static inline bool ice_dcb_is_mib_change_pending(u8 state)
+{
+	return ICE_AQ_LLDP_MIB_CHANGE_PENDING ==
+		FIELD_GET(ICE_AQ_LLDP_MIB_CHANGE_STATE_M, state);
+}
+
 /**
  * ice_dcb_process_lldp_set_mib_change - Process MIB change
  * @pf: ptr to ice_pf
@@ -946,6 +956,7 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
 	struct device *dev = ice_pf_to_dev(pf);
 	struct ice_aqc_lldp_get_mib *mib;
 	struct ice_dcbx_cfg tmp_dcbx_cfg;
+	bool pending_handled = false;
 	bool need_reconfig = false;
 	struct ice_port_info *pi;
 	u8 mib_type;
@@ -966,7 +977,7 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
 	mib_type = FIELD_GET(ICE_AQ_LLDP_BRID_TYPE_M, mib->type);
 	dev_dbg(dev, "LLDP event MIB bridge type 0x%x\n", mib_type);
 	if (mib_type != ICE_AQ_LLDP_BRID_TYPE_NEAREST_BRID)
-		return;
+		goto handle_pending;
 
 	/* Check MIB Type and return if event for Remote MIB update */
 	mib_type = FIELD_GET(ICE_AQ_LLDP_MIB_TYPE_M, mib->type);
@@ -974,7 +985,7 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
 	if (mib_type == ICE_AQ_LLDP_MIB_REMOTE) {
 		/* Update the remote cached instance and return */
 		ice_get_dcb_cfg_from_mib_change(pi, event);
-		return;
+		goto handle_pending;
 	}
 
 	mutex_lock(&pf->tc_mutex);
@@ -1009,6 +1020,12 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
 		clear_bit(ICE_FLAG_DCB_ENA, pf->flags);
 	}
 
+	/* Send Execute Pending MIB Change event if it is a Pending event */
+	if (ice_dcb_is_mib_change_pending(mib->state)) {
+		ice_lldp_execute_pending_mib(&pf->hw);
+		pending_handled = true;
+	}
+
 	rtnl_lock();
 	/* disable VSIs affected by DCB changes */
 	ice_dcb_ena_dis_vsi(pf, false, true);
@@ -1028,4 +1045,8 @@ ice_dcb_process_lldp_set_mib_change(struct ice_pf *pf,
 	rtnl_unlock();
 out:
 	mutex_unlock(&pf->tc_mutex);
+handle_pending:
+	/* Send Execute Pending MIB Change event if it is a Pending event */
+	if (!pending_handled && ice_dcb_is_mib_change_pending(mib->state))
+		ice_lldp_execute_pending_mib(&pf->hw);
 }
-- 
2.25.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net-next 3/3] ice: Handle LLDP MIB Pending change

[Tony Nguyen]

On 7/28/2022 7:04 AM, Anatolii Gerasymenko wrote:
> If the number of Traffic Classes (TC) is decreased, the FW will no
> longer remove TC nodes, but will send a pending change notification. This
> will allow RDMA to destroy corresponding Control QP markers. After RDMA
> finishes outstanding operations, the ice driver will send an execute MIB
> Pending change admin queue command to FW to finish DCB configuration
> change.
> 
> The FW will buffer all incoming Pending changes, so there can be only
> one active Pending change.
> 
> RDMA driver guarantees to remove Control QP markers within 5000 ms.
> Hence, LLDP response timeout txTTL (default 30 sec) will be met.
> 
> Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 25 ++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> index 1c54eb643b6f..f16e4c54409f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
> @@ -933,6 +933,16 @@ ice_tx_prepare_vlan_flags_dcb(struct ice_tx_ring *tx_ring,
>   	}
>   }
>   
> +/**
> + * ice_dcb_is_mib_change_pending - Check if MIB change is pending
> + * @state: MIB change state
> + */
> +static inline bool ice_dcb_is_mib_change_pending(u8 state)

No 'inline' in .c files

> +{
> +	return ICE_AQ_LLDP_MIB_CHANGE_PENDING ==
> +		FIELD_GET(ICE_AQ_LLDP_MIB_CHANGE_STATE_M, state);
> +}
> +


_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net-next 3/3] ice: Handle LLDP MIB Pending change

[Anatolii Gerasymenko]

On 29.07.2022 20:55, Tony Nguyen wrote:
> 
> 
> On 7/28/2022 7:04 AM, Anatolii Gerasymenko wrote:
>> If the number of Traffic Classes (TC) is decreased, the FW will no
>> longer remove TC nodes, but will send a pending change notification. This
>> will allow RDMA to destroy corresponding Control QP markers. After RDMA
>> finishes outstanding operations, the ice driver will send an execute MIB
>> Pending change admin queue command to FW to finish DCB configuration
>> change.
>>
>> The FW will buffer all incoming Pending changes, so there can be only
>> one active Pending change.
>>
>> RDMA driver guarantees to remove Control QP markers within 5000 ms.
>> Hence, LLDP response timeout txTTL (default 30 sec) will be met.
>>
>> Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
>> ---
>>   drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 25 ++++++++++++++++++--
>>   1 file changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>> index 1c54eb643b6f..f16e4c54409f 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>> @@ -933,6 +933,16 @@ ice_tx_prepare_vlan_flags_dcb(struct ice_tx_ring *tx_ring,
>>       }
>>   }
>>   +/**
>> + * ice_dcb_is_mib_change_pending - Check if MIB change is pending
>> + * @state: MIB change state
>> + */
>> +static inline bool ice_dcb_is_mib_change_pending(u8 state)
> 
> No 'inline' in .c files
> 

Cannot find such rule in the coding style. Is this some kind of tacit agreement?
So, just remove inline hint and leave the function static? Or move it as static inline to *.h?

The function is only used in this *.c file and doesn't need to be exported.

>> +{
>> +    return ICE_AQ_LLDP_MIB_CHANGE_PENDING ==
>> +        FIELD_GET(ICE_AQ_LLDP_MIB_CHANGE_STATE_M, state);
>> +}
>> +
> 
> 
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net-next 3/3] ice: Handle LLDP MIB Pending change

[Tony Nguyen]

On 8/1/2022 1:06 AM, Anatolii Gerasymenko wrote:
> On 29.07.2022 20:55, Tony Nguyen wrote:
>>
>>
>> On 7/28/2022 7:04 AM, Anatolii Gerasymenko wrote:
>>> If the number of Traffic Classes (TC) is decreased, the FW will no
>>> longer remove TC nodes, but will send a pending change notification. This
>>> will allow RDMA to destroy corresponding Control QP markers. After RDMA
>>> finishes outstanding operations, the ice driver will send an execute MIB
>>> Pending change admin queue command to FW to finish DCB configuration
>>> change.
>>>
>>> The FW will buffer all incoming Pending changes, so there can be only
>>> one active Pending change.
>>>
>>> RDMA driver guarantees to remove Control QP markers within 5000 ms.
>>> Hence, LLDP response timeout txTTL (default 30 sec) will be met.
>>>
>>> Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
>>> ---
>>>    drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 25 ++++++++++++++++++--
>>>    1 file changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>> index 1c54eb643b6f..f16e4c54409f 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>> @@ -933,6 +933,16 @@ ice_tx_prepare_vlan_flags_dcb(struct ice_tx_ring *tx_ring,
>>>        }
>>>    }
>>>    +/**
>>> + * ice_dcb_is_mib_change_pending - Check if MIB change is pending
>>> + * @state: MIB change state
>>> + */
>>> +static inline bool ice_dcb_is_mib_change_pending(u8 state)
>>
>> No 'inline' in .c files
>>
> 
> Cannot find such rule in the coding style. Is this some kind of tacit agreement?

It's a requirement of netdev. If you search archive history, you can 
find plenty of comments on it (a couple of examples [1][2]). It's also 
something specifically checked for on netdev CI [3]; note the 
'netdev/source_inline' check.

> So, just remove inline hint and leave the function static? Or move it as static inline to *.h?

Just remove the inline hint and leave the function static.

> The function is only used in this *.c file and doesn't need to be exported.
> 
>>> +{
>>> +    return ICE_AQ_LLDP_MIB_CHANGE_PENDING ==
>>> +        FIELD_GET(ICE_AQ_LLDP_MIB_CHANGE_STATE_M, state);
>>> +}
>>> +
>>
>>

[1] 
https://lore.kernel.org/netdev/20200731.142538.868196979893920242.davem@davemloft.net/
[2] 
https://lore.kernel.org/netdev/20201011122422.56beacaa@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/
[3] 
https://patchwork.kernel.org/project/netdevbpf/patch/20220517073259.23476-2-harini.katakam@xilinx.com/
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH net-next 3/3] ice: Handle LLDP MIB Pending change

[Anatolii Gerasymenko]

On 01.08.2022 17:52, Tony Nguyen wrote:
> 
> 
> On 8/1/2022 1:06 AM, Anatolii Gerasymenko wrote:
>> On 29.07.2022 20:55, Tony Nguyen wrote:
>>>
>>>
>>> On 7/28/2022 7:04 AM, Anatolii Gerasymenko wrote:
>>>> If the number of Traffic Classes (TC) is decreased, the FW will no
>>>> longer remove TC nodes, but will send a pending change notification. This
>>>> will allow RDMA to destroy corresponding Control QP markers. After RDMA
>>>> finishes outstanding operations, the ice driver will send an execute MIB
>>>> Pending change admin queue command to FW to finish DCB configuration
>>>> change.
>>>>
>>>> The FW will buffer all incoming Pending changes, so there can be only
>>>> one active Pending change.
>>>>
>>>> RDMA driver guarantees to remove Control QP markers within 5000 ms.
>>>> Hence, LLDP response timeout txTTL (default 30 sec) will be met.
>>>>
>>>> Signed-off-by: Anatolii Gerasymenko <anatolii.gerasymenko@intel.com>
>>>> ---
>>>>    drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 25 ++++++++++++++++++--
>>>>    1 file changed, 23 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>>> index 1c54eb643b6f..f16e4c54409f 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c
>>>> @@ -933,6 +933,16 @@ ice_tx_prepare_vlan_flags_dcb(struct ice_tx_ring *tx_ring,
>>>>        }
>>>>    }
>>>>    +/**
>>>> + * ice_dcb_is_mib_change_pending - Check if MIB change is pending
>>>> + * @state: MIB change state
>>>> + */
>>>> +static inline bool ice_dcb_is_mib_change_pending(u8 state)
>>>
>>> No 'inline' in .c files
>>>
>>
>> Cannot find such rule in the coding style. Is this some kind of tacit agreement?
> 
> It's a requirement of netdev. If you search archive history, you can find plenty of comments on it (a couple of examples [1][2]). It's also something specifically checked for on netdev CI [3]; note the 'netdev/source_inline' check.
> 

Thank you for the info, Tony!

>> So, just remove inline hint and leave the function static? Or move it as static inline to *.h?
> 
> Just remove the inline hint and leave the function static.
> 

Fixed in v2.

>> The function is only used in this *.c file and doesn't need to be exported.
>>
>>>> +{
>>>> +    return ICE_AQ_LLDP_MIB_CHANGE_PENDING ==
>>>> +        FIELD_GET(ICE_AQ_LLDP_MIB_CHANGE_STATE_M, state);
>>>> +}
>>>> +
>>>
>>>
> 
> [1] https://lore.kernel.org/netdev/20200731.142538.868196979893920242.davem@davemloft.net/
> [2] https://lore.kernel.org/netdev/20201011122422.56beacaa@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/
> [3] https://patchwork.kernel.org/project/netdevbpf/patch/20220517073259.23476-2-harini.katakam@xilinx.com/
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan