Subject: Re: [patch net 2/2] mlxsw: spectrum_router: Correctly dump neighbour activity

From: Ido Schimmel <[email-masked]>
Date: Fri, 11 Nov 2016 14:54:05 +0200

On Fri, Nov 11, 2016 at 11:20:42AM +0100, Jiri Pirko wrote:
> From: Arkadi Sharshevsky <[email-masked]>
> 
> During neighbour activity check the device's table is dumped by multiple
> query requests. The query session should end when the response carries
> less records than requested or when a given record is not full. Current
> code only stops the dumping process if the number of returned records is
> zero, which can result in infinite loop in case of activity.
> 
> Fix this by stopping the dumping process according to the above logic.
> 
> Fixes: c723c735fa6b ("mlxsw: spectrum_router: Periodically update the kernel's neigh table")
> Signed-off-by: Arkadi Sharshevsky <[email-masked]>
> Signed-off-by: Ido Schimmel <[email-masked]>
> Signed-off-by: Jiri Pirko <[email-masked]>
> ---
>  .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 040737e..d437457 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -800,6 +800,26 @@ static void mlxsw_sp_router_neigh_rec_process(struct mlxsw_sp *mlxsw_sp,
>  	}
>  }
>  
> +static bool mlxsw_sp_router_rauhtd_is_full(char *rauhtd_pl)
> +{
> +	u8 num_rec, last_rec_index, num_entries;
> +
> +	num_rec = mlxsw_reg_rauhtd_num_rec_get(rauhtd_pl);
> +	last_rec_index = num_rec - 1;
> +
> +	if (num_rec < MLXSW_REG_RAUHTD_REC_MAX_NUM)
> +		return false;
> +	if (mlxsw_reg_rauhtd_rec_type_get(rauhtd_pl, last_rec_index) ==
> +	    MLXSW_REG_RAUHTD_TYPE_IPV6)
> +		return true;
> +
> +	num_entries = mlxsw_reg_rauhtd_ipv4_rec_num_entries_get(rauhtd_pl,
> +								last_rec_index);
> +	if (++num_entries ==  MLXSW_REG_RAUHTD_IPV4_ENT_PER_REC)

Jiri, I just noticed we have an extra space after the '=='. Can you
please remove it in v2? Sorry for not spotting this earlier.

> +		return true;
> +	return false;
> +}
> +
>  static int mlxsw_sp_router_neighs_update_rauhtd(struct mlxsw_sp *mlxsw_sp)
>  {
>  	char *rauhtd_pl;
> @@ -826,7 +846,7 @@ static int mlxsw_sp_router_neighs_update_rauhtd(struct mlxsw_sp *mlxsw_sp)
>  		for (i = 0; i < num_rec; i++)
>  			mlxsw_sp_router_neigh_rec_process(mlxsw_sp, rauhtd_pl,
>  							  i);
> -	} while (num_rec);
> +	} while (mlxsw_sp_router_rauhtd_is_full(rauhtd_pl));
>  	rtnl_unlock();
>  
>  	kfree(rauhtd_pl);
> -- 
> 2.7.4
>