Message-ID: <76b74086-ea65-7bb3-1eb0-391f79d1c615@nvidia.com>
Date:   Thu, 16 Feb 2023 16:40:33 +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 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>
Content-Language: en-US
From:   Gavin Li <gavinl@nvidia.com>
In-Reply-To: <dcc5578e-89d1-589f-3175-eb8bcd58f7ec@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MjRBQWNSOWc3UXZabm5BUHRJY0tCQWdmcWxzYmZhQ3RkWUpId3FlWDlYdWYx?=
 =?utf-8?B?WE5qR29Dc2NTcWV3M0FuMTJUL0tLYVo4ampwM2NyOHZmWWlEWkk0UE1xQ1VC?=
 =?utf-8?B?MHEzNVNqMVBtNVJGQVdxemtmUlowZ0cyT3hhK3BydCtCYWpUUTFHYlVzY0xG?=
 =?utf-8?B?MHBNQmNTUWRRSC8vME8wSTdqRnExNmdTVDVDY3ZIS3JtaXJOd2NLNm5vU29G?=
 =?utf-8?B?ZXNidTJRaFFEbUdlMjlQVElNUGxvd3lzYjh5dGFXeGY4Yk8zMzc2bG5ialVL?=
 =?utf-8?B?d3pka1J5cmNZdzdYYVprMDZtUVVjcDVjRCtoZnJkMFEyQm1oS242dUp0UzhS?=
 =?utf-8?B?NVB6RmY2czVnZlJIL0hrUW9WS0Rka0NMVzZZNHdoNnRneFliQUQwbU9IV0g4?=
 =?utf-8?B?SzZPc0s5RkRpNVNZLysybFp3REt1K3d5ZDQrSkV1cHhIekwwU0pvUnY2amdt?=
 =?utf-8?B?dCtrQXYwbk5wOVhmRy9mOGttZXV5bmtpUFhDeTg4VmRnQjlPRUZuQnpNS2pE?=
 =?utf-8?B?dzA1REk0ZzJBR1RiV2h6UHdjRlZOTWVMV0ZLakgzdGFSMEZBbGpyd2E0OXFt?=
 =?utf-8?B?WEtHZm0rM3VWbDhmYWJRMHNCVmNhT0RWbVk5NDd4SldxRGVuNHRocHRJZDNK?=
 =?utf-8?B?Y1FzZGFDTGR2T1JCNkNJNmUvUDlOMXhkUjkwaHRPSjFVcnhlcGxGd3AzeWRS?=
 =?utf-8?B?dzRKN3Z2VVBrbjZtVzVWZEh6R1pPRHBpampZakFwMGZMT1QwamlFVG56R1JP?=
 =?utf-8?B?UXZpcGYxM0c0SDRvVGVybVVuSExnSXk3RUJrN0luUmxWbnJNOVk2dUhzc1hV?=
 =?utf-8?B?d1lSRWl2dVdmaDRxNjY5eEhaV1czM0U1NHpSK3NmclAwQlNsV2RxOVVxNkQr?=
 =?utf-8?B?YjZoS1VpQ3ZkMkxYd1pPUkhoMUh1K24yMUdvN3ptSWJka3ZCaGpYZktaY0V2?=
 =?utf-8?B?TUNEbFdNbkU0QlpJTnNVZ0pzWCs2N09JZkFlajg3dWVaZXYxRm9ESjFNZ2pN?=
 =?utf-8?B?YmhHNlZ4TnNwSUZqd21SUGFkUzRwaDQ2ZUFHZVVBblJqZkZvZDczTUlBUDhh?=
 =?utf-8?B?QzRjbjRNWjArNndPVlVUT2dNdlFpTjlFOEMrem9FOVRyUzJGMEE5eHdBZDhh?=
 =?utf-8?B?MHZGV1dxUm5EeFQ5eXVsUENWQ1BvZW9VZmtJVmxyZjRpck0yT2NuR3RSK05D?=
 =?utf-8?B?TFc1dHNOL2NqbzZKMjN0di93blQ2d3lOZkZDbzR0Y2F2WGdleitnZTk0L3ll?=
 =?utf-8?B?NFVjaHFSNkJ2cTJBN21IYTdSLzgrSVdhMTdRcGF6MjVJSVFZb3FVT1VuOGxS?=
 =?utf-8?B?akV3QUUrUVI3ZkIwTmxkUGhtS054NU83eC8wZXJCMDJwT3Z0dS9Pby8rVUNp?=
 =?utf-8?B?THp6bDZoYzBrVWs1am1aSVZuRzQxWFQ2K2lZTTdRbUQyaWo2OFpGSExQTWla?=
 =?utf-8?B?Zm4wRjc2UHpmemdkVHlZc3NIOElFbWp4T1E4WXh6YnFYVWtYc28rMU9zQ3Qw?=
 =?utf-8?B?NGFHMEN1dHN4RXFOL0JFM1FybllnaDJkZThOZVJtZS9ReWQwNUFxSmF1OTZh?=
 =?utf-8?B?TTY5aFMzMW5BamllbGFDaGY4VXB5YzU3bjJYVlF6L3RmUm9YQm95RTZ1Z0hq?=
 =?utf-8?B?QXVPa0tNVkFacWZQTDc4ZmFGdDk5UkduR1NKMjNTYm44YUp1ZzhhK0lRR0Nw?=
 =?utf-8?B?TFQ3KzB3SHlkVUR3aW52bDdJRW9SRU1weEoySlNBaExKaFVIbjhNWWIzZjNh?=
 =?utf-8?B?M2k2ZzJPQUJOV2Y0UCtPeTRQandTUElxWFFrNzNodHNYdGdEOStxM05NSnpz?=
 =?utf-8?B?M21SWFc4QVJ0aVJPUDZ2bHRic1NVOG1aYTJnd1NEWmc1WnFybVVQaS9Iaisr?=
 =?utf-8?B?bS9hNjF0QmhkTk1HV1lLZ2ZYOXBEb1ord1grQWtJV1FYdGRBUThxbmZaQXJl?=
 =?utf-8?B?MzFQbzRXcm5tOWpMOTdwZjduTkdCc3VUd1FGYW9NdGpCczdIc0FudHlDSmxK?=
 =?utf-8?B?c0x6Rkx1aFFWM2F3akxjMGs5ZmZqNFVqOFFyS1VqMVh1NG9qUVFzWkVLTm4v?=
 =?utf-8?B?SkRkcnhOMjM4ZUhCb3VKdHY5ODVRWmRCYWdNdUNneUx6WHJpckdQTUhpei9L?=
 =?utf-8?Q?YaWSV8YuWb7LHNIeZvUx5DFbr?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7abd5f2c-133d-4aaa-13a9-08db0ff980c2
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3835.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Feb 2023 08:40:48.8126
 (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: E9vt+t0v2oLzkFLieP4b1RHSmquaiiNO664GPeTq7UnwG7TgnGl8cWrHAxi+OA3pEWmy8C7gOGzvqfK1XpOGQQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB8245
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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?

>
>>>>> +     }
>>>>> +
>>>>> +     return 0;
>>>>> +}
> [...]
>
> Thanks,
> Olek