On 11/6/2020 2:20 PM, Bing Zhao wrote:
Hi Ferruh,
-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@intel.com>
Sent: Friday, November 6, 2020 7:35 PM
To: Bing Zhao <bi...@nvidia.com>; Slava Ovsiienko
<viachesl...@nvidia.com>; Matan Azrad <ma...@nvidia.com>
Cc: dev@dpdk.org; Ori Kam <or...@nvidia.com>; Raslan Darawsheh
<rasl...@nvidia.com>; sta...@dpdk.org
Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix eCPRI previous
layer checking
External email: Use caution opening links or attachments
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: sta...@dpdk.org
Signed-off-by: Bing Zhao <bi...@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viachesl...@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?
Yes, because when I am doing the fix. I found this log part is different from
others in the same file and just want to be consistent.
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.
Good point, in the past when I was searching some logs and I failed due to the
long log line break.
What do you think remove the whitespace changes out of this commit
and make another patch to merge the log strings?
Yes I can and will send v2 of this.
Or should I keep the log in a single line @Slava Ovsiienko, what do you think?
Any comments?
I remember in the past, my "checkpatch.pl" will report warning against this.
Could we ignore this?
As far as I know checkpatch is not complaining for the long lines of the string,
even it does I am OK to ignore it.
