From: Stephen Hurd <stephen.hurd@broadcom.com>
To: Ajit Khaparde <ajit.khaparde@broadcom.com>
Cc: dev@dpdk.org, Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 2/8] net/bnxt: fix to avoid a segfault
Date: Fri, 21 Jul 2017 12:09:22 -0700 [thread overview]
Message-ID: <CAJ9nmBYOfBRajmrsQ8aT2Op-p=toNOc10FiqHXhx8ibioevoBw@mail.gmail.com> (raw)
In-Reply-To: <20170721032233.59657-4-ajit.khaparde@broadcom.com>
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 <ajit.khaparde@broadcom.com>
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 <stephen.hurd@broadcom.com>
> 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
next prev parent reply other threads:[~2017-07-21 19:09 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-20 4:48 [dpdk-dev] [PATCH 0/8] bnxt patchset Ajit Khaparde
2017-07-20 4:48 ` [dpdk-dev] [PATCH 1/8] net/bnxt: fix log levels for non error conditions Ajit Khaparde
2017-07-20 4:48 ` [dpdk-dev] [PATCH 2/8] net/bnxt: fix to avoid a segfault Ajit Khaparde
2017-07-20 9:56 ` Ferruh Yigit
2017-07-20 4:48 ` [dpdk-dev] [PATCH 3/8] net/bnxt: fix vnic cleanup Ajit Khaparde
2017-07-20 4:48 ` [dpdk-dev] [PATCH 4/8] net/bnxt: fix set link config Ajit Khaparde
2017-07-20 4:48 ` [dpdk-dev] [PATCH 5/8] net/bnxt: reset VF stats during initialization Ajit Khaparde
2017-07-20 4:48 ` [dpdk-dev] [PATCH 6/8] net/bnxt: fix VLAN antispoof configuration code Ajit Khaparde
2017-07-20 4:48 ` [dpdk-dev] [PATCH 7/8] net/bnxt: check invalid l2_filter_id Ajit Khaparde
2017-07-20 4:48 ` [dpdk-dev] [PATCH 8/8] net/bnxt: fix to free a filter before reusing it Ajit Khaparde
2017-07-20 10:02 ` [dpdk-dev] [PATCH 0/8] bnxt patchset Ferruh Yigit
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 " Ajit Khaparde
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 1/8] net/bnxt: fix log levels for non error conditions Ajit Khaparde
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 2/8] net/bnxt: fix to avoid a segfault Ajit Khaparde
2017-07-21 19:09 ` Stephen Hurd [this message]
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 3/8] net/bnxt: fix vnic cleanup Ajit Khaparde
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 4/8] net/bnxt: fix set link config Ajit Khaparde
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 5/8] net/bnxt: reset VF stats during initialization Ajit Khaparde
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 6/8] net/bnxt: fix VLAN antispoof configuration code Ajit Khaparde
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 7/8] net/bnxt: check invalid l2_filter_id Ajit Khaparde
2017-07-21 3:22 ` [dpdk-dev] [PATCH v2 8/8] net/bnxt: fix to free a filter before reusing it Ajit Khaparde
2017-07-21 9:31 ` [dpdk-dev] [PATCH v2 0/8] bnxt patchset Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJ9nmBYOfBRajmrsQ8aT2Op-p=toNOc10FiqHXhx8ibioevoBw@mail.gmail.com' \
--to=stephen.hurd@broadcom.com \
--cc=ajit.khaparde@broadcom.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).