diff mbox series

[03/49] net/ice/base: add API to configure MIB

Message ID 20190604054248.68510-4-leyi.rong@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series shared code update |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Leyi Rong June 4, 2019, 5:42 a.m. UTC 
  Add ice_cfg_lldp_mib_change and treat DCBx state NOT_STARTED as valid.

Signed-off-by: Chinh T Cao <chinh.t.cao@intel.com>
Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
Signed-off-by: Leyi Rong <leyi.rong@intel.com>
---
 drivers/net/ice/base/ice_dcb.c | 41 +++++++++++++++++++++++++++++-----
 drivers/net/ice/base/ice_dcb.h |  3 ++-
 2 files changed, 38 insertions(+), 6 deletions(-)

Comments

Maxime Coquelin June 4, 2019, 5:14 p.m. UTC | #1 
On 6/4/19 7:42 AM, Leyi Rong wrote:
> Add ice_cfg_lldp_mib_change and treat DCBx state NOT_STARTED as valid.
> 
> Signed-off-by: Chinh T Cao <chinh.t.cao@intel.com>
> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> Signed-off-by: Leyi Rong <leyi.rong@intel.com>
> ---
>   drivers/net/ice/base/ice_dcb.c | 41 +++++++++++++++++++++++++++++-----
>   drivers/net/ice/base/ice_dcb.h |  3 ++-
>   2 files changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ice/base/ice_dcb.c b/drivers/net/ice/base/ice_dcb.c
> index a7810578d..100c4bb0f 100644
> --- a/drivers/net/ice/base/ice_dcb.c
> +++ b/drivers/net/ice/base/ice_dcb.c
> @@ -927,10 +927,11 @@ enum ice_status ice_get_dcb_cfg(struct ice_port_info *pi)
>   /**
>    * ice_init_dcb
>    * @hw: pointer to the HW struct
> + * @enable_mib_change: enable MIB change event
>    *
>    * Update DCB configuration from the Firmware
>    */
> -enum ice_status ice_init_dcb(struct ice_hw *hw)
> +enum ice_status ice_init_dcb(struct ice_hw *hw, bool enable_mib_change)
>   {
>   	struct ice_port_info *pi = hw->port_info;
>   	enum ice_status ret = ICE_SUCCESS;
> @@ -944,7 +945,8 @@ enum ice_status ice_init_dcb(struct ice_hw *hw)
>   	pi->dcbx_status = ice_get_dcbx_status(hw);
>   
>   	if (pi->dcbx_status == ICE_DCBX_STATUS_DONE ||
> -	    pi->dcbx_status == ICE_DCBX_STATUS_IN_PROGRESS) {
> +	    pi->dcbx_status == ICE_DCBX_STATUS_IN_PROGRESS ||
> +	    pi->dcbx_status == ICE_DCBX_STATUS_NOT_STARTED) {

Should this really be in this patch?
It does not seem related to the API addition.

>   		/* Get current DCBX configuration */
>   		ret = ice_get_dcb_cfg(pi);
>   		pi->is_sw_lldp = (hw->adminq.sq_last_status == ICE_AQ_RC_EPERM);
> @@ -952,13 +954,42 @@ enum ice_status ice_init_dcb(struct ice_hw *hw)
>   			return ret;
>   	} else if (pi->dcbx_status == ICE_DCBX_STATUS_DIS) {
>   		return ICE_ERR_NOT_READY;
> -	} else if (pi->dcbx_status == ICE_DCBX_STATUS_MULTIPLE_PEERS) {

Ditto.
Stillwell Jr, Paul M June 5, 2019, midnight UTC | #2 
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, June 4, 2019 10:15 AM
> To: Rong, Leyi <leyi.rong@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Cao, Chinh T <chinh.t.cao@intel.com>; Ertman, David M
> <david.m.ertman@intel.com>; Stillwell Jr, Paul M
> <paul.m.stillwell.jr@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 03/49] net/ice/base: add API to configure
> MIB
> 
> 
> 
> On 6/4/19 7:42 AM, Leyi Rong wrote:
> > Add ice_cfg_lldp_mib_change and treat DCBx state NOT_STARTED as valid.
> >
> > Signed-off-by: Chinh T Cao <chinh.t.cao@intel.com>
> > Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
> > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> > Signed-off-by: Leyi Rong <leyi.rong@intel.com>
> > ---
> >   drivers/net/ice/base/ice_dcb.c | 41
> +++++++++++++++++++++++++++++-----
> >   drivers/net/ice/base/ice_dcb.h |  3 ++-
> >   2 files changed, 38 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/ice/base/ice_dcb.c
> > b/drivers/net/ice/base/ice_dcb.c index a7810578d..100c4bb0f 100644
> > --- a/drivers/net/ice/base/ice_dcb.c
> > +++ b/drivers/net/ice/base/ice_dcb.c
> > @@ -927,10 +927,11 @@ enum ice_status ice_get_dcb_cfg(struct
> ice_port_info *pi)
> >   /**
> >    * ice_init_dcb
> >    * @hw: pointer to the HW struct
> > + * @enable_mib_change: enable MIB change event
> >    *
> >    * Update DCB configuration from the Firmware
> >    */
> > -enum ice_status ice_init_dcb(struct ice_hw *hw)
> > +enum ice_status ice_init_dcb(struct ice_hw *hw, bool
> > +enable_mib_change)
> >   {
> >   	struct ice_port_info *pi = hw->port_info;
> >   	enum ice_status ret = ICE_SUCCESS;
> > @@ -944,7 +945,8 @@ enum ice_status ice_init_dcb(struct ice_hw *hw)
> >   	pi->dcbx_status = ice_get_dcbx_status(hw);
> >
> >   	if (pi->dcbx_status == ICE_DCBX_STATUS_DONE ||
> > -	    pi->dcbx_status == ICE_DCBX_STATUS_IN_PROGRESS) {
> > +	    pi->dcbx_status == ICE_DCBX_STATUS_IN_PROGRESS ||
> > +	    pi->dcbx_status == ICE_DCBX_STATUS_NOT_STARTED) {
> 
> Should this really be in this patch?
> It does not seem related to the API addition.
> 

This seems ok since the commit message says that we changed the API and are treating dcbx_status in a different manor. Is the objection that we have 2 things in one commit?

> >   		/* Get current DCBX configuration */
> >   		ret = ice_get_dcb_cfg(pi);
> >   		pi->is_sw_lldp = (hw->adminq.sq_last_status ==
> ICE_AQ_RC_EPERM);
> > @@ -952,13 +954,42 @@ enum ice_status ice_init_dcb(struct ice_hw *hw)
> >   			return ret;
> >   	} else if (pi->dcbx_status == ICE_DCBX_STATUS_DIS) {
> >   		return ICE_ERR_NOT_READY;
> > -	} else if (pi->dcbx_status == ICE_DCBX_STATUS_MULTIPLE_PEERS) {
> 
> Ditto.
>
Maxime Coquelin June 5, 2019, 8:03 a.m. UTC | #3 
On 6/5/19 2:00 AM, Stillwell Jr, Paul M wrote:
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, June 4, 2019 10:15 AM
>> To: Rong, Leyi <leyi.rong@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
>> Cc: dev@dpdk.org; Cao, Chinh T <chinh.t.cao@intel.com>; Ertman, David M
>> <david.m.ertman@intel.com>; Stillwell Jr, Paul M
>> <paul.m.stillwell.jr@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH 03/49] net/ice/base: add API to configure
>> MIB
>>
>>
>>
>> On 6/4/19 7:42 AM, Leyi Rong wrote:
>>> Add ice_cfg_lldp_mib_change and treat DCBx state NOT_STARTED as valid.
>>>
>>> Signed-off-by: Chinh T Cao <chinh.t.cao@intel.com>
>>> Signed-off-by: Dave Ertman <david.m.ertman@intel.com>
>>> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
>>> Signed-off-by: Leyi Rong <leyi.rong@intel.com>
>>> ---
>>>    drivers/net/ice/base/ice_dcb.c | 41
>> +++++++++++++++++++++++++++++-----
>>>    drivers/net/ice/base/ice_dcb.h |  3 ++-
>>>    2 files changed, 38 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/ice/base/ice_dcb.c
>>> b/drivers/net/ice/base/ice_dcb.c index a7810578d..100c4bb0f 100644
>>> --- a/drivers/net/ice/base/ice_dcb.c
>>> +++ b/drivers/net/ice/base/ice_dcb.c
>>> @@ -927,10 +927,11 @@ enum ice_status ice_get_dcb_cfg(struct
>> ice_port_info *pi)
>>>    /**
>>>     * ice_init_dcb
>>>     * @hw: pointer to the HW struct
>>> + * @enable_mib_change: enable MIB change event
>>>     *
>>>     * Update DCB configuration from the Firmware
>>>     */
>>> -enum ice_status ice_init_dcb(struct ice_hw *hw)
>>> +enum ice_status ice_init_dcb(struct ice_hw *hw, bool
>>> +enable_mib_change)
>>>    {
>>>    	struct ice_port_info *pi = hw->port_info;
>>>    	enum ice_status ret = ICE_SUCCESS;
>>> @@ -944,7 +945,8 @@ enum ice_status ice_init_dcb(struct ice_hw *hw)
>>>    	pi->dcbx_status = ice_get_dcbx_status(hw);
>>>
>>>    	if (pi->dcbx_status == ICE_DCBX_STATUS_DONE ||
>>> -	    pi->dcbx_status == ICE_DCBX_STATUS_IN_PROGRESS) {
>>> +	    pi->dcbx_status == ICE_DCBX_STATUS_IN_PROGRESS ||
>>> +	    pi->dcbx_status == ICE_DCBX_STATUS_NOT_STARTED) {
>>
>> Should this really be in this patch?
>> It does not seem related to the API addition.
>>
> 
> This seems ok since the commit message says that we changed the API and are treating dcbx_status in a different manor. Is the objection that we have 2 things in one commit?

Well, it depends if DCBx NOT_STARTED becomes valid thanks to
ice_cfg_lldp_mib_change addition. It is not obvious by looking at the
commit message and the patch itself.

It this is not the case, then 2 commits are prefered, as one could
backport only the DCBx NOT_STARTED part.

> 
>>>    		/* Get current DCBX configuration */
>>>    		ret = ice_get_dcb_cfg(pi);
>>>    		pi->is_sw_lldp = (hw->adminq.sq_last_status ==
>> ICE_AQ_RC_EPERM);
>>> @@ -952,13 +954,42 @@ enum ice_status ice_init_dcb(struct ice_hw *hw)
>>>    			return ret;
>>>    	} else if (pi->dcbx_status == ICE_DCBX_STATUS_DIS) {
>>>    		return ICE_ERR_NOT_READY;
>>> -	} else if (pi->dcbx_status == ICE_DCBX_STATUS_MULTIPLE_PEERS) {
>>
>> Ditto.
>>
>
diff mbox series

Patch

diff --git a/drivers/net/ice/base/ice_dcb.c b/drivers/net/ice/base/ice_dcb.c
index a7810578d..100c4bb0f 100644
--- a/drivers/net/ice/base/ice_dcb.c
+++ b/drivers/net/ice/base/ice_dcb.c
@@ -927,10 +927,11 @@  enum ice_status ice_get_dcb_cfg(struct ice_port_info *pi)
 /**
  * ice_init_dcb
  * @hw: pointer to the HW struct
+ * @enable_mib_change: enable MIB change event
  *
  * Update DCB configuration from the Firmware
  */
-enum ice_status ice_init_dcb(struct ice_hw *hw)
+enum ice_status ice_init_dcb(struct ice_hw *hw, bool enable_mib_change)
 {
 	struct ice_port_info *pi = hw->port_info;
 	enum ice_status ret = ICE_SUCCESS;
@@ -944,7 +945,8 @@  enum ice_status ice_init_dcb(struct ice_hw *hw)
 	pi->dcbx_status = ice_get_dcbx_status(hw);
 
 	if (pi->dcbx_status == ICE_DCBX_STATUS_DONE ||
-	    pi->dcbx_status == ICE_DCBX_STATUS_IN_PROGRESS) {
+	    pi->dcbx_status == ICE_DCBX_STATUS_IN_PROGRESS ||
+	    pi->dcbx_status == ICE_DCBX_STATUS_NOT_STARTED) {
 		/* Get current DCBX configuration */
 		ret = ice_get_dcb_cfg(pi);
 		pi->is_sw_lldp = (hw->adminq.sq_last_status == ICE_AQ_RC_EPERM);
@@ -952,13 +954,42 @@  enum ice_status ice_init_dcb(struct ice_hw *hw)
 			return ret;
 	} else if (pi->dcbx_status == ICE_DCBX_STATUS_DIS) {
 		return ICE_ERR_NOT_READY;
-	} else if (pi->dcbx_status == ICE_DCBX_STATUS_MULTIPLE_PEERS) {
 	}
 
 	/* Configure the LLDP MIB change event */
-	ret = ice_aq_cfg_lldp_mib_change(hw, true, NULL);
+	if (enable_mib_change) {
+		ret = ice_aq_cfg_lldp_mib_change(hw, true, NULL);
+		if (!ret)
+			pi->is_sw_lldp = false;
+	}
+
+	return ret;
+}
+
+/**
+ * ice_cfg_lldp_mib_change
+ * @hw: pointer to the HW struct
+ * @ena_mib: enable/disable MIB change event
+ *
+ * Configure (disable/enable) MIB
+ */
+enum ice_status ice_cfg_lldp_mib_change(struct ice_hw *hw, bool ena_mib)
+{
+	struct ice_port_info *pi = hw->port_info;
+	enum ice_status ret;
+
+	if (!hw->func_caps.common_cap.dcb)
+		return ICE_ERR_NOT_SUPPORTED;
+
+	/* Get DCBX status */
+	pi->dcbx_status = ice_get_dcbx_status(hw);
+
+	if (pi->dcbx_status == ICE_DCBX_STATUS_DIS)
+		return ICE_ERR_NOT_READY;
+
+	ret = ice_aq_cfg_lldp_mib_change(hw, ena_mib, NULL);
 	if (!ret)
-		pi->is_sw_lldp = false;
+		pi->is_sw_lldp = !ena_mib;
 
 	return ret;
 }
diff --git a/drivers/net/ice/base/ice_dcb.h b/drivers/net/ice/base/ice_dcb.h
index d922c8a29..65d2bafef 100644
--- a/drivers/net/ice/base/ice_dcb.h
+++ b/drivers/net/ice/base/ice_dcb.h
@@ -197,7 +197,7 @@  ice_aq_get_dcb_cfg(struct ice_hw *hw, u8 mib_type, u8 bridgetype,
 		   struct ice_dcbx_cfg *dcbcfg);
 enum ice_status ice_get_dcb_cfg(struct ice_port_info *pi);
 enum ice_status ice_set_dcb_cfg(struct ice_port_info *pi);
-enum ice_status ice_init_dcb(struct ice_hw *hw);
+enum ice_status ice_init_dcb(struct ice_hw *hw, bool enable_mib_change);
 void ice_dcb_cfg_to_lldp(u8 *lldpmib, u16 *miblen, struct ice_dcbx_cfg *dcbcfg);
 enum ice_status
 ice_query_port_ets(struct ice_port_info *pi,
@@ -217,6 +217,7 @@  enum ice_status ice_aq_start_lldp(struct ice_hw *hw, struct ice_sq_cd *cd);
 enum ice_status
 ice_aq_start_stop_dcbx(struct ice_hw *hw, bool start_dcbx_agent,
 		       bool *dcbx_agent_status, struct ice_sq_cd *cd);
+enum ice_status ice_cfg_lldp_mib_change(struct ice_hw *hw, bool ena_mib);
 enum ice_status
 ice_aq_cfg_lldp_mib_change(struct ice_hw *hw, bool ena_update,
 			   struct ice_sq_cd *cd);