[dpdk-dev] [PATCH 03/49] net/ice/base: add API to configure MIB
Maxime Coquelin
maxime.coquelin at redhat.com
Wed Jun 5 10:03:05 CEST 2019
On 6/5/19 2:00 AM, Stillwell Jr, Paul M wrote:
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin at redhat.com>
>> Sent: Tuesday, June 4, 2019 10:15 AM
>> To: Rong, Leyi <leyi.rong at intel.com>; Zhang, Qi Z <qi.z.zhang at intel.com>
>> Cc: dev at dpdk.org; Cao, Chinh T <chinh.t.cao at intel.com>; Ertman, David M
>> <david.m.ertman at intel.com>; Stillwell Jr, Paul M
>> <paul.m.stillwell.jr at 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 at intel.com>
>>> Signed-off-by: Dave Ertman <david.m.ertman at intel.com>
>>> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr at intel.com>
>>> Signed-off-by: Leyi Rong <leyi.rong at 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.
>>
>
More information about the dev
mailing list