From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 834DBA0096 for ; Wed, 5 Jun 2019 10:03:24 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 11F9B1B95D; Wed, 5 Jun 2019 10:03:24 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 947FC4F91 for ; Wed, 5 Jun 2019 10:03:22 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E8E08307D857; Wed, 5 Jun 2019 08:03:11 +0000 (UTC) Received: from [10.36.112.53] (ovpn-112-53.ams2.redhat.com [10.36.112.53]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 862D760A35; Wed, 5 Jun 2019 08:03:07 +0000 (UTC) To: "Stillwell Jr, Paul M" , "Rong, Leyi" , "Zhang, Qi Z" Cc: "dev@dpdk.org" , "Cao, Chinh T" , "Ertman, David M" References: <20190604054248.68510-1-leyi.rong@intel.com> <20190604054248.68510-4-leyi.rong@intel.com> <9f509ade-13b9-3952-39f9-82e5bbdf5013@redhat.com> From: Maxime Coquelin Message-ID: <9e164fb3-9dd2-b819-a6ed-bb232e82c1da@redhat.com> Date: Wed, 5 Jun 2019 10:03:05 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.48]); Wed, 05 Jun 2019 08:03:17 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 03/49] net/ice/base: add API to configure MIB X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 6/5/19 2:00 AM, Stillwell Jr, Paul M wrote: >> -----Original Message----- >> From: Maxime Coquelin >> Sent: Tuesday, June 4, 2019 10:15 AM >> To: Rong, Leyi ; Zhang, Qi Z >> Cc: dev@dpdk.org; Cao, Chinh T ; Ertman, David M >> ; Stillwell Jr, Paul M >> >> 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 >>> Signed-off-by: Dave Ertman >>> Signed-off-by: Paul M Stillwell Jr >>> Signed-off-by: Leyi Rong >>> --- >>> 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. >> >