[dpdk-dev] [PATCH 2/8] net/bnxt: fix to avoid a segfault

[Ferruh Yigit]

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 <stephen.hurd at broadcom.com>
> Signed-off-by: Ajit Khaparde <ajit.khaparde at broadcom.com>
> ---
>  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,
>