Re: [PATCH v2 2/8] net/bnxt: fix to avoid a segfault
From: Stephen Hurd <hidden>
Date: 2017-07-21 19:09:23
So the only change was replacing a nesting level with a continue? Yeah, looks good. On Thu, Jul 20, 2017 at 8:22 PM, Ajit Khaparde [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com> -- v1->v2: incorporate review feedback. --- drivers/net/bnxt/rte_pmd_bnxt.c | 101 +++++++++++++++++++----------- ---------- 1 file changed, 49 insertions(+), 52 deletions(-)diff --git a/drivers/net/bnxt/rte_pmd_bnxt.c b/drivers/net/bnxt/rte_pmd_bnxt.c index 0a8fb1e..ec5855d 100644--- a/drivers/net/bnxt/rte_pmd_bnxt.c +++ b/drivers/net/bnxt/rte_pmd_bnxt.c@@ -473,62 +473,59 @@ int rte_pmd_bnxt_set_vf_vlan_filter(uint8_t port,uint16_t vlan, for (i = 0; vf_mask; i++, vf_mask >>= 1) { cnt = bp->pf.vf_info[i].vlan_count; - if (vf_mask & 1) { - if (bp->pf.vf_info[i].vlan_table == NULL) { - rc = -1; - continue; + if ((vf_mask & 1) == 0) + continue; + + if (bp->pf.vf_info[i].vlan_table == NULL) { + rc = -1; + continue; + } + if (vlan_on) { + /* First, search for a duplicate... */ + for (j = 0; j < cnt; j++) { + if (rte_be_to_cpu_16( + bp->pf.vf_info[i].vlan_table[j].vid) == vlan) + break; } - if (vlan_on) { - /* First, search for a duplicate... */ - for (j = 0; j < cnt; j++) { - if (rte_be_to_cpu_16( - bp->pf.vf_info[i].vlan_table[j].vid) == - vlan) - break; - } - if (j == cnt) { - /* Now check that there's space */ - if (cnt == getpagesize() / - sizeof(struct bnxt_vlan_table_entry)) { - RTE_LOG(ERR, PMD, - "VF %d VLAN table is full\n", - i); - RTE_LOG(ERR, PMD, - "cannot add VLAN %u\n", - vlan); - rc = -1; - continue; - } - - cnt = bp->pf.vf_info[i].vlan_count++; - /* - * And finally, add to the - * end of the table - */ - ve = &bp->pf.vf_info[i].vlan_table[ cnt]; - /* TODO: Hardcoded TPID */ - ve->tpid = rte_cpu_to_be_16(0x8100); - ve->vid = rte_cpu_to_be_16(vlan); - } - } else { - for (j = 0; cnt; j++) { - if (rte_be_to_cpu_16( - 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], - getpagesize() - - ((j + 1) * - sizeof(struct bnxt_vlan_table_entry))); - j--; - cnt = bp->pf.vf_info[i].vlan_count--; + if (j == cnt) { + /* Now check that there's space */ + if (cnt == getpagesize() / + sizeof(struct bnxt_vlan_table_entry)) { + RTE_LOG(ERR, PMD, + "VF %d VLAN table is full\n", + i); + RTE_LOG(ERR, PMD, + "cannot add VLAN %u\n", vlan); + rc = -1; + continue; } + + /* cnt is one less than vlan_count */ + cnt = bp->pf.vf_info[i].vlan_count++; + /* + * And finally, add to the + * end of the table + */ + ve = &bp->pf.vf_info[i].vlan_table[cnt]; + /* TODO: Hardcoded TPID */ + ve->tpid = rte_cpu_to_be_16(0x8100); + ve->vid = rte_cpu_to_be_16(vlan); + } + } else { + for (j = 0; j < cnt; j++) { + if (rte_be_to_cpu_16( + 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], + getpagesize() - ((j + 1) * + sizeof(struct bnxt_vlan_table_entry))); + j--; + cnt = --bp->pf.vf_info[i].vlan_count; } - rte_pmd_bnxt_set_vf_vlan_anti_ spoof(dev->data->port_id, - i, bp->pf.vf_info[i].vlan_spoof_ en); } + rte_pmd_bnxt_set_vf_vlan_anti_spoof(dev->data->port_id, i, + bp->pf.vf_info[i].vlan_spoof_en); } return rc; -- 2.10.1 (Apple Git-78)
-- Stephen Hurd 949-926-8039 stephen.hurd@broadcom.com