On Wed, May 21, 2025 at 05:28:33PM +0000, Haiyang Zhang wrote:
>
>
> > -----Original Message-----
> > From: Simon Horman <ho...@kernel.org>
> > Sent: Wednesday, May 21, 2025 10:03 AM
> > To: Haiyang Zhang <haiya...@microsoft.com>
> > Cc: linux-hyperv@vger.kernel.org; net...@vger.kernel.org; Dexuan Cui
> > <de...@microsoft.com>; step...@networkplumber.org; KY Srinivasan
> > <k...@microsoft.com>; Paul Rosswurm <paul...@microsoft.com>;
> > o...@aepfle.de; vkuzn...@redhat.com; da...@davemloft.net;
> > wei....@kernel.org; eduma...@google.com; k...@kernel.org;
> > pab...@redhat.com; l...@kernel.org; Long Li <lon...@microsoft.com>;
> > ssen...@linux.microsoft.com; linux-r...@vger.kernel.org;
> > dan...@iogearbox.net; john.fastab...@gmail.com; b...@vger.kernel.org;
> > a...@kernel.org; h...@kernel.org; t...@linutronix.de;
> > shradhagu...@linux.microsoft.com; andrew+net...@lunn.ch; Konstantin
> > Taranov <kotara...@microsoft.com>; linux-ker...@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 <haiya...@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?
