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

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

[Haiyang Zhang]

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;
+
 	debugfs_create_u16("adapter-MTU", 0400, gc->mana_pci_debugfs, &gc->adapter_mtu);
 
 	return 0;
@@ -2441,7 +2446,7 @@ static void mana_destroy_vport(struct mana_port_context *apc)
 	mana_destroy_txq(apc);
 	mana_uncfg_vport(apc);
 
-	if (gd->gdma_context->is_pf)
+	if (gd->gdma_context->is_pf && !apc->ac->bm_hostmode)
 		mana_pf_deregister_hw_vport(apc);
 }
 
@@ -2453,7 +2458,7 @@ static int mana_create_vport(struct mana_port_context *apc,
 
 	apc->default_rxobj = INVALID_MANA_HANDLE;
 
-	if (gd->gdma_context->is_pf) {
+	if (gd->gdma_context->is_pf && !apc->ac->bm_hostmode) {
 		err = mana_pf_register_hw_vport(apc);
 		if (err)
 			return err;
@@ -2689,7 +2694,7 @@ int mana_alloc_queues(struct net_device *ndev)
 		goto destroy_vport;
 	}
 
-	if (gd->gdma_context->is_pf) {
+	if (gd->gdma_context->is_pf && !apc->ac->bm_hostmode) {
 		err = mana_pf_register_filter(apc);
 		if (err)
 			goto destroy_vport;
@@ -2751,7 +2756,7 @@ static int mana_dealloc_queues(struct net_device *ndev)
 
 	mana_chn_setxdp(apc, NULL);
 
-	if (gd->gdma_context->is_pf)
+	if (gd->gdma_context->is_pf && !apc->ac->bm_hostmode)
 		mana_pf_deregister_filter(apc);
 
 	/* No packet can be transmitted now since apc->port_is_up is false.
@@ -2998,6 +3003,7 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
 	struct gdma_context *gc = gd->gdma_context;
 	struct mana_context *ac = gd->driver_data;
 	struct device *dev = gc->dev;
+	u8 bm_hostmode = 0;
 	u16 num_ports = 0;
 	int err;
 	int i;
@@ -3026,10 +3032,12 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
 	}
 
 	err = mana_query_device_cfg(ac, MANA_MAJOR_VERSION, MANA_MINOR_VERSION,
-				    MANA_MICRO_VERSION, &num_ports);
+				    MANA_MICRO_VERSION, &num_ports, &bm_hostmode);
 	if (err)
 		goto out;
 
+	ac->bm_hostmode = bm_hostmode;
+
 	if (!resuming) {
 		ac->num_ports = num_ports;
 	} else {
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 0f78065de8fe..38238c1d00bf 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -408,6 +408,7 @@ struct mana_context {
 	struct gdma_dev *gdma_dev;
 
 	u16 num_ports;
+	u8 bm_hostmode;
 
 	struct mana_eq *eqs;
 	struct dentry *mana_eqs_debugfs;
@@ -557,7 +558,8 @@ struct mana_query_device_cfg_resp {
 	u64 pf_cap_flags4;
 
 	u16 max_num_vports;
-	u16 reserved;
+	u8 bm_hostmode; /* response v3: Bare Metal Host Mode */
+	u8 reserved;
 	u32 max_num_eqs;
 
 	/* response v2: */
-- 
2.34.1

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

[Simon Horman]

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?

...

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

[Haiyang Zhang]

> -----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,
- Haiyang

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

[Simon Horman]

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?

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

[Paolo Abeni]

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@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?

@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

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

[Haiyang Zhang]

> -----Original Message-----
> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Thursday, May 22, 2025 9:44 AM
> To: Simon Horman <horms@kernel.org>; Haiyang Zhang
> <haiyangz@microsoft.com>

> >>>>  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.

Simon:
Yes, you are right. So newer HW can support older driver, and vice
versa.

Paolo:
The MANA protocol doesn't remove any existing fields during upgrades.

So (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3) will continue
to work in the future. If we change it to 
(resp.hdr.response.msg_version == GDMA_MESSAGE_V3), 
we will have to remember to update it to something like:
(resp.hdr.response.msg_version >= GDMA_MESSAGE_V3 &&
 resp.hdr.response.msg_version <= GDMA_MESSAGE_V5), 
if the version is upgraded to v5 in the future. And keep on updating
the checks on existing fields every time when the version is
upgraded.

So, can I keep the ">=" condition, to avoid future bug if anyone
forget to update checks on all existing fields?

Thanks,
- Haiyang

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

[Paolo Abeni]

On 5/22/25 4:51 PM, Haiyang Zhang wrote:
>> -----Original Message-----
>> From: Paolo Abeni <pabeni@redhat.com>
>> Sent: Thursday, May 22, 2025 9:44 AM
>> To: Simon Horman <horms@kernel.org>; Haiyang Zhang
>> <haiyangz@microsoft.com>
> 
>>>>>>  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.
> 
> Simon:
> Yes, you are right. So newer HW can support older driver, and vice
> versa.
> 
> Paolo:
> The MANA protocol doesn't remove any existing fields during upgrades.
> 
> So (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3) will continue
> to work in the future. If we change it to 
> (resp.hdr.response.msg_version == GDMA_MESSAGE_V3), 
> we will have to remember to update it to something like:
> (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3 &&
>  resp.hdr.response.msg_version <= GDMA_MESSAGE_V5), 
> if the version is upgraded to v5 in the future. And keep on updating
> the checks on existing fields every time when the version is
> upgraded.
> 
> So, can I keep the ">=" condition, to avoid future bug if anyone
> forget to update checks on all existing fields?

Ok, thanks for the clarification. fine by me.

/P

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 19 May 2025 09:20:36 -0700 you 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.
> 
> [...]

Here is the summary with links:
  - [net-next,v2] net: mana: Add support for Multi Vports on Bare metal
    https://git.kernel.org/netdev/net-next/c/290e5d3c49f6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html