Re: [dpdk-stable] [PATCH] net/mlx5: fix eCPRI previous layer checking

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 11/3/2020 5:42 AM, Bing Zhao wrote:
> Based on the specification, eCPRI can only follow ETH (VLAN) layer
> or UDP layer. When creating a flow with eCPRI item, this should be
> checked and invalid layout of the layers should be rejected.
> 
> Fixes: c7eca23657b7 ("net/mlx5: add flow validation of eCPRI header")
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bing Zhao <bingz@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
>   drivers/net/mlx5/mlx5_flow.c | 29 ++++++++++++++++++-----------
>   1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index a6e60af..11dba3b 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -2896,17 +2896,23 @@ struct mlx5_flow_tunnel_info {
>   					MLX5_FLOW_LAYER_OUTER_VLAN);
>   	struct rte_flow_item_ecpri mask_lo;
>   
> +	if (!(last_item & outer_l2_vlan) &&
> +	    last_item != MLX5_FLOW_LAYER_OUTER_L4_UDP)
> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_ITEM, item,
> +					  "eCPRI can only follow L2/VLAN layer"
> +					  " or UDP layer.");
>   	if ((last_item & outer_l2_vlan) && ether_type &&
>   	    ether_type != RTE_ETHER_TYPE_ECPRI)
>   		return rte_flow_error_set(error, EINVAL,
>   					  RTE_FLOW_ERROR_TYPE_ITEM, item,
> -					  "eCPRI cannot follow L2/VLAN layer "
> -					  "which ether type is not 0xAEFE.");
> +					  "eCPRI cannot follow L2/VLAN layer"
> +					  " which ether type is not 0xAEFE.");
>   	if (item_flags & MLX5_FLOW_LAYER_TUNNEL)
>   		return rte_flow_error_set(error, EINVAL,
>   					  RTE_FLOW_ERROR_TYPE_ITEM, item,
> -					  "eCPRI with tunnel is not supported "
> -					  "right now.");
> +					  "eCPRI with tunnel is not supported"
> +					  " right now.");

Why these changes done, it only moves space from end of first line to beginning 
of the second line?

Overall I think no need to break the log strings, keeping them intact helps 
users search the error message in the code.
I assume the break is because of the 80 chars limit but for log strings we don't 
have that limit, unless it is too long (lets say 120 chars as thumb of rule, 
there is no official convention) I think better to not break.

What do you think remove the whitespace changes out of this commit and make 
another patch to merge the log strings?