[03/49] net/ice/base: add API to configure MIB
Checks
Commit Message
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
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.
> -----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.
>
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.
>>
>
@@ -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;
}
@@ -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);