Message-ID: <40a616a5-f350-2ac1-eda1-7e4c777ed487@nvidia.com>
Date:   Wed, 15 Feb 2023 10:54:12 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.2
Subject: Re: [PATCH net-next v1 2/3] net/mlx5e: Add helper for
 encap_info_equal for tunnels with options
Content-Language: en-US
To:     Alexander Lobakin <alexandr.lobakin@intel.com>
Cc:     davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, roopa@nvidia.com,
        eng.alaamohamedsoliman.am@gmail.com, bigeasy@linutronix.de,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Roi Dayan <roid@nvidia.com>, Maor Dickman <maord@nvidia.com>,
        Saeed Mahameed <saeedm@nvidia.com>
References: <20230214134137.225999-1-gavinl@nvidia.com>
 <20230214134137.225999-3-gavinl@nvidia.com>
 <611a9a70-0f6e-cba5-dcb3-3412e6e9956f@intel.com>
From:   Gavin Li <gavinl@nvidia.com>
In-Reply-To: <611a9a70-0f6e-cba5-dcb3-3412e6e9956f@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?d3JtSWRnVGs3MWhaaW5WUVNyQ2FEa09TRWVETkROempsTE5HcW13VjY2R1c5?=
 =?utf-8?B?VS8xRTArRFZ3OWI5UFlUdWxOaGt6UlJLMjN2ekhZbzd0QjJUUjVqbWc2czdZ?=
 =?utf-8?B?Q293RXlKNUZLMldSTjhJaml0UGNxMmdYSGpwUXVibjQ4QStMT2JUVlF2UmN4?=
 =?utf-8?B?SlRBbmFsekl4SHNOQjZWWXN3YWVnQU1MQlhDZlFRZ3d0VnBxRkxNS3ZkZW90?=
 =?utf-8?B?R1dvVk9hcTN6anprVWNRTmdTcFJBUDB6WXd3WEtHNFRPL3lCTlQ4aUhvM2Y1?=
 =?utf-8?B?bXAzVHZOWDFFN3dJZTJESDFNb2N2TDNVajNXUTBxbk5QNDVUTlh6NmVEM0du?=
 =?utf-8?B?QURCcTVsc2MzdlRzajd1cHVSemQ3WlpVM0dZMGVUME5Bdy8xeS9LOHQzTVQ3?=
 =?utf-8?B?TDVLV3RObGxwZytUTHlnRTlrWDYyRStIc2NyV1cvSS9ZM0x0U2RQYWozamVx?=
 =?utf-8?B?Y3MvcTkyOE5HbEd0ZUxRU2MvaDZZMXhGMTlEem5KQllsNnU0R216MWJBeWh0?=
 =?utf-8?B?OThQZnpQOXNKN3NXT2VEcFhuTXdxY2d4cDFJc0s2UG8rdmJ4NEN2ZUlMS0lW?=
 =?utf-8?B?RENuSUFDbVBVckU0Y1ZUYkFIZHZxd3YxYVl1NU5oQ1pBZjExWXM5RmFpM21r?=
 =?utf-8?B?UUs5dUtxcEFrRjJXb1Uwbit3eGNOK3NsakVEb2RoUHA2cW44VnFGRVlwWFNt?=
 =?utf-8?B?L1lFM0xYb0UvSFNWd1FHdklJRVhSa0xDejZsL2hoUUxxYkVYVjdWQ0U5YmxG?=
 =?utf-8?B?dS96WjVOTXoraVBBWTNBRVlUcFh3MHMxQi80Ty9xaktSditWQzczaGhjWTFp?=
 =?utf-8?B?M3ZiSVREZ3pWRkF6clZuSWNCL0VZaWkxQytJVHRRWnFkOFBsOFh4Q2wxMUh0?=
 =?utf-8?B?bk5kelFyay9ueEY3RGpJbE9FM28xNHBmZ0YzOGVVSmYyRlZET0s1REc1bHBM?=
 =?utf-8?B?QWVrcGFRemx1Ui90QU9MdlhPS0l0cDUyUkFBRkVlRVRMRDBsZDBEbDJmMTFj?=
 =?utf-8?B?eUROeW10MEt4WGV5NytVaCtXREpZMk1yRjRFS21tY1R5TzNRNTBPc0prY2ZN?=
 =?utf-8?B?Y1hMRzM3dE9QZXRZdENhamg4dmhGTTlicHZMOUUrSFBWcmJKOCsyOWtJY3dw?=
 =?utf-8?B?NGdTL0JDVjRpcjR1eFFpc2FRVHlsN0dOM1NOTlNGVDBMZGhzVTBta244azFq?=
 =?utf-8?B?TVlBdFFrOHhtSkNzN2UxRzlsc2x4eFpTTzVjU243TTJJOUlzdUl2eEV2OXdJ?=
 =?utf-8?B?QkY1WHU2RXpmOXI1ZmRiQU9kdk9vK25qU0tRNVZGemVrR3lWT0JNYlhEZGFL?=
 =?utf-8?B?MmxJSk1PQ09Nd2JTeHFqaWd6eTMvR0dGMjRFVk5idnh2cHVSTEJUeEk3aW1n?=
 =?utf-8?B?ZGg3ekV5NHlzWUtWNEJGTnRaRDdwQTZiN2FLVXE4MENYVHVzUHFwM1lHVUlx?=
 =?utf-8?B?ek5DUUlObnVLMGJBZUlsdCs5NzFiYUtNVUFOTXBIeG9jcDA1Ui9mQ1NhR092?=
 =?utf-8?B?UEpMM0Vha0lwU3owQjNVcG1EUTJBeGxNbjg0UWs0TDFIODczR2RMUmozYjNN?=
 =?utf-8?B?RHBheVZNUnFSeWVnTjhYR2FlSEhad0hRZjFoalRBQUxQTUljb0ViMDZiSG9D?=
 =?utf-8?B?anQyQ3RwR2x0NmNzd0h5clpMbWpjWHBhOUtwWWkyaHJFRDZHeVM0SWNkS1dW?=
 =?utf-8?B?SzNNZUdPRVI0UHN1RXdFL1NpTktCUEtpQmY4SThDSFREUDczNUNKOVBXeXRC?=
 =?utf-8?B?R0M5dWZDMWxaYXE5UHFUa2xFZVdqVFV2Zlh4Z2V6NmZEQlRWRVB5OG9mT1dV?=
 =?utf-8?B?NDQxc2syeXQ2NmFGVktxcHg5dy9YdjVQcys4TFdHOEdDaWdhL2pXRW1Qb1dp?=
 =?utf-8?B?UnZNZXIzbExqRUR3eGk2QXc1QVdiSEpIQ2RBTE1pbHpHMHNWc2E1OXFjYkE5?=
 =?utf-8?B?MlNzZnhJaER0eHdsNkdHQjlic1lLZnUwUDZlMGFSVGhHN21Ub2tVYTZYSGwz?=
 =?utf-8?B?bDVMZWovN0YxSUZSMW1ld09WUHJzT0RySlZPRWNWamtuZXlpTldZN1pIRGNU?=
 =?utf-8?B?N2g2Rko3NSs3aVhkeWJEakFsU3dMalZXNk9SQ3lNbkE0dzB1bHF4ZnkrbmEr?=
 =?utf-8?Q?SSbb1Tk0HnPv4RxnHsMerBMQu?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b8807680-717a-408b-f8e4-08db0efff566
