Message-ID: <ca729a48-35a1-ef05-59d3-ef1539003051@nvidia.com>
Date:   Fri, 17 Feb 2023 10:43:38 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP
 encap/decap flows offload
To:     Alexander Lobakin <aleksander.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-4-gavinl@nvidia.com>
 <711630a2-b810-f8b0-2dcf-1eb7056ecf1d@intel.com>
 <231a227d-dda6-fe15-e39a-68aee72a1d59@nvidia.com>
 <92d83584-2238-f8e8-3ed6-f292223e4061@nvidia.com>
 <dcc5578e-89d1-589f-3175-eb8bcd58f7ec@intel.com>
 <76b74086-ea65-7bb3-1eb0-391f79d1c615@nvidia.com>
 <aefe00f0-2a15-9a43-2451-6d01e74cc48a@intel.com>
Content-Language: en-US
From:   Gavin Li <gavinl@nvidia.com>
In-Reply-To: <aefe00f0-2a15-9a43-2451-6d01e74cc48a@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?VGNYNjVRcHk3MmdPcGFoYzl1KzU1Q2xOZGpWQ2hiQUZwajU5T3NjaWF0cE5k?=
 =?utf-8?B?NlJrM2NYcnhkU1hYR0Joa0xEbWVSUCtwbnhYVVJERHh1Nk1ma25VVWFwNjZr?=
 =?utf-8?B?TTliSGZ1WU1tYjQyN2xzYUh5OFlBWGwwV21rVEFzUjZsMzdlWGNWWGgrajlt?=
 =?utf-8?B?SUVOMHlqNW9kN3hOdjVQQUxNL0dxSjE5SUJUOTVydVpOWXRZRnV5SUk0cnUy?=
 =?utf-8?B?S2cva2VsTFRlcm9RK2luMFJKSHJ1S2wvUDE1NzJJSjh3dW94YjRKZnAyN0Nx?=
 =?utf-8?B?bklvS0RVcy9VQTJkbXRYamVvQ3JvUHU5Wi92THhSdkF4WElZTEhBbGlBOWZh?=
 =?utf-8?B?dEZRNFpIbkN6NGJXRDNGWWpYaWtqSVM2Ni9CTzRVVXlMbk93UjJPd2x0bXlQ?=
 =?utf-8?B?NEh0QXNFdkZ4UjJpMEpYS1Y5Y1ovM0NlL0ZKbTlxZVFvcnp2MVNFQlJVQyti?=
 =?utf-8?B?TmN2Vkl3MTJ4RnAvSGRoQi9VZzgzNHQ2dDczZjRnVkxibDBRdUNmQlpvV1c4?=
 =?utf-8?B?VExKc01VVjZ5UGY5UXhoZ2UwbW1hQVl5Q1pUVi9pTytyR2FDYTl5NXlzK3hJ?=
 =?utf-8?B?eE90UUorVS8rLzJvZHNmS0d4NlJJRkExYTIyV2JMSFF1WHdoTDZXTnVzV2Zx?=
 =?utf-8?B?eUVCVEhzVWgwRmtEV2UrOWJac2lSdDVqVVR4N25qc1FUTkxGZExQQjNBTGoy?=
 =?utf-8?B?TkhZcnhyNnF0YnlMMDBqK2FtNDQ4TFRZWUxFUWVuQmJ6MnlPamJySVpsdHVp?=
 =?utf-8?B?WHM4WFNMUTB0WWRtK0xtY00vWHMzNDErSklveEdFaGhpYVRvMVJXSmhIaVJ6?=
 =?utf-8?B?VXFsL1FaUXdyVkloL1Fhb2tLZlRESjBmZFJDbUxMdUd6SnBoSkRMU2RHbGRx?=
 =?utf-8?B?bENGbzV5Rms1S1VkYVNuNHVBZVFmdEp6bHVucXdoWkxZdmx1NHNVekdMUUly?=
 =?utf-8?B?Y0NzYkJTOGFNRnF3cVlzeldya3ljOGZ5b3pxQ3ZLeW9VcEFaeVIxWDRHWTZ3?=
 =?utf-8?B?ZzBqREF0UVFHbHRLWlRLUmNIN3NnZmozeG9hV3plVEJpUmNmZUhya1o1U254?=
 =?utf-8?B?UWRYYTV3Q0l3aGhyV1pXdU0wVjV2aTIxSjBZYmFwR1lrS0M2QTUveURxNWll?=
 =?utf-8?B?VjRRTGZEbzNUVHI4WEJFUkFSYXo1NHRaRTFPYktDa3ArSlJOZFcxdlNFL2Jm?=
 =?utf-8?B?RmpGaEtGVmJDaXJNY1NnNGloclIwWGcwSmQreVBNZDlSR3FOYndJOUhEbUpp?=
 =?utf-8?B?UGhYeWRSYmE2UkJrMkF3Smp5bm1tTFMrc09HOXAwZzdWVDlUb2FFWHVKM1d6?=
 =?utf-8?B?NDdRMFJQc1FHQ0tWWnVKSW84NlQ1SThGRS9MYThHODgvbGJWQU8zNHhZTmIx?=
 =?utf-8?B?NVVFa3d6OFhhZjFnZCtQZWNGc3hONC8wcno4WDNUUlVaT0p2aUJjblBPZHZi?=
 =?utf-8?B?K1N4RkJORkh1VnhrcjRsaHNjZ2hLbHhQdFRvS1NKZFBaZ1FOWjUzRERmRlhY?=
 =?utf-8?B?em1nUGxJdmEzcEgzME01d0pDanF3aS9EL3hsNnpZTHc3VE5mYXhqZlF4Y29T?=
 =?utf-8?B?THJ1UndrRUdTT0V3ZjJ5M09zWHZMVFI3L1lGQlhFUCtpMVdKcTlnOVlUY2Q3?=
 =?utf-8?B?THc2bzBkaTRMbERBZW01Z3VHNWpON0hzYXVrZnFYdVBNMkYvdkE5V2ZZbkRD?=
 =?utf-8?B?MjR6a21JR3g5R2wwcEhpZjB2Qktmb0ZlMGFjUHJKTS9TN0dpZTM0Vm1IMkRL?=
 =?utf-8?B?d0d2Um8yTUd5UTRPTUxFK0VtUFFEMStLYmR0K2dPSGhPQitySkxNazBkSkpJ?=
 =?utf-8?B?eDIzSWhmVDhEdHFwNjNkZTl3NXRjYTNRSGlHQ3IvZ3d3bk1yN2tKeVJ6RmlI?=
 =?utf-8?B?UDUyZGk0N0c4Uk9FQ3QvY2xUbFNpNEJ4eC9maklDb045dW5YZVQ0YjNMQS9S?=
 =?utf-8?B?Y1JZK0Rub1g4cmFhK1FIdjA5dmhiVGo0UzY3WEhxT25uNXR6WjB1YlJxdVMz?=
 =?utf-8?B?ZDJHbTY1VFp4c0JoaEc4WUVvQzBFamt4aURSWUdESkJCcnJYRDFYSG8zek5L?=
 =?utf-8?B?L1hMa1ROWi9MLzFYMFJVMTZOWWlic1FmcDloTHRRaDBjUWVQLzdrMEZKakhv?=
 =?utf-8?Q?fHTHtV1t4xMLjJtaS4QAgbeKV?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1e396b09-6cfe-4a1d-b171-08db1090d06a
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3835.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Feb 2023 02:43:56.5107
 (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: Os9Y96nAs3ASvxESB9wz34oUpwU66c3TKrqiZzFBgVAismLwadyL+5sVZaUVZf6MjlwWPHjFtwfEBU8dKDIMdg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4502
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 2/16/2023 11:19 PM, Alexander Lobakin wrote:
> External email: Use caution opening links or attachments
>
>
> From: Gavin Li <gavinl@nvidia.com>
> Date: Thu, 16 Feb 2023 16:40:33 +0800
>
>> On 2/16/2023 1:01 AM, Alexander Lobakin wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> From: Gavin Li <gavinl@nvidia.com>
>>> Date: Wed, 15 Feb 2023 16:30:04 +0800
>>>
>>>> On 2/15/2023 11:36 AM, Gavin Li wrote:
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> On 2/14/2023 11:26 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:37 +0200
>>> [...]
>>>
>>>>>>> @@ -96,6 +99,70 @@ static int mlx5e_gen_ip_tunnel_header_vxlan(char
>>>>>>> buf[],
>>>>>>>          udp->dest = tun_key->tp_dst;
>>>>>>>          vxh->vx_flags = VXLAN_HF_VNI;
>>>>>>>          vxh->vx_vni = vxlan_vni_field(tun_id);
>>>>>>> +     if (tun_key->tun_flags & TUNNEL_VXLAN_OPT) {
>>>>>>> +             md = ip_tunnel_info_opts((struct ip_tunnel_info
>>>>>>> *)e->tun_info);
>>>>>>> +             vxlan_build_gbp_hdr(vxh, tun_key->tun_flags,
>>>>>>> +                                 (struct vxlan_metadata *)md);
>>>>>> Maybe constify both ip_tunnel_info_opts() and vxlan_build_gbp_hdr()
>>>>>> arguments instead of working around by casting away?
>>>>> ACK. Sorry for the confusion---I misunderstood the comment.
>>>> This ip_tunnel_info_opts is tricky to use const to annotate the arg
>>>> because it will have to cast from const to non-const again upon
>>>> returning.
>>> It's okay to cast away for the `void *` returned.
>>> Alternatively, use can convert it to a macro and use
>>> __builtin_choose_expr() or _Generic to return const or non-const
>>> depending on whether the argument is constant. That's what was recently
>>> done for container_of() IIRC.
>> I've fixed vxlan_build_gbp_hdr in V2. For ip_tunnel_info_opts, it's
>> confusing to me.
>>
>> It would be as below after constifying the parameter.
>>
>> static inline void *ip_tunnel_info_opts(const struct ip_tunnel_info *info)
>> {
>>      return (void *)(info + 1);
>> }
>> Is there any value gained by this change?
> You wouldn't need to W/A it each time in each driver, just do it once in
> the inline itself.
> I did it once in __skb_header_pointer()[0] to be able to pass data
> pointer as const to optimize code a bit and point out explicitly that
> the function doesn't modify the packet anyhow, don't see any reason to
> not do the same here.
> Or, as I said, you can use macros + __builtin_choose_expr() or _Generic.
> container_of_const() uses the latter[1]. A __builtin_choose_expr()
> variant could rely on the __same_type() macro to check whether the
> pointer passed from the driver const or not.
ACK
>
>>>>>>> +     }
>>>>>>> +
>>>>>>> +     return 0;
>>>>>>> +}
>>> [...]
>>>
>>> Thanks,
>>> Olek
> [0]
> https://elixir.bootlin.com/linux/v6.2-rc8/source/include/linux/skbuff.h#L3992
> [1]
> https://elixir.bootlin.com/linux/v6.2-rc8/source/include/linux/container_of.h#L33
>
> Thanks,
> Olek