From: Gavin Li <gavinl@nvidia.com>
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>
Subject: Re: [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
Date: Fri, 17 Feb 2023 10:43:38 +0800 [thread overview]
Message-ID: <ca729a48-35a1-ef05-59d3-ef1539003051@nvidia.com> (raw)
In-Reply-To: <aefe00f0-2a15-9a43-2451-6d01e74cc48a@intel.com>
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
next prev parent reply other threads:[~2023-02-17 2:44 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-14 13:41 [PATCH net-next v1 0/3] net/mlx5e: Add GBP VxLAN HW offload support Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 1/3] vxlan: Expose helper vxlan_build_gbp_hdr Gavin Li
2023-02-14 14:56 ` Alexander Lobakin
2023-02-15 8:26 ` Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 2/3] net/mlx5e: Add helper for encap_info_equal for tunnels with options Gavin Li
2023-02-14 15:01 ` Alexander Lobakin
2023-02-15 2:54 ` Gavin Li
2023-02-15 16:56 ` Alexander Lobakin
2023-02-20 11:31 ` Gavin Li
2023-02-14 13:41 ` [PATCH net-next v1 3/3] net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload Gavin Li
2023-02-14 15:26 ` Alexander Lobakin
2023-02-15 2:50 ` Gavin Li
2023-02-15 16:53 ` Alexander Lobakin
2023-02-15 3:36 ` Gavin Li
2023-02-15 8:30 ` Gavin Li
2023-02-15 17:01 ` Alexander Lobakin
2023-02-16 8:40 ` Gavin Li
2023-02-16 15:19 ` Alexander Lobakin
2023-02-17 2:43 ` Gavin Li [this message]
2023-02-15 16:57 ` Alexander Lobakin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ca729a48-35a1-ef05-59d3-ef1539003051@nvidia.com \
--to=gavinl@nvidia.com \
--cc=aleksander.lobakin@intel.com \
--cc=bigeasy@linutronix.de \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=eng.alaamohamedsoliman.am@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maord@nvidia.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=roid@nvidia.com \
--cc=roopa@nvidia.com \
--cc=saeedm@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).