[PATCH 0/3] Add support for speed in MANA ethtool

[PATCH 0/3] Add support for speed in MANA ethtool

[Erni Sri Satya Vennela]

Display speed information of the port via get_link_ksettings
ethtool operation using a HWC command MANA_QUERY_LINK_CONFIG
to fetch the speed information from the hardware.

Add support for mana_set_link_ksettings in MANA ethtool operation.
A HWC command (MANA_SET_BW_CLAMP) is sent to hardware to set
the bandwidth at specified speed.

This feature is not supported by all hardware. For any unsupported
clusters, the speed will be displayed as Unknown.

Erni Sri Satya Vennela (3):
  net: mana: Add speed support in mana_get_link_ksettings
  net: mana: Implement set_link_ksettings in ethtool for speed
  net: mana: Handle unsupported HWC commands

 .../net/ethernet/microsoft/mana/hw_channel.c  |  4 +
 drivers/net/ethernet/microsoft/mana/mana_en.c | 92 +++++++++++++++++++
 .../ethernet/microsoft/mana/mana_ethtool.c    | 19 ++++
 include/net/mana/mana.h                       | 33 +++++++
 4 files changed, 148 insertions(+)

-- 
2.34.1

[PATCH 1/3] net: mana: Add speed support in mana_get_link_ksettings

[Erni Sri Satya Vennela]

Add support for speed in mana ethtool get_link_ksettings
operation. This feature is not supported by all hardware.

Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 42 +++++++++++++++++++
 .../ethernet/microsoft/mana/mana_ethtool.c    |  6 +++
 include/net/mana/mana.h                       | 17 ++++++++
 3 files changed, 65 insertions(+)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index aa1e47233fe5..5fa8e1e2ff9a 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -1161,6 +1161,48 @@ static int mana_cfg_vport_steering(struct mana_port_context *apc,
 	return err;
 }
 
