[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7612377e-1dd0-1350-feb3-3a737710c261@intel.com>
Date: Tue, 7 Mar 2023 17:58:49 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Gavin Li <gavinl@...dia.com>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
<pabeni@...hat.com>, <roopa@...dia.com>,
<eng.alaamohamedsoliman.am@...il.com>, <bigeasy@...utronix.de>,
<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<gavi@...dia.com>, <roid@...dia.com>, <maord@...dia.com>,
<saeedm@...dia.com>
Subject: Re: [PATCH RESEND net-next v4 0/4] net/mlx5e: Add GBP VxLAN HW
offload support
From: Gavin Li <gavinl@...dia.com>
Date: Tue, 7 Mar 2023 17:19:35 +0800
>
> On 3/6/2023 10:47 PM, Alexander Lobakin wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> From: Gavin Li <gavinl@...dia.com>
>> Date: Mon, 6 Mar 2023 05:02:58 +0200
>>
>>> Patch-1: Remove unused argument from functions.
>>> Patch-2: Expose helper function vxlan_build_gbp_hdr.
>>> Patch-3: Add helper function for encap_info_equal for tunnels with
>>> options.
>>> Patch-4: Add HW offloading support for TC flows with VxLAN GBP
>>> encap/decap
>>> in mlx ethernet driver.
>>>
>>> Gavin Li (4):
>>> vxlan: Remove unused argument from vxlan_build_gbp_hdr( ) and
>>> vxlan_build_gpe_hdr( )
>>> ---
>>> changelog:
>>> v2->v3
>>> - Addressed comments from Paolo Abeni
>>> - Add new patch
>>> ---
>>> vxlan: Expose helper vxlan_build_gbp_hdr
>>> ---
>>> changelog:
>>> v1->v2
>>> - Addressed comments from Alexander Lobakin
>>> - Use const to annotate read-only the pointer parameter
>>> ---
>>> net/mlx5e: Add helper for encap_info_equal for tunnels with options
>>> ---
>>> changelog:
>>> v3->v4
>>> - Addressed comments from Alexander Lobakin
>>> - Fix vertical alignment issue
>>> v1->v2
>>> - Addressed comments from Alexander Lobakin
>>> - Replace confusing pointer arithmetic with function call
>>> - Use boolean operator NOT to check if the function return value is
>>> not zero
>>> ---
>>> net/mlx5e: TC, Add support for VxLAN GBP encap/decap flows offload
>>> ---
>>> changelog:
>>> v3->v4
>>> - Addressed comments from Simon Horman
>>> - Using cast in place instead of changing API
>> I don't remember me acking this. The last thing I said is that in order
>> to avoid cast-aways you need to use _Generic(). 2 times. IIRC you said
>> "Ack" and that was the last message in that thread.
>> Now this. Without me in CCs, so I noticed it accidentally.
>> ???
>
> Not asked by you but you said you were OK if I used cast-aways. So I did
> the
>
> change in V3 and reverted back to using cast-away in V4.
My last reply was[0]:
"
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.
[...]
[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
"
Where did I say here I'm fine with W/As in the drivers? I mentioned two
options: cast-away in THE GENERIC INLINE, not the driver, or, more
preferred, following the way of container_of_const().
Then your reply[1]:
"ACK"
What did you ack then if you picked neither of those 2 options?
>
>>> v2->v3
>>> - Addressed comments from Alexander Lobakin
>>> - Remove the WA by casting away
>>> v1->v2
>>> - Addressed comments from Alexander Lobakin
>>> - Add a separate pair of braces around bitops
>>> - Remove the WA by casting away
>>> - Fit all log messages into one line
>>> - Use NL_SET_ERR_MSG_FMT_MOD to print the invalid value on error
>>> ---
>>>
>>> .../ethernet/mellanox/mlx5/core/en/tc_tun.h | 3 +
>>> .../mellanox/mlx5/core/en/tc_tun_encap.c | 32 ++++++++
>>> .../mellanox/mlx5/core/en/tc_tun_geneve.c | 24 +-----
>>> .../mellanox/mlx5/core/en/tc_tun_vxlan.c | 76 ++++++++++++++++++-
>>> drivers/net/vxlan/vxlan_core.c | 27 +------
>>> include/linux/mlx5/device.h | 6 ++
>>> include/linux/mlx5/mlx5_ifc.h | 13 +++-
>>> include/net/vxlan.h | 19 +++++
>>> 8 files changed, 149 insertions(+), 51 deletions(-)
>>>
>> Thanks,
>> Olek
[0]
https://lore.kernel.org/netdev/aefe00f0-2a15-9a43-2451-6d01e74cc48a@intel.com
[1]
https://lore.kernel.org/netdev/ca729a48-35a1-ef05-59d3-ef1539003051@nvidia.com
Thanks,
Olek
Powered by blists - more mailing lists