X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB3840.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2023 02:54:30.3266
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: rzm5KZV6JnSVpN6fcHEjA9WNOTTB5DyghmC5/ipAqFERuh7PqcAO+OtMf0SG85RLCbfT0KR1l88Wz6GKI3rosQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB6489
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 2/14/2023 11:01 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Tue, 14 Feb 2023 15:41:36 +0200
>
> (dropping non-existent nikolay@nvidia.com)
>
>> For tunnels with options, eg, geneve and vxlan with gbp, they share the
>> same way to compare the headers and options. Extract the code as a common
>> function for them
>>
>> Change-Id: I3ea697293c8d5d66c0c20080dbde88f60bcbd62f
>> Signed-off-by: Gavin Li <gavinl@nvidia.com>
>> Reviewed-by: Roi Dayan <roid@nvidia.com>
>> Reviewed-by: Maor Dickman <maord@nvidia.com>
>> Acked-by: Saeed Mahameed <saeedm@nvidia.com>
> [...]
>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
>> index 780224fd67a1..4df9d27a63ad 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun_encap.c
>> @@ -571,6 +571,35 @@ bool mlx5e_tc_tun_encap_info_equal_generic(struct mlx5e_encap_key *a,
>>                a->tc_tunnel->tunnel_type == b->tc_tunnel->tunnel_type;
>>   }
>>
>> +bool mlx5e_tc_tun_encap_info_equal_options(struct mlx5e_encap_key *a,
>> +                                        struct mlx5e_encap_key *b,
>> +                                        __be16 tun_flags)
>> +{
>> +     struct ip_tunnel_info *a_info;
>> +     struct ip_tunnel_info *b_info;
>> +     bool a_has_opts, b_has_opts;
>> +
>> +     if (!mlx5e_tc_tun_encap_info_equal_generic(a, b))
>> +             return false;
>> +
>> +     a_has_opts = !!(a->ip_tun_key->tun_flags & tun_flags);
>> +     b_has_opts = !!(b->ip_tun_key->tun_flags & tun_flags);
>> +
>> +     /* keys are equal when both don't have any options attached */
>> +     if (!a_has_opts && !b_has_opts)
>> +             return true;
>> +
>> +     if (a_has_opts != b_has_opts)
>> +             return false;
>> +
>> +     /* options stored in memory next to ip_tunnel_info struct */
>> +     a_info = container_of(a->ip_tun_key, struct ip_tunnel_info, key);
>> +     b_info = container_of(b->ip_tun_key, struct ip_tunnel_info, key);
>> +
>> +     return a_info->options_len == b_info->options_len &&
>> +             memcmp(a_info + 1, b_info + 1, a_info->options_len) == 0;
> 1. memcmp() is not aligned to the first expr (off-by-one to the right).
Options start from "info + 1", see ip_tunnel_info_opts and will use it 
here to replace the "info+1".
> 2. `!expr` is preferred over `expr == 0`.
ACK
>
>> +}
>> +
>>   static int cmp_decap_info(struct mlx5e_decap_key *a,
>>                          struct mlx5e_decap_key *b)
>>   {
> [...]
>
> Thanks,
> Olek