From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id DE05FA04B5
for <public@inbox.dpdk.org>; Fri, 6 Nov 2020 18:43:33 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id D42C923D;
Fri, 6 Nov 2020 18:43:32 +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 <bingz@nvidia.com>, Slava Ovsiienko <viacheslavo@nvidia.com>,
Matan Azrad <matan@nvidia.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Ori Kam <orika@nvidia.com>,
Raslan Darawsheh <rasland@nvidia.com>, "stable@dpdk.org" <stable@dpdk.org>
References: <1604382154-336373-1-git-send-email-bingz@nvidia.com>
<ed6af423-88a1-a6bd-21f2-13aa893317b3@intel.com>
<CY4PR1201MB0072EF7DAE1B24652B116FF0D0ED0@CY4PR1201MB0072.namprd12.prod.outlook.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
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: <CY4PR1201MB0072EF7DAE1B24652B116FF0D0ED0@CY4PR1201MB0072.namprd12.prod.outlook.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-stable] [PATCH] net/mlx5: fix eCPRI previous layer
checking
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>
On 11/6/2020 2:20 PM, Bing Zhao wrote:
> Hi Ferruh,
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Friday, November 6, 2020 7:35 PM
>> To: Bing Zhao <bingz@nvidia.com>; Slava Ovsiienko
>> <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>
>> Cc: dev@dpdk.org; Ori Kam <orika@nvidia.com>; Raslan Darawsheh
>> <rasland@nvidia.com>; 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 <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?
>
> 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.