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

[Simon Horman <horms@kernel.org>]

On Wed, May 21, 2025 at 05:28:33PM +0000, Haiyang Zhang wrote:
> 
> 
> > -----Original Message-----
> > From: Simon Horman <horms@kernel.org>
> > Sent: Wednesday, May 21, 2025 10:03 AM
> > To: Haiyang Zhang <haiyangz@microsoft.com>
> > Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; Dexuan Cui
> > <decui@microsoft.com>; stephen@networkplumber.org; KY Srinivasan
> > <kys@microsoft.com>; Paul Rosswurm <paulros@microsoft.com>;
> > olaf@aepfle.de; vkuznets@redhat.com; davem@davemloft.net;
> > wei.liu@kernel.org; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; leon@kernel.org; Long Li <longli@microsoft.com>;
> > ssengar@linux.microsoft.com; linux-rdma@vger.kernel.org;
> > daniel@iogearbox.net; john.fastabend@gmail.com; bpf@vger.kernel.org;
> > ast@kernel.org; hawk@kernel.org; tglx@linutronix.de;
> > shradhagupta@linux.microsoft.com; andrew+netdev@lunn.ch; Konstantin
> > Taranov <kotaranov@microsoft.com>; linux-kernel@vger.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@microsoft.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?