linux-kernel - Re: [EXTERNAL] Re: [PATCH net-next,v2] net: mana: Add support for Multi Vports on Bare metal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5470f2d2-d3cb-4451-b8e8-5ee768ed9741@redhat.com>
Date: Thu, 22 May 2025 15:43:52 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Simon Horman <horms@...nel.org>, Haiyang Zhang <haiyangz@...rosoft.com>
Cc: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 Dexuan Cui <decui@...rosoft.com>,
 "stephen@...workplumber.org" <stephen@...workplumber.org>,
 KY Srinivasan <kys@...rosoft.com>, Paul Rosswurm <paulros@...rosoft.com>,
 "olaf@...fle.de" <olaf@...fle.de>, "vkuznets@...hat.com"
 <vkuznets@...hat.com>, "davem@...emloft.net" <davem@...emloft.net>,
 "wei.liu@...nel.org" <wei.liu@...nel.org>,
 "edumazet@...gle.com" <edumazet@...gle.com>,
 "kuba@...nel.org" <kuba@...nel.org>, "leon@...nel.org" <leon@...nel.org>,
 Long Li <longli@...rosoft.com>,
 "ssengar@...ux.microsoft.com" <ssengar@...ux.microsoft.com>,
 "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
 "daniel@...earbox.net" <daniel@...earbox.net>,
 "john.fastabend@...il.com" <john.fastabend@...il.com>,
 "bpf@...r.kernel.org" <bpf@...r.kernel.org>, "ast@...nel.org"
 <ast@...nel.org>, "hawk@...nel.org" <hawk@...nel.org>,
 "tglx@...utronix.de" <tglx@...utronix.de>,
 "shradhagupta@...ux.microsoft.com" <shradhagupta@...ux.microsoft.com>,
 "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
 Konstantin Taranov <kotaranov@...rosoft.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [EXTERNAL] Re: [PATCH net-next,v2] net: mana: Add support for
 Multi Vports on Bare metal

On 5/22/25 2:02 PM, Simon Horman wrote:
> On Wed, May 21, 2025 at 05:28:33PM +0000, Haiyang Zhang wrote:
>>> -----Original Message-----
>>> From: Simon Horman <horms@...nel.org>
>>> Sent: Wednesday, May 21, 2025 10:03 AM
>>> To: Haiyang Zhang <haiyangz@...rosoft.com>
>>> Cc: linux-hyperv@...r.kernel.org; netdev@...r.kernel.org; Dexuan Cui
>>> <decui@...rosoft.com>; stephen@...workplumber.org; KY Srinivasan
>>> <kys@...rosoft.com>; Paul Rosswurm <paulros@...rosoft.com>;
>>> olaf@...fle.de; vkuznets@...hat.com; davem@...emloft.net;
>>> wei.liu@...nel.org; edumazet@...gle.com; kuba@...nel.org;
>>> pabeni@...hat.com; leon@...nel.org; Long Li <longli@...rosoft.com>;
>>> ssengar@...ux.microsoft.com; linux-rdma@...r.kernel.org;
>>> daniel@...earbox.net; john.fastabend@...il.com; bpf@...r.kernel.org;
>>> ast@...nel.org; hawk@...nel.org; tglx@...utronix.de;
>>> shradhagupta@...ux.microsoft.com; andrew+netdev@...n.ch; Konstantin
>>> Taranov <kotaranov@...rosoft.com>; linux-kernel@...r.kernel.org
>>> Subject: [EXTERNAL] Re: [PATCH net-next,v2] net: mana: Add support for
>>> Multi Vports on Bare metal
>>>
>>> On Mon, May 19, 2025 at 09:20:36AM -0700, Haiyang Zhang wrote:
>>>> To support Multi Vports on Bare metal, increase the device config
>>> response
>>>> version. And, skip the register HW vport, and register filter steps,
>>> when
>>>> the Bare metal hostmode is set.
>>>>
>>>> Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>
>>>> ---
>>>> v2:
>>>>   Updated comments as suggested by ALOK TIWARI.
>>>>   Fixed the version check.
>>>>
>>>> ---
>>>>  drivers/net/ethernet/microsoft/mana/mana_en.c | 24 ++++++++++++-------
>>>>  include/net/mana/mana.h                       |  4 +++-
>>>>  2 files changed, 19 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
>>> b/drivers/net/ethernet/microsoft/mana/mana_en.c
>>>> index 2bac6be8f6a0..9c58d9e0bbb5 100644
>>>> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
>>>> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
>>>> @@ -921,7 +921,7 @@ static void mana_pf_deregister_filter(struct
>>> mana_port_context *apc)
>>>>
>>>>  static int mana_query_device_cfg(struct mana_context *ac, u32
>>> proto_major_ver,
>>>>  				 u32 proto_minor_ver, u32 proto_micro_ver,
>>>> -				 u16 *max_num_vports)
>>>> +				 u16 *max_num_vports, u8 *bm_hostmode)
>>>>  {
>>>>  	struct gdma_context *gc = ac->gdma_dev->gdma_context;
>>>>  	struct mana_query_device_cfg_resp resp = {};
>>>> @@ -932,7 +932,7 @@ static int mana_query_device_cfg(struct mana_context
>>> *ac, u32 proto_major_ver,
>>>>  	mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_DEV_CONFIG,
>>>>  			     sizeof(req), sizeof(resp));
>>>>
>>>> -	req.hdr.resp.msg_version = GDMA_MESSAGE_V2;
>>>> +	req.hdr.resp.msg_version = GDMA_MESSAGE_V3;
>>>>
>>>>  	req.proto_major_ver = proto_major_ver;
>>>>  	req.proto_minor_ver = proto_minor_ver;
>>>
>>>> @@ -956,11 +956,16 @@ static int mana_query_device_cfg(struct
>>> mana_context *ac, u32 proto_major_ver,
>>>>
>>>>  	*max_num_vports = resp.max_num_vports;
>>>>
>>>> -	if (resp.hdr.response.msg_version == GDMA_MESSAGE_V2)
>>>> +	if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2)
>>>>  		gc->adapter_mtu = resp.adapter_mtu;
>>>>  	else
>>>>  		gc->adapter_mtu = ETH_FRAME_LEN;
>>>>
>>>> +	if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3)
>>>> +		*bm_hostmode = resp.bm_hostmode;
>>>> +	else
>>>> +		*bm_hostmode = 0;
>>>
>>> Hi,
>>>
>>> Perhaps not strictly related to this patch, but I see
>>> that mana_verify_resp_hdr() is called a few lines above.
>>> And that verifies a minimum msg_version. But I do not see
>>> any verification of the maximum msg_version supported by the code.
>>>
>>> I am concerned about a hypothetical scenario where, say the as yet unknown
>>> version 5 is sent as the version, and the above behaviour is used, while
>>> not being correct.
>>>
>>> Could you shed some light on this?
>>>
>>
>> In driver, we specify the expected reply msg version is v3 here:
>> req.hdr.resp.msg_version = GDMA_MESSAGE_V3;
>>
>> If the HW side is upgraded, it won't send reply msg version higher
>> than expected, which may break the driver.
> 
> Thanks,
> 
> If I understand things correctly the HW side will honour the
> req.hdr.resp.msg_version and thus the SW won't receive anything
> it doesn't expect. Is that right?

@Haiyang, if Simon's interpretation is correct, please change the
version checking in the driver from:

	if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3)

to
	if (resp.hdr.response.msg_version == GDMA_MESSAGE_V3)

As the current code is misleading.

Thanks,

Paolo