+int mana_query_link_cfg(struct mana_port_context *apc)
+{
+	struct net_device *ndev = apc->ndev;
+	struct mana_query_link_config_req req = {};
+	struct mana_query_link_config_resp resp = {};
+	int err;
+
+	mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_LINK_CONFIG,
+			     sizeof(req), sizeof(resp));
+
+	req.vport = apc->port_handle;
+
+	err = mana_send_request(apc->ac, &req, sizeof(req), &resp,
+				sizeof(resp));
+
+	if (err) {
+		netdev_err(ndev, "Failed to query link config: %d\n", err);
+		goto out;
+	}
+
+	err = mana_verify_resp_hdr(&resp.hdr, MANA_QUERY_LINK_CONFIG,
+				   sizeof(resp));
+
+	if (err || resp.hdr.status) {
+		netdev_err(ndev, "Failed to query link config: %d, 0x%x\n", err,
+			   resp.hdr.status);
+		if (!err)
+			err = -EPROTO;
+		goto out;
+	}
+
+	if (resp.qos_unconfigured) {
+		err = -EINVAL;
+		goto out;
+	}
+	apc->speed = resp.link_speed;
+	return 0;
+
+out:
+	return err;
+}
+
 int mana_create_wq_obj(struct mana_port_context *apc,
 		       mana_handle_t vport,
 		       u32 wq_type, struct mana_obj_spec *wq_spec,
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index c419626073f5..48234a738d26 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -427,6 +427,12 @@ static int mana_set_ringparam(struct net_device *ndev,
 static int mana_get_link_ksettings(struct net_device *ndev,
 				   struct ethtool_link_ksettings *cmd)
 {
+	struct mana_port_context *apc = netdev_priv(ndev);
+	int err;
+
+	err = mana_query_link_cfg(apc);
+
+	cmd->base.speed = (err) ? SPEED_UNKNOWN : apc->speed;
 	cmd->base.duplex = DUPLEX_FULL;
 	cmd->base.port = PORT_OTHER;
 
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 0d00b24eacaf..5f039ce99ade 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -468,6 +468,8 @@ struct mana_port_context {
 
 	u16 port_idx;
 
+	u32 speed;
+
 	bool port_is_up;
 	bool port_st_save; /* Saved port state */
 
@@ -497,6 +499,7 @@ struct bpf_prog *mana_xdp_get(struct mana_port_context *apc);
 void mana_chn_setxdp(struct mana_port_context *apc, struct bpf_prog *prog);
 int mana_bpf(struct net_device *ndev, struct netdev_bpf *bpf);
 void mana_query_gf_stats(struct mana_port_context *apc);
+int mana_query_link_cfg(struct mana_port_context *apc);
 int mana_pre_alloc_rxbufs(struct mana_port_context *apc, int mtu, int num_queues);
 void mana_pre_dealloc_rxbufs(struct mana_port_context *apc);
 
@@ -523,6 +526,7 @@ enum mana_command_code {
 	MANA_FENCE_RQ		= 0x20006,
 	MANA_CONFIG_VPORT_RX	= 0x20007,
 	MANA_QUERY_VPORT_CONFIG	= 0x20008,
+	MANA_QUERY_LINK_CONFIG	= 0x2000A,
 
 	/* Privileged commands for the PF mode */
 	MANA_REGISTER_FILTER	= 0x28000,
@@ -531,6 +535,19 @@ enum mana_command_code {
 	MANA_DEREGISTER_HW_PORT	= 0x28004,
 };
 
+/* Query Link Configuration*/
+struct mana_query_link_config_req {
+	struct gdma_req_hdr hdr;
+	mana_handle_t vport;
+}; /* HW DATA */
+
+struct mana_query_link_config_resp {
+	struct gdma_resp_hdr hdr;
+	u32 link_speed;
+	bool qos_unconfigured;
+	u8 reserved[3];
+}; /* HW DATA */
+
 /* Query Device Configuration */
 struct mana_query_device_cfg_req {
 	struct gdma_req_hdr hdr;
-- 
2.34.1

[PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Erni Sri Satya Vennela]

Add support for ethtool_set_link_ksettings for mana.
Set speed information of the port using ethtool. This
feature is not supported by all hardware.

Before the change:
$ethtool -s enP30832s1 speed 100
>netlink error: Operation not supported
$ethtool enP30832s1
>Settings for enP30832s1:
        Supported ports: [  ]
        Supported link modes:   Not reported
        Supported pause frame use: No
        Supports auto-negotiation: No
        Supported FEC modes: Not reported
        Advertised link modes:  Not reported
        Advertised pause frame use: No
        Advertised auto-negotiation: No
        Advertised FEC modes: Not reported
        Speed: Unknown!
        Duplex: Full
        Auto-negotiation: off
        Port: Other
        PHYAD: 0
        Transceiver: internal
        Link detected: yes

After the change:
$ethtool -s enP30832s1 speed 100
$ethtool enP30832s1
>Settings for enP30832s1:
        Supported ports: [  ]
        Supported link modes:   Not reported
        Supported pause frame use: No
        Supports auto-negotiation: No
        Supported FEC modes: Not reported
        Advertised link modes:  Not reported
        Advertised pause frame use: No
        Advertised auto-negotiation: No
        Advertised FEC modes: Not reported
        Speed: 100Mb/s
        Duplex: Full
        Auto-negotiation: off
        Port: Other
        PHYAD: 0
        Transceiver: internal
        Link detected: yes

Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/mana_en.c | 39 +++++++++++++++++++
 .../ethernet/microsoft/mana/mana_ethtool.c    | 13 +++++++
 include/net/mana/mana.h                       | 16 ++++++++
 3 files changed, 68 insertions(+)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 5fa8e1e2ff9a..bcc273427423 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -1203,6 +1203,45 @@ int mana_query_link_cfg(struct mana_port_context *apc)
 	return err;
 }
 
+int mana_set_bw_clamp(struct mana_port_context *apc, u32 speed)
+{
+	struct mana_set_bw_clamp_req req = {};
+	struct mana_set_bw_clamp_resp resp = {};
+	struct net_device *ndev = apc->ndev;
+	int err;
+
+	mana_gd_init_req_hdr(&req.hdr, MANA_SET_BW_CLAMP,
+			     sizeof(req), sizeof(resp));
+	req.vport = apc->port_handle;
+	req.link_speed = speed;
+	req.enable_clamping = TRI_STATE_TRUE;
+
+	err = mana_send_request(apc->ac, &req, sizeof(req), &resp,
+				sizeof(resp));
+
+	if (err) {
+		netdev_err(ndev, "Failed to set bandwidth clamp for speed %u, err = %d",
+			   speed, err);
+		return err;
+	}
+
+	err = mana_verify_resp_hdr(&resp.hdr, MANA_SET_BW_CLAMP,
+				   sizeof(resp));
+
+	if (err || resp.hdr.status) {
+		netdev_err(ndev, "Failed to set bandwidth clamp: %d, 0x%x\n", err,
+			   resp.hdr.status);
+		if (!err)
+			err = -EPROTO;
+		return err;
+	}
+
+	if (resp.qos_unconfigured)
+		netdev_info(ndev, "QoS is unconfigured\n");
+
+	return 0;
+}
+
 int mana_create_wq_obj(struct mana_port_context *apc,
 		       mana_handle_t vport,
 		       u32 wq_type, struct mana_obj_spec *wq_spec,
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index 48234a738d26..b29d0fe0a201 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -439,6 +439,18 @@ static int mana_get_link_ksettings(struct net_device *ndev,
 	return 0;
 }
 
+static int mana_set_link_ksettings(struct net_device *ndev,
+				   const struct ethtool_link_ksettings *cmd)
+{
+	struct mana_port_context *apc = netdev_priv(ndev);
+	int err;
+
+	err = mana_set_bw_clamp(apc, cmd->base.speed);
+
+	apc->speed = (err) ? apc->speed : cmd->base.speed;
+	return 0;
+}
+
 const struct ethtool_ops mana_ethtool_ops = {
 	.get_ethtool_stats	= mana_get_ethtool_stats,
 	.get_sset_count		= mana_get_sset_count,
@@ -453,5 +465,6 @@ const struct ethtool_ops mana_ethtool_ops = {
 	.get_ringparam          = mana_get_ringparam,
 	.set_ringparam          = mana_set_ringparam,
 	.get_link_ksettings	= mana_get_link_ksettings,
+	.set_link_ksettings	= mana_set_link_ksettings,
 	.get_link		= ethtool_op_get_link,
 };
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 5f039ce99ade..b4c66ce9ee3a 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -500,6 +500,7 @@ void mana_chn_setxdp(struct mana_port_context *apc, struct bpf_prog *prog);
 int mana_bpf(struct net_device *ndev, struct netdev_bpf *bpf);
 void mana_query_gf_stats(struct mana_port_context *apc);
 int mana_query_link_cfg(struct mana_port_context *apc);
+int mana_set_bw_clamp(struct mana_port_context *apc, u32 speed);
 int mana_pre_alloc_rxbufs(struct mana_port_context *apc, int mtu, int num_queues);
 void mana_pre_dealloc_rxbufs(struct mana_port_context *apc);
 
@@ -527,6 +528,7 @@ enum mana_command_code {
 	MANA_CONFIG_VPORT_RX	= 0x20007,
 	MANA_QUERY_VPORT_CONFIG	= 0x20008,
 	MANA_QUERY_LINK_CONFIG	= 0x2000A,
+	MANA_SET_BW_CLAMP	= 0x2000B,
 
 	/* Privileged commands for the PF mode */
 	MANA_REGISTER_FILTER	= 0x28000,
@@ -548,6 +550,20 @@ struct mana_query_link_config_resp {
 	u8 reserved[3];
 }; /* HW DATA */
 
+/* Set Bandwidth Clamp*/
+struct mana_set_bw_clamp_req {
+	struct gdma_req_hdr hdr;
+	mana_handle_t vport;
+	enum TRI_STATE enable_clamping;
+	u32 link_speed;
+}; /* HW DATA */
+
+struct mana_set_bw_clamp_resp {
+	struct gdma_resp_hdr hdr;
+	bool qos_unconfigured;
+	u8 reserved[7];
+}; /* HW DATA */
+
 /* Query Device Configuration */
 struct mana_query_device_cfg_req {
 	struct gdma_req_hdr hdr;
-- 
2.34.1

[PATCH 3/3] net: mana: Handle unsupported HWC commands

[Erni Sri Satya Vennela]

If any of the HWC commands are not recognized by the
underlying hardware, the hardware returns the response
header status of -1. Log the information using
netdev_info_once to avoid multiple error logs in dmesg.

Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/hw_channel.c |  4 ++++
 drivers/net/ethernet/microsoft/mana/mana_en.c    | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index a00f915c5188..280218895c0e 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -867,6 +867,10 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 	}
 
 	if (ctx->status_code && ctx->status_code != GDMA_STATUS_MORE_ENTRIES) {
+		if (ctx->status_code == -1) {
+			err = -EOPNOTSUPP;
+			goto out;
+		}
 		dev_err(hwc->dev, "HWC: Failed hw_channel req: 0x%x\n",
 			ctx->status_code);
 		err = -EPROTO;
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index bcc273427423..898d18b220b8 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -779,6 +779,9 @@ static int mana_send_request(struct mana_context *ac, void *in_buf,
 	err = mana_gd_send_request(gc, in_len, in_buf, out_len,
 				   out_buf);
 	if (err || resp->status) {
+		if (err == -EOPNOTSUPP)
+			return err;
+
 		dev_err(dev, "Failed to send mana message: %d, 0x%x\n",
 			err, resp->status);
 		return err ? err : -EPROTO;
@@ -1177,6 +1180,10 @@ int mana_query_link_cfg(struct mana_port_context *apc)
 				sizeof(resp));
 
 	if (err) {
+		if (err == -EOPNOTSUPP) {
+			netdev_info_once(ndev, "MANA_QUERY_LINK_CONFIG not supported\n");
+			goto out;
+		}
 		netdev_err(ndev, "Failed to query link config: %d\n", err);
 		goto out;
 	}
@@ -1220,6 +1227,10 @@ int mana_set_bw_clamp(struct mana_port_context *apc, u32 speed)
 				sizeof(resp));
 
 	if (err) {
+		if (err == -EOPNOTSUPP) {
+			netdev_info_once(ndev, "MANA_SET_BW_CLAMP not supported\n");
+			return err;
+		}
 		netdev_err(ndev, "Failed to set bandwidth clamp for speed %u, err = %d",
 			   speed, err);
 		return err;
-- 
2.34.1

Re: [PATCH 1/3] net: mana: Add speed support in mana_get_link_ksettings

[Andrew Lunn]

> +int mana_query_link_cfg(struct mana_port_context *apc)
> +{
> +	struct net_device *ndev = apc->ndev;
> +	struct mana_query_link_config_req req = {};
> +	struct mana_query_link_config_resp resp = {};
> +	int err;
> +
> +	mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_LINK_CONFIG,
> +			     sizeof(req), sizeof(resp));
> +
> +	req.vport = apc->port_handle;
> +
> +	err = mana_send_request(apc->ac, &req, sizeof(req), &resp,
> +				sizeof(resp));
> +
> +	if (err) {
> +		netdev_err(ndev, "Failed to query link config: %d\n", err);
> +		goto out;
> +	}
> +
> +	err = mana_verify_resp_hdr(&resp.hdr, MANA_QUERY_LINK_CONFIG,
> +				   sizeof(resp));
> +
> +	if (err || resp.hdr.status) {
> +		netdev_err(ndev, "Failed to query link config: %d, 0x%x\n", err,
> +			   resp.hdr.status);
> +		if (!err)
> +			err = -EPROTO;
> +		goto out;
> +	}
> +
> +	if (resp.qos_unconfigured) {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +	apc->speed = resp.link_speed;

Might be worth adding a comment that the firmware is returning speed
in Mbps.

Or name the struct member link_speed_mbps.

	Andrew

Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Andrew Lunn]

On Thu, Mar 20, 2025 at 05:22:20AM -0700, Erni Sri Satya Vennela wrote:
> Add support for ethtool_set_link_ksettings for mana.
> Set speed information of the port using ethtool. This
> feature is not supported by all hardware.
> 
> Before the change:
> $ethtool -s enP30832s1 speed 100
> >netlink error: Operation not supported
> $ethtool enP30832s1
> >Settings for enP30832s1:
>         Supported ports: [  ]
>         Supported link modes:   Not reported

Since there are no link modes, what does this speed actually mean?

> After the change:
> $ethtool -s enP30832s1 speed 100

Is 

$ethtool -s enP30832s1 speed 42

permitted? 

or

$ethtool -s enP30832s1 speed -1

	Andrew

Re: [PATCH 1/3] net: mana: Add speed support in mana_get_link_ksettings

[Erni Sri Satya Vennela]

On Thu, Mar 20, 2025 at 02:37:47PM +0100, Andrew Lunn wrote:
> > +int mana_query_link_cfg(struct mana_port_context *apc)
> > +{
> > +	struct net_device *ndev = apc->ndev;
> > +	struct mana_query_link_config_req req = {};
> > +	struct mana_query_link_config_resp resp = {};
> > +	int err;
> > +
> > +	mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_LINK_CONFIG,
> > +			     sizeof(req), sizeof(resp));
> > +
> > +	req.vport = apc->port_handle;
> > +
> > +	err = mana_send_request(apc->ac, &req, sizeof(req), &resp,
> > +				sizeof(resp));
> > +
> > +	if (err) {
> > +		netdev_err(ndev, "Failed to query link config: %d\n", err);
> > +		goto out;
> > +	}
> > +
> > +	err = mana_verify_resp_hdr(&resp.hdr, MANA_QUERY_LINK_CONFIG,
> > +				   sizeof(resp));
> > +
> > +	if (err || resp.hdr.status) {
> > +		netdev_err(ndev, "Failed to query link config: %d, 0x%x\n", err,
> > +			   resp.hdr.status);
> > +		if (!err)
> > +			err = -EPROTO;
> > +		goto out;
> > +	}
> > +
> > +	if (resp.qos_unconfigured) {
> > +		err = -EINVAL;
> > +		goto out;
> > +	}
> > +	apc->speed = resp.link_speed;
> 
> Might be worth adding a comment that the firmware is returning speed
> in Mbps.
> 
> Or name the struct member link_speed_mbps.
> 
Thank you for your suggestion. I'll make this change for the next
version of this patchset.
> 	Andrew

Re: [PATCH 1/3] net: mana: Add speed support in mana_get_link_ksettings

[Andrew Lunn]

On Mon, Mar 24, 2025 at 10:43:39AM -0700, Erni Sri Satya Vennela wrote:
> On Thu, Mar 20, 2025 at 02:37:47PM +0100, Andrew Lunn wrote:
> > > +int mana_query_link_cfg(struct mana_port_context *apc)
> > > +{
> > > +	struct net_device *ndev = apc->ndev;
> > > +	struct mana_query_link_config_req req = {};
> > > +	struct mana_query_link_config_resp resp = {};
> > > +	int err;
> > > +
> > > +	mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_LINK_CONFIG,
> > > +			     sizeof(req), sizeof(resp));
> > > +
> > > +	req.vport = apc->port_handle;
> > > +
> > > +	err = mana_send_request(apc->ac, &req, sizeof(req), &resp,
> > > +				sizeof(resp));
> > > +
> > > +	if (err) {
> > > +		netdev_err(ndev, "Failed to query link config: %d\n", err);
> > > +		goto out;
> > > +	}
> > > +
> > > +	err = mana_verify_resp_hdr(&resp.hdr, MANA_QUERY_LINK_CONFIG,
> > > +				   sizeof(resp));
> > > +
> > > +	if (err || resp.hdr.status) {
> > > +		netdev_err(ndev, "Failed to query link config: %d, 0x%x\n", err,
> > > +			   resp.hdr.status);
> > > +		if (!err)
> > > +			err = -EPROTO;
> > > +		goto out;
> > > +	}
> > > +
> > > +	if (resp.qos_unconfigured) {
> > > +		err = -EINVAL;
> > > +		goto out;
> > > +	}
> > > +	apc->speed = resp.link_speed;
> > 
> > Might be worth adding a comment that the firmware is returning speed
> > in Mbps.
> > 
> > Or name the struct member link_speed_mbps.
> > 
> Thank you for your suggestion. I'll make this change for the next
> version of this patchset.

Please answer my other questions before posting the next version of
the patch. I'm really questioning if this is the correct uAPI to be
using. You have a very poor description of what you are trying to
do. Maybe TC is the better fit? Does this speed apply to ingress and
egress? If so, there are two leaky buckets, so why only one
configuration value? Or can you only configure ingress?

Also, if i understand correctly MANA is a virtual device and this is
the VM side of it. If this is used for bandwidth limitation, why is
the VM controlling this, not the hypervisor? What is the security
model?

	Andrew

Re: [PATCH 1/3] net: mana: Add speed support in mana_get_link_ksettings

[Erni Sri Satya Vennela]

On Mon, Mar 24, 2025 at 07:44:39PM +0100, Andrew Lunn wrote:
> On Mon, Mar 24, 2025 at 10:43:39AM -0700, Erni Sri Satya Vennela wrote:
> > On Thu, Mar 20, 2025 at 02:37:47PM +0100, Andrew Lunn wrote:
> > > > +int mana_query_link_cfg(struct mana_port_context *apc)
> > > > +{
> > > > +	struct net_device *ndev = apc->ndev;
> > > > +	struct mana_query_link_config_req req = {};
> > > > +	struct mana_query_link_config_resp resp = {};
> > > > +	int err;
> > > > +
> > > > +	mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_LINK_CONFIG,
> > > > +			     sizeof(req), sizeof(resp));
> > > > +
> > > > +	req.vport = apc->port_handle;
> > > > +
> > > > +	err = mana_send_request(apc->ac, &req, sizeof(req), &resp,
> > > > +				sizeof(resp));
> > > > +
> > > > +	if (err) {
> > > > +		netdev_err(ndev, "Failed to query link config: %d\n", err);
> > > > +		goto out;
> > > > +	}
> > > > +
> > > > +	err = mana_verify_resp_hdr(&resp.hdr, MANA_QUERY_LINK_CONFIG,
> > > > +				   sizeof(resp));
> > > > +
> > > > +	if (err || resp.hdr.status) {
> > > > +		netdev_err(ndev, "Failed to query link config: %d, 0x%x\n", err,
> > > > +			   resp.hdr.status);
> > > > +		if (!err)
> > > > +			err = -EPROTO;
> > > > +		goto out;
> > > > +	}
> > > > +
> > > > +	if (resp.qos_unconfigured) {
> > > > +		err = -EINVAL;
> > > > +		goto out;
> > > > +	}
> > > > +	apc->speed = resp.link_speed;
> > > 
> > > Might be worth adding a comment that the firmware is returning speed
> > > in Mbps.
> > > 
> > > Or name the struct member link_speed_mbps.
> > > 
> > Thank you for your suggestion. I'll make this change for the next
> > version of this patchset.
> 
> Please answer my other questions before posting the next version of
> the patch. I'm really questioning if this is the correct uAPI to be
> using. You have a very poor description of what you are trying to
> do. Maybe TC is the better fit? Does this speed apply to ingress and
> egress? If so, there are two leaky buckets, so why only one
> configuration value? Or can you only configure ingress?
> 
The QoS control is at the hardware/firmware level and applies to the
egress rate.
> Also, if i understand correctly MANA is a virtual device and this is
> the VM side of it. If this is used for bandwidth limitation, why is
> the VM controlling this, not the hypervisor? What is the security
> model?
> 
In certain cluster and hardware versions, Azure allows this API to
restrict the bandwidth limit to a lesser value than what was configured
by the Azure control plane. The device will not allow a higher limit
than what was configured through the Azure control plane to be set by
the VM through this API.
> 	Andrew

Re: [PATCH 1/3] net: mana: Add speed support in mana_get_link_ksettings

[Andrew Lunn]

> The QoS control is at the hardware/firmware level and applies to the
> egress rate.

egress relative to the VM? So what the VM sends to the hypervisor.
There is no restriction the other way, hypervisor to the VM?

That is not what link modes do. 10Mbps is the limit in both
directions.

> > Also, if i understand correctly MANA is a virtual device and this is
> > the VM side of it. If this is used for bandwidth limitation, why is
> > the VM controlling this, not the hypervisor? What is the security
> > model?
> > 
> In certain cluster and hardware versions, Azure allows this API to
> restrict the bandwidth limit to a lesser value than what was configured
> by the Azure control plane. The device will not allow a higher limit
> than what was configured through the Azure control plane to be set by
> the VM through this API.

So all this information needs adding to the commit message. When you
are using an API in a strange way, you have to expect questions to be
asked, and you can save a lot of time by answering those questions in
the commit message, before they are even asked.

So, i think this is the wrong API.

Please implement it as a TC offload. I'm not an TC expert, but htb
might work.

	Andrew

Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Erni Sri Satya Vennela]

On Thu, Mar 20, 2025 at 02:43:34PM +0100, Andrew Lunn wrote:
> On Thu, Mar 20, 2025 at 05:22:20AM -0700, Erni Sri Satya Vennela wrote:
> > Add support for ethtool_set_link_ksettings for mana.
> > Set speed information of the port using ethtool. This
> > feature is not supported by all hardware.
> > 
> > Before the change:
> > $ethtool -s enP30832s1 speed 100
> > >netlink error: Operation not supported
> > $ethtool enP30832s1
> > >Settings for enP30832s1:
> >         Supported ports: [  ]
> >         Supported link modes:   Not reported
> 
> Since there are no link modes, what does this speed actually mean?
> 
Initially we planned to support for speed incrementally by 1Mbps, but
after internal discussion with the host team, we will be supporting 
only speed which is multiples of 100. The HWC commands 
MANA_QUERY_LINK_CONFIG and MANA_SET_BW_CLAMP help us to get and
set the speed in the hardware respectively.
> > After the change:
> > $ethtool -s enP30832s1 speed 100
> 
> Is 
> 
> $ethtool -s enP30832s1 speed 42
> 
> permitted? 
> 
The user will be allowed to set the speed which are multiples 
of 100. And the minimum allowed bandwidth is 100Mbps. I will be making
this change in the next version of this patch.
> or
> 
> $ethtool -s enP30832s1 speed -1
> 
This case is handled by the firmware, which throws an error:
ethtool (-s): invalid value '-1' for parameter 'speed'
> 	Andrew

Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Andrew Lunn]

> > $ethtool -s enP30832s1 speed -1
> > 
> This case is handled by the firmware, which throws an error:
> ethtool (-s): invalid value '-1' for parameter 'speed'

So how do i remove the speed limitation i previously installed? -1 is
SPEED_UNKNOWN which is what you default to.

Using TC would be more natural in this case. The user action is to
remove the TC filter and that should set it back to unlimited.

	Andrew

Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Jakub Kicinski]

On Tue, 25 Mar 2025 18:52:40 +0100 Andrew Lunn wrote:
> Using TC would be more natural in this case. The user action is to
> remove the TC filter and that should set it back to unlimited.

better still - net shapers

RE: [EXTERNAL] Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Haiyang Zhang]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Tuesday, March 25, 2025 3:22 PM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Erni Sri Satya Vennela <ernis@linux.microsoft.com>; KY Srinivasan
> <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>;
> wei.liu@kernel.org; Dexuan Cui <decui@microsoft.com>;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> pabeni@redhat.com; Long Li <longli@microsoft.com>; Konstantin Taranov
> <kotaranov@microsoft.com>; horms@kernel.org; brett.creeley@amd.com;
> surenb@google.com; schakrabarti@linux.microsoft.com;
> kent.overstreet@linux.dev; shradhagupta@linux.microsoft.com;
> erick.archer@outlook.com; rosenp@gmail.com; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> rdma@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH 2/3] net: mana: Implement
> set_link_ksettings in ethtool for speed
> 
> On Tue, 25 Mar 2025 18:52:40 +0100 Andrew Lunn wrote:
> > Using TC would be more natural in this case. The user action is to
> > remove the TC filter and that should set it back to unlimited.
> 
> better still - net shapers

Thank you, Andrew & Jakub:
Sure, there are many (better) ways to control/shape the traffic.

This patch is just to support the ethtool option for the speed.
And seems there is no option on ethtool to reset NIC to the default
speed?

- Haiyang

Re: [EXTERNAL] Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Andrew Lunn]

> This patch is just to support the ethtool option for the speed.
> And seems there is no option on ethtool to reset NIC to the default
> speed?

There is no such thing as default speed. Speed is either:

1) Negotiated with the link partner, picking the highest speed link
modes both partners support

2) Forced to a specific speed, based on one of the link modes the
interfaces supports.

Since you don't have link modes, you are abusing the API. You should
choose a different API which actually fits what you are doing,
configuring a leaky bucket shaper.

	Andrew

RE: [EXTERNAL] Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Haiyang Zhang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Tuesday, March 25, 2025 3:59 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Jakub Kicinski <kuba@kernel.org>; Erni Sri Satya Vennela
> <ernis@linux.microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> wei.liu@kernel.org; Dexuan Cui <decui@microsoft.com>;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> pabeni@redhat.com; Long Li <longli@microsoft.com>; Konstantin Taranov
> <kotaranov@microsoft.com>; horms@kernel.org; brett.creeley@amd.com;
> surenb@google.com; schakrabarti@linux.microsoft.com;
> kent.overstreet@linux.dev; shradhagupta@linux.microsoft.com;
> erick.archer@outlook.com; rosenp@gmail.com; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> rdma@vger.kernel.org; Paul Rosswurm <paulros@microsoft.com>
> Subject: Re: [EXTERNAL] Re: [PATCH 2/3] net: mana: Implement
> set_link_ksettings in ethtool for speed
> 
> > This patch is just to support the ethtool option for the speed.
> > And seems there is no option on ethtool to reset NIC to the default
> > speed?
> 
> There is no such thing as default speed. Speed is either:
> 
> 1) Negotiated with the link partner, picking the highest speed link
> modes both partners support
> 
> 2) Forced to a specific speed, based on one of the link modes the
> interfaces supports.
> 
> Since you don't have link modes, you are abusing the API. You should
> choose a different API which actually fits what you are doing,
> configuring a leaky bucket shaper.
> 

Could you please point us to the interface struct, callback function
names, and/or docs you are suggesting us to use?

Thanks,
- Haiyang

Re: [EXTERNAL] Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Andrew Lunn]

> Could you please point us to the interface struct, callback function
> names, and/or docs you are suggesting us to use?

If you cannot search the sources for tc htb, or net shaper, google the
same, etc, you probably cannot write the code either.

http://www.catb.org/esr/faqs/smart-questions.html

	Andrew

Re: [EXTERNAL] Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Erni Sri Satya Vennela]

Thank you Andrew, Jakub for pointing out all the available options. 

We are currently investigating their feasibility based on our use case.
Would update the thread/get in touch, about our findings before sending
out the next version.

Re: [EXTERNAL] Re: [PATCH 2/3] net: mana: Implement set_link_ksettings in ethtool for speed

[Erni Sri Satya Vennela]

I hope this message finds you well.

I wanted to follow up on this patchset I previously submitted. 
Based on your valuable feedback, I have made modifications 
and incorporated your suggestions along with my
previous logic. Please find the updated patchset attached.

https://lore.kernel.org/lkml/1744876630-26918-1-git-send-email-ernis@linux.microsoft.com/