From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id D1147231E for ; Thu, 20 Jul 2017 11:56:19 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Jul 2017 02:56:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,383,1496127600"; d="scan'208";a="127269234" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.255.136.225]) ([10.255.136.225]) by orsmga005.jf.intel.com with ESMTP; 20 Jul 2017 02:56:14 -0700 To: Ajit Khaparde , dev@dpdk.org Cc: Stephen Hurd References: <20170720044826.44103-1-ajit.khaparde@broadcom.com> <20170720044826.44103-4-ajit.khaparde@broadcom.com> From: Ferruh Yigit Message-ID: <390250b1-9e64-c711-be4f-f1bf0e627a9c@intel.com> Date: Thu, 20 Jul 2017 10:56:13 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170720044826.44103-4-ajit.khaparde@broadcom.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 2/8] net/bnxt: fix to avoid a segfault 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: , X-List-Received-Date: Thu, 20 Jul 2017 09:56:20 -0000 On 7/20/2017 5:48 AM, Ajit Khaparde wrote: > Fix use of local variable to avoid segfault. > cnt was incorrectly tested and decremented in the loop that removes > a VLAN from the table. > > Fixes: 36735a932ca7 ("support set VF QOS and MAC anti spoof") > > Signed-off-by: Stephen Hurd > Signed-off-by: Ajit Khaparde > --- > drivers/net/bnxt/rte_pmd_bnxt.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/bnxt/rte_pmd_bnxt.c b/drivers/net/bnxt/rte_pmd_bnxt.c > index 0a8fb1e..0d48873 100644 > --- a/drivers/net/bnxt/rte_pmd_bnxt.c > +++ b/drivers/net/bnxt/rte_pmd_bnxt.c > @@ -500,6 +500,7 @@ int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port, uint16_t vlan, > continue; > } > > + /* cnt is one less than vlan_count */ > cnt = bp->pf.vf_info[i].vlan_count++; > /* > * And finally, add to the > @@ -511,19 +512,19 @@ int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port, uint16_t vlan, > ve->vid = rte_cpu_to_be_16(vlan); > } > } else { > - for (j = 0; cnt; j++) { > + for (j = 0; j < cnt; j++) { > if (rte_be_to_cpu_16( > - bp->pf.vf_info[i].vlan_table[j].vid) != > - vlan) > + bp->pf.vf_info[i].vlan_table[j].vid) != > + vlan) > continue; > memmove( > - &bp->pf.vf_info[i].vlan_table[j], > - &bp->pf.vf_info[i].vlan_table[j + 1], I may be missing something but if &bp->pf.vf_info[i].vlan_table[j + 1].vid == vlan, this may be endless loop. because each time vlan_table[j + 1] copied into vlan_table[j] and j--, in next iteration same thing will happen. > - getpagesize() - > - ((j + 1) * > + &bp->pf.vf_info[i].vlan_table[j], > + &bp->pf.vf_info[i].vlan_table[j + 1], I guess line realigned to fit into 80 characters limitation, one of the good thing with 80 chars limitation is it forces to reduce indention. It is up to you, but if you do following, it can be possible to reduce the indention one level and it will be easier to fit into limit: if.((vf_mask.&.1) == 0) continue; > + getpagesize() - > + ((j + 1) * > sizeof(struct bnxt_vlan_table_entry))); > j--; > - cnt = bp->pf.vf_info[i].vlan_count--; > + cnt = --bp->pf.vf_info[i].vlan_count; > } > } > rte_pmd_bnxt_set_vf_vlan_anti_spoof(dev->data->port_id, >