From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 15FA95939 for ; Fri, 23 Sep 2016 08:05:59 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga105.fm.intel.com with ESMTP; 22 Sep 2016 23:05:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,380,1470726000"; d="scan'208";a="12860002" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga004.jf.intel.com with ESMTP; 22 Sep 2016 23:05:58 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 22 Sep 2016 23:05:58 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.118]) by shsmsx102.ccr.corp.intel.com ([169.254.2.15]) with mapi id 14.03.0248.002; Fri, 23 Sep 2016 14:05:56 +0800 From: "Wang, Xiao W" To: "Yigit, Ferruh" , "Lu, Wenzhuo" CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 18/39] net/ixgbe/base: fix check on NACK Thread-Index: AQHSAHqSPcgAytktKEqH/23v9JMpX6CAqXiAgAYO9MA= Date: Fri, 23 Sep 2016 06:05:55 +0000 Message-ID: References: <1472312902-16963-1-git-send-email-xiao.w.wang@intel.com> <1472312902-16963-19-git-send-email-xiao.w.wang@intel.com> <0b71b81e-a025-bbc4-ad9d-1a14c0962735@intel.com> In-Reply-To: <0b71b81e-a025-bbc4-ad9d-1a14c0962735@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZDYzNjdmYWEtZjliNi00M2VkLWE2MGUtM2FlZjc1OWI5NjhiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImVaVlVhYVVQazRiZ1JaZHRKYmU5U2h0UCtnSnRodVVzSzgwUU5hYlh2UTQ9In0= x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 18/39] net/ixgbe/base: fix check on NACK X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Sep 2016 06:06:00 -0000 Hi Ferruh, > -----Original Message----- > From: Yigit, Ferruh > Sent: Tuesday, September 20, 2016 1:07 AM > To: Wang, Xiao W ; Lu, Wenzhuo > > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 18/39] net/ixgbe/base: fix check on NACK >=20 > On 8/27/2016 4:48 PM, Xiao Wang wrote: > > Previously we checked only msgbuf[0] for >=20 > "return buffer" instead of msgbuf[0] ? >=20 Looks better. Use it in v2. > > (IXGBE_VF_SET_MACVLAN | IXGBE_VT_MSGTYPE_NACK), but this would not > > work if index !=3D 0 and as a result NACK will not be detected. >=20 > "write buffer is not 0" instead of "index !=3D 0" >=20 msgbuf[0] |=3D index << IXGBE_VT_MSGINFO_SHIFT; msgbuf[0] |=3D IXGBE_VF_SET_MACVLAN; "index !=3D 0" has effect on the msgbuf[0], so we should emphasize on "inde= x". I will change it to "index is not 0" in v2. > > >=20 > Function also starts using ixgbevf_write_msg_read_ack() instead of > separate write and read, is it possible to fix NACK only in this patch, > and do ixgbevf_write_msg_read_ack() switch in patch 27/39. > If prefer to keep in this patch, please mention about this switch in > comment log. >=20 Agree. Thanks. > > Fixes: af75078fece3 ("first public release") > > > > Signed-off-by: Xiao Wang > > --- > > drivers/net/ixgbe/base/ixgbe_vf.c | 18 ++++++++---------- > > 1 file changed, 8 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_vf.c > b/drivers/net/ixgbe/base/ixgbe_vf.c > > index c0fedea..f60ff7d 100644 > > --- a/drivers/net/ixgbe/base/ixgbe_vf.c > > +++ b/drivers/net/ixgbe/base/ixgbe_vf.c > > @@ -529,8 +529,7 @@ s32 ixgbe_get_mac_addr_vf(struct ixgbe_hw *hw, u8 > *mac_addr) > > > > s32 ixgbevf_set_uc_addr_vf(struct ixgbe_hw *hw, u32 index, u8 *addr) > > { > > - struct ixgbe_mbx_info *mbx =3D &hw->mbx; > > - u32 msgbuf[3]; > > + u32 msgbuf[3], msgbuf_chk; > > u8 *msg_addr =3D (u8 *)(&msgbuf[1]); > > s32 ret_val; > > > > @@ -543,18 +542,17 @@ s32 ixgbevf_set_uc_addr_vf(struct ixgbe_hw *hw, > u32 index, u8 *addr) > > */ > > msgbuf[0] |=3D index << IXGBE_VT_MSGINFO_SHIFT; > > msgbuf[0] |=3D IXGBE_VF_SET_MACVLAN; > > + msgbuf_chk =3D msgbuf[0]; > > if (addr) > > memcpy(msg_addr, addr, 6); > > - ret_val =3D mbx->ops.write_posted(hw, msgbuf, 3, 0); > > > > - if (!ret_val) > > - ret_val =3D mbx->ops.read_posted(hw, msgbuf, 3, 0); > > + ret_val =3D ixgbevf_write_msg_read_ack(hw, msgbuf, msgbuf, 3); > > + if (!ret_val) { > > + msgbuf[0] &=3D ~IXGBE_VT_MSGTYPE_CTS; > > > > - msgbuf[0] &=3D ~IXGBE_VT_MSGTYPE_CTS; > > - > > - if (!ret_val) > > - if (msgbuf[0] =3D=3D (IXGBE_VF_SET_MACVLAN | > IXGBE_VT_MSGTYPE_NACK)) > > - ret_val =3D IXGBE_ERR_OUT_OF_MEM; > > + if (msgbuf[0] =3D=3D (msgbuf_chk | IXGBE_VT_MSGTYPE_NACK)) > > + return IXGBE_ERR_OUT_OF_MEM; >=20 > What about following instead of introducing msgbuf_chk: >=20 > if ((msgbuf[0] & IXGBE_VF_SET_MACVLAN) && > (msgbuf[0] & IXGBE_VT_MSGTYPE_NACK)) >=20 > Please check patch 15/39 It's different from 15/39 where the write buffer is simple, Here the write buffer msgbuf[0] is more complicated, if we don't introduce msgbuf_chk, the code will looks bloated.=20 >=20 > > + } > > > > return ret_val; > > } > > >=20 >=20