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 1DFBB2C15 for ; Thu, 21 Apr 2016 15:51:38 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP; 21 Apr 2016 06:51:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,512,1455004800"; d="scan'208";a="959824129" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.220.132]) by orsmga002.jf.intel.com with SMTP; 21 Apr 2016 06:51:35 -0700 Received: by (sSMTP sendmail emulation); Thu, 21 Apr 2016 14:51:35 +0025 Date: Thu, 21 Apr 2016 14:51:35 +0100 From: Bruce Richardson To: Tomasz Kulasek Cc: dev@dpdk.org, helin.zhang@intel.com, konstantin.ananyev@intel.com Message-ID: <20160421135134.GA15304@bricha3-MOBL3> References: <1460727549-4380-1-git-send-email-tomaszx.kulasek@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1460727549-4380-1-git-send-email-tomaszx.kulasek@intel.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] ixgbe: fix bad shift operation in ixgbe_set_pool_rx 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: Thu, 21 Apr 2016 13:51:39 -0000 On Fri, Apr 15, 2016 at 03:39:09PM +0200, Tomasz Kulasek wrote: > CID 13193 (#1 of 1): Bad bit shift operation (BAD_SHIFT) > large_shift: In expression 1 << pool, left shifting by more than 31 bits > has undefined behavior. The shift amount, pool, is at least 32. > > This patch limits mask shift to be in range of 32 bit PFVFRE[1] register, > for pool > 31. > > Fixes: fe3a45fd4104 ("ixgbe: add VMDq support") > > Signed-off-by: Tomasz Kulasek > --- > drivers/net/ixgbe/ixgbe_ethdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index 3f1ebc1..f676a64 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -4401,7 +4401,7 @@ ixgbe_set_pool_rx(struct rte_eth_dev *dev, uint16_t pool, uint8_t on) > > addr = IXGBE_VFRE(pool >= ETH_64_POOLS/2); > reg = IXGBE_READ_REG(hw, addr); > - val = bit1 << pool; > + val = bit1 << (pool & 0x01F); > Are we sure this is the correct way to fix this. Rather than silently truncating the pool value, are we not better to check our input parameters and return EINVAL to the caller if pool overflows? /Bruce