From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3341EA04B5; Fri, 6 Nov 2020 18:43:34 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 10F2529C6; Fri, 6 Nov 2020 18:43:33 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 0AED623D; Fri, 6 Nov 2020 18:43:29 +0100 (CET) IronPort-SDR: 1hYr6tIuFjYSIw/LmjEMxmXfEvuQ793DJVgm2PeDehUhGpzpDF3kEcrR6rCv0V1/HBZ7MCiq8W e+mhEWeVjAqQ== X-IronPort-AV: E=McAfee;i="6000,8403,9797"; a="148856126" X-IronPort-AV: E=Sophos;i="5.77,457,1596524400"; d="scan'208";a="148856126" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Nov 2020 09:43:28 -0800 IronPort-SDR: HTHAXimYGJAMpMazf4vjCbpIDFalHMtt/zwTfV1o/Zn+8K0cI/KaPaqzMdNNIcDvR9yCwT2UlB Kt8dTNVqatEA== X-IronPort-AV: E=Sophos;i="5.77,457,1596524400"; d="scan'208";a="539922750" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.228.45]) ([10.213.228.45]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Nov 2020 09:43:27 -0800 To: Bing Zhao , Slava Ovsiienko , Matan Azrad Cc: "dev@dpdk.org" , Ori Kam , Raslan Darawsheh , "stable@dpdk.org" References: <1604382154-336373-1-git-send-email-bingz@nvidia.com> From: Ferruh Yigit Message-ID: <92bd14b5-a1a3-dc43-b797-08ed02e6da2a@intel.com> Date: Fri, 6 Nov 2020 17:43:25 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/mlx5: fix eCPRI previous layer checking X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 11/6/2020 2:20 PM, Bing Zhao wrote: > Hi Ferruh, > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Friday, November 6, 2020 7:35 PM >> To: Bing Zhao ; Slava Ovsiienko >> ; Matan Azrad >> Cc: dev@dpdk.org; Ori Kam ; Raslan Darawsheh >> ; stable@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: stable@dpdk.org >>> >>> Signed-off-by: Bing Zhao >>> Acked-by: Viacheslav Ovsiienko >>> --- >>> 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.