From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 78D502946 for ; Mon, 19 Sep 2016 19:07:19 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP; 19 Sep 2016 10:07:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,362,1470726000"; d="scan'208";a="763370341" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.115]) ([10.237.220.115]) by FMSMGA003.fm.intel.com with ESMTP; 19 Sep 2016 10:07:16 -0700 From: Ferruh Yigit To: Xiao Wang , wenzhuo.lu@intel.com References: <1472312902-16963-1-git-send-email-xiao.w.wang@intel.com> <1472312902-16963-19-git-send-email-xiao.w.wang@intel.com> Cc: dev@dpdk.org Message-ID: <0b71b81e-a025-bbc4-ad9d-1a14c0962735@intel.com> Date: Mon, 19 Sep 2016 18:07:15 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1472312902-16963-19-git-send-email-xiao.w.wang@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Mon, 19 Sep 2016 17:07:19 -0000 On 8/27/2016 4:48 PM, Xiao Wang wrote: > Previously we checked only msgbuf[0] for "return buffer" instead of msgbuf[0] ? > (IXGBE_VF_SET_MACVLAN | IXGBE_VT_MSGTYPE_NACK), but this would not > work if index != 0 and as a result NACK will not be detected. "write buffer is not 0" instead of "index != 0" > 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. > 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 = &hw->mbx; > - u32 msgbuf[3]; > + u32 msgbuf[3], msgbuf_chk; > u8 *msg_addr = (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] |= index << IXGBE_VT_MSGINFO_SHIFT; > msgbuf[0] |= IXGBE_VF_SET_MACVLAN; > + msgbuf_chk = msgbuf[0]; > if (addr) > memcpy(msg_addr, addr, 6); > - ret_val = mbx->ops.write_posted(hw, msgbuf, 3, 0); > > - if (!ret_val) > - ret_val = mbx->ops.read_posted(hw, msgbuf, 3, 0); > + ret_val = ixgbevf_write_msg_read_ack(hw, msgbuf, msgbuf, 3); > + if (!ret_val) { > + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; > > - msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; > - > - if (!ret_val) > - if (msgbuf[0] == (IXGBE_VF_SET_MACVLAN | IXGBE_VT_MSGTYPE_NACK)) > - ret_val = IXGBE_ERR_OUT_OF_MEM; > + if (msgbuf[0] == (msgbuf_chk | IXGBE_VT_MSGTYPE_NACK)) > + return IXGBE_ERR_OUT_OF_MEM; What about following instead of introducing msgbuf_chk: if ((msgbuf[0] & IXGBE_VF_SET_MACVLAN) && (msgbuf[0] & IXGBE_VT_MSGTYPE_NACK)) Please check patch 15/39 > + } > > return ret_val; > } >