[PATCH 0/3] net: mana: Add HTB Qdisc offload support

[PATCH 0/3] net: mana: Add HTB Qdisc offload support

[Erni Sri Satya Vennela]

Introduce support for HTB Qdisc offload on the mana ethernet 
controller to enable bandwidth clamping for egress traffic.

The controller offloads only one HTB leaf to support bandwidth
clamping on the hardware. This involves calling the function 
mana_set_bw_clamp() which internally calls a HWC command 
to the hardware to set the speed.

The minimum supported bandwidth is 100 Mbps, and only multiples
of 100 Mbps are supported by the hardware. The speed will be reset to
maximum bandwidth supported by the SKU, when the HTB leaf is deleted.

Also add speed support in mana_get_link_ksettings to display speed in the
standard port information using ethtool. This involves calling
mana_query_link_config(), which internally sends a HWC command to
the hardware to query speed information.

Note that this feature is not supported by all hardware.

Erni Sri Satya Vennela (3):
  net: mana: Add speed support in mana_get_link_ksettings
  net: mana: Add sched HTB offload support
  net: mana: Handle unsupported HWC commands

 .../net/ethernet/microsoft/mana/hw_channel.c  |   4 +
 drivers/net/ethernet/microsoft/mana/mana_en.c | 191 ++++++++++++++++++
 .../ethernet/microsoft/mana/mana_ethtool.c    |   6 +
 include/net/mana/mana.h                       |  36 ++++
 4 files changed, 237 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: Shradha Gupta <shradhagupta@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@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 2bac6be8f6a0..ba550fc7ece0 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -1156,6 +1156,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_resp resp = {};
+	struct mana_query_link_config_req req = {};
+	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_mbps;
+	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 0f78065de8fe..63193613c185 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_mbps;
+	u8 qos_unconfigured;
+	u8 reserved[3];
+}; /* HW DATA */
+
 /* Query Device Configuration */
 struct mana_query_device_cfg_req {
 	struct gdma_req_hdr hdr;
-- 
2.34.1

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

[Zhu Yanjun]

On 17.04.25 09:57, Erni Sri Satya Vennela wrote:
> 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: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@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 2bac6be8f6a0..ba550fc7ece0 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -1156,6 +1156,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_resp resp = {};
> +	struct mana_query_link_config_req req = {};
> +	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;

EPROTO means Protocol error. Thus, ENOTSUPP or EPROTONOSUPPORT is better?

Zhu Yanjun

> +		goto out;
> +	}
> +
> +	if (resp.qos_unconfigured) {
> +		err = -EINVAL;
> +		goto out;
> +	}
> +	apc->speed = resp.link_speed_mbps;
> +	return 0;
> +
> +out:
> +	return err;
> +}
> +

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

[Erni Sri Satya Vennela]

On Thu, Apr 17, 2025 at 03:38:26PM +0200, Zhu Yanjun wrote:
> On 17.04.25 09:57, Erni Sri Satya Vennela wrote:
> >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: Shradha Gupta <shradhagupta@linux.microsoft.com>
> >Reviewed-by: Haiyang Zhang <haiyangz@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 2bac6be8f6a0..ba550fc7ece0 100644
> >--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> >+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> >@@ -1156,6 +1156,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_resp resp = {};
> >+	struct mana_query_link_config_req req = {};
> >+	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;
> 
> EPROTO means Protocol error. Thus, ENOTSUPP or EPROTONOSUPPORT is better?
> 
> Zhu Yanjun
Thank you for the suggestion, Zhu Yanjun. I will also make this change
in mana_set_bw_clamp(). This update will be included in the next version
of the patch.
> 
> >+		goto out;
> >+	}
> >+
> >+	if (resp.qos_unconfigured) {
> >+		err = -EINVAL;
> >+		goto out;
> >+	}
> >+	apc->speed = resp.link_speed_mbps;
> >+	return 0;
> >+
> >+out:
> >+	return err;
> >+}
> >+

[PATCH 2/3] net: mana: Add sched HTB offload support

[Erni Sri Satya Vennela]

Introduce support for HTB qdisc offload in the mana ethernet
controller. This controller can offload only one HTB leaf.
The HTB leaf supports clamping the bandwidth for egress traffic.
It uses the function mana_set_bw_clamp(), which internally calls
a HWC command to the hardware to set the speed.

The minimum bandwidth is 100 Mbps, and only multiples of 100 Mbps
are handled by the hardware. When the HTB leaf/root is deleted,
the speed will be reset to maximum bandwidth supported by the SKU.

This feature is not supported by all hardware.

Steps to configure speed:

Add the root qdisc
$tc qdisc add dev enP30832s1 root handle 1: htb offload

Add the class with required rate
$tc class add dev enP30832s1 parent 1: classid 1:1 htb rate 1000mbit

Display class details
$tc class show dev enP30832s1 classid 1:1
>class htb 1:1 root prio 0 rate 1Gbit ceil 1Gbit 
>burst 1375b cburst 1375b  offload

Display port information using ethtool
$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: 1000Mb/s
        Duplex: Full
        Auto-negotiation: off
        Port: Other
        PHYAD: 0
        Transceiver: internal
        Link detected: yes

Delete class
$tc class del dev enP30832s1 classid 1:1

Delete root qdisc (If used alone, also deletes the attached class)
$tc qdisc del dev enP30832s1 root

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

diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index ba550fc7ece0..5b62f1443716 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -16,10 +16,13 @@
 #include <net/netdev_lock.h>
 #include <net/page_pool/helpers.h>
 #include <net/xdp.h>
+#include <net/pkt_cls.h>
 
 #include <net/mana/mana.h>
 #include <net/mana/mana_auxiliary.h>
 
+#define MIN_BANDWIDTH 100
+
 static DEFINE_IDA(mana_adev_ida);
 
 static int mana_adev_idx_alloc(void)
@@ -719,6 +722,99 @@ static int mana_change_mtu(struct net_device *ndev, int new_mtu)
 	return err;
 }
 
+static int mana_tc_htb_handle_leaf_queue(struct mana_port_context *mpc,
+					 struct tc_htb_qopt_offload *opt,
+					 bool alloc)
+{
+	u32 rate, old_speed;
+	int err;
+
+	if (opt->command == TC_HTB_LEAF_ALLOC_QUEUE) {
+		if (opt->parent_classid != TC_HTB_CLASSID_ROOT) {
+			NL_SET_ERR_MSG_MOD(opt->extack, "invalid parent classid");
+			return -EINVAL;
+		} else if (mpc->classid) {
+			NL_SET_ERR_MSG_MOD(opt->extack, "Cannot create multiple classes");
+			return -EOPNOTSUPP;
+		}
+		mpc->classid = opt->classid;
+	}
+
+	rate = div_u64(opt->rate, 1000) << 3; //Convert Bps to Kbps
+	rate = div_u64(rate, 1000);	      //Convert Kbps to Mbps
+
+	/*Get current speed*/
+	err = mana_query_link_cfg(mpc);
+	old_speed = (err) ? SPEED_UNKNOWN : mpc->speed;
+
+	if (!err) {
+		if (alloc) {
+			/*Support only multiples of 100Mbps for rate parameter*/
+			rate = max(rate, MIN_BANDWIDTH);
+			rate = rounddown(rate, MIN_BANDWIDTH);
+
+			err = mana_set_bw_clamp(mpc, rate, TRI_STATE_TRUE);
+			mpc->speed = (err) ? old_speed : rate;
+		} else {
+			err = mana_set_bw_clamp(mpc, rate, TRI_STATE_FALSE);
+			mpc->classid = (err) ? : 0;
+		}
+	}
+
+	return err;
+}
+
+static int mana_create_tc_htb(struct mana_port_context *mpc)
+{
+	int err;
+
+	/*Check for hardware support*/
+	err = mana_query_link_cfg(mpc);
+	if (err == -EINVAL)
+		netdev_info(mpc->ndev, "QoS is not configured yet\n");
+
+	return err;
+}
+
+static int mana_tc_setup_htb(struct mana_port_context *mpc,
+			     struct tc_htb_qopt_offload *opt)
+{
+	int err;
+
+	switch (opt->command) {
+	case TC_HTB_CREATE:
+		err = mana_create_tc_htb(mpc);
+		return err;
+	case TC_HTB_NODE_MODIFY:
+	case TC_HTB_LEAF_ALLOC_QUEUE:
+		err = mana_tc_htb_handle_leaf_queue(mpc, opt, 1);
+		return err;
+	case TC_HTB_DESTROY:
+	case TC_HTB_LEAF_DEL:
+	case TC_HTB_LEAF_DEL_LAST:
+	case TC_HTB_LEAF_DEL_LAST_FORCE:
+		return mana_tc_htb_handle_leaf_queue(mpc, opt, 0);
+	case TC_HTB_LEAF_QUERY_QUEUE:
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
+static int mana_setup_tc(struct net_device *dev, enum tc_setup_type type,
+			 void *type_data)
+{
+	struct mana_port_context *mpc = netdev_priv(dev);
+
+	switch (type) {
+	case TC_SETUP_QDISC_HTB:
+		return mana_tc_setup_htb(mpc, type_data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static const struct net_device_ops mana_devops = {
 	.ndo_open		= mana_open,
 	.ndo_stop		= mana_close,
@@ -729,6 +825,7 @@ static const struct net_device_ops mana_devops = {
 	.ndo_bpf		= mana_bpf,
 	.ndo_xdp_xmit		= mana_xdp_xmit,
 	.ndo_change_mtu		= mana_change_mtu,
+	.ndo_setup_tc		= mana_setup_tc,
 };
 
 static void mana_cleanup_port_context(struct mana_port_context *apc)
@@ -1198,6 +1295,46 @@ int mana_query_link_cfg(struct mana_port_context *apc)
 	return err;
 }
 
+int mana_set_bw_clamp(struct mana_port_context *apc, u32 speed,
+		      int enable_clamping)
+{
+	struct mana_set_bw_clamp_resp resp = {};
+	struct mana_set_bw_clamp_req req = {};
+	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 = enable_clamping;
+
+	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,
@@ -2942,6 +3079,7 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
 	ndev->hw_features |= NETIF_F_RXCSUM;
 	ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
 	ndev->hw_features |= NETIF_F_RXHASH;
+	ndev->hw_features |= NETIF_F_HW_TC;
 	ndev->features = ndev->hw_features | NETIF_F_HW_VLAN_CTAG_TX |
 			 NETIF_F_HW_VLAN_CTAG_RX;
 	ndev->vlan_features = ndev->features;
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 63193613c185..69687dfe7540 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -469,6 +469,8 @@ struct mana_port_context {
 	u16 port_idx;
 
 	u32 speed;
+	/*HTB class parameters*/
+	u16 classid;
 
 	bool port_is_up;
 	bool port_st_save; /* Saved port state */
@@ -500,6 +502,8 @@ 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 enable_clamping);
 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 +531,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 +553,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;
+	u8 qos_unconfigured;
+	u8 reserved[7];
+}; /* HW DATA */
+
 /* Query Device Configuration */
 struct mana_query_device_cfg_req {
 	struct gdma_req_hdr hdr;
-- 
2.34.1

Re: [PATCH 2/3] net: mana: Add sched HTB offload support

[Stephen Hemminger]

On Thu, 17 Apr 2025 00:57:09 -0700
Erni Sri Satya Vennela <ernis@linux.microsoft.com> wrote:

> Introduce support for HTB qdisc offload in the mana ethernet
> controller. This controller can offload only one HTB leaf.
> The HTB leaf supports clamping the bandwidth for egress traffic.
> It uses the function mana_set_bw_clamp(), which internally calls
> a HWC command to the hardware to set the speed.

A single leaf is just Token Bucket Filter (TBF).
Are you just trying to support some vendor config?

Re: [PATCH 2/3] net: mana: Add sched HTB offload support

[Erni Sri Satya Vennela]

On Thu, Apr 17, 2025 at 08:10:53AM -0700, Stephen Hemminger wrote:
> On Thu, 17 Apr 2025 00:57:09 -0700
> Erni Sri Satya Vennela <ernis@linux.microsoft.com> wrote:
> 
> > Introduce support for HTB qdisc offload in the mana ethernet
> > controller. This controller can offload only one HTB leaf.
> > The HTB leaf supports clamping the bandwidth for egress traffic.
> > It uses the function mana_set_bw_clamp(), which internally calls
> > a HWC command to the hardware to set the speed.
> 
> A single leaf is just Token Bucket Filter (TBF).
> Are you just trying to support some vendor config?
TBF does not support hardware offloading.
Out of the qdiscs that support hardware offloading, I have chosen HTB
which can help set bandwidth for the MANA NIC.

Re: [PATCH 2/3] net: mana: Add sched HTB offload support

[Jakub Kicinski]

On Thu, 17 Apr 2025 12:47:27 -0700 Erni Sri Satya Vennela wrote:
> > A single leaf is just Token Bucket Filter (TBF).
> > Are you just trying to support some vendor config?  
> TBF does not support hardware offloading.

Did you take a look at net_shapers? Will it not let you set a global
config the way you intend?

Re: [PATCH 2/3] net: mana: Add sched HTB offload support

[Erni Sri Satya Vennela]

On Thu, Apr 17, 2025 at 05:00:52PM -0700, Jakub Kicinski wrote:
> On Thu, 17 Apr 2025 12:47:27 -0700 Erni Sri Satya Vennela wrote:
> > > A single leaf is just Token Bucket Filter (TBF).
> > > Are you just trying to support some vendor config?  
> > TBF does not support hardware offloading.
> 
> Did you take a look at net_shapers? Will it not let you set a global
> config the way you intend?
Yes, Jakub. I have reviewed net-shapers and noted that it is not
integrated into the kernel like tc. I mean there isn't a standard,
general-purpose command for net-shaper in Linux. It is used by other
tools or potentially device-specific drivers that want to leverage the
NIC's hardware shaping capabilities.

To configure shaping with net-shapers, users would need to execute a
command similar to:

./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml
--do set --json '{"ifindex":'$IFINDEX', 
		  "shaper": {"handle": 
			    {"scope": "node", "id":'$NODEID' },
		  "bw-max": 2000000}}'

Ref: https://lore.kernel.org/all/cover.1722357745.git.pabeni@redhat.com/

Given the simplicity of code implementation and ease of use for users in
writing commands, I opted for tc-htb.

Re: [PATCH 2/3] net: mana: Add sched HTB offload support

[Simon Horman]

On Fri, Apr 18, 2025 at 09:53:24AM -0700, Erni Sri Satya Vennela wrote:
> On Thu, Apr 17, 2025 at 05:00:52PM -0700, Jakub Kicinski wrote:
> > On Thu, 17 Apr 2025 12:47:27 -0700 Erni Sri Satya Vennela wrote:
> > > > A single leaf is just Token Bucket Filter (TBF).
> > > > Are you just trying to support some vendor config?  
> > > TBF does not support hardware offloading.
> > 
> > Did you take a look at net_shapers? Will it not let you set a global
> > config the way you intend?
> Yes, Jakub. I have reviewed net-shapers and noted that it is not
> integrated into the kernel like tc. I mean there isn't a standard,
> general-purpose command for net-shaper in Linux. It is used by other
> tools or potentially device-specific drivers that want to leverage the
> NIC's hardware shaping capabilities.
> 
> To configure shaping with net-shapers, users would need to execute a
> command similar to:
> 
> ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml
> --do set --json '{"ifindex":'$IFINDEX', 
> 		  "shaper": {"handle": 
> 			    {"scope": "node", "id":'$NODEID' },
> 		  "bw-max": 2000000}}'
> 
> Ref: https://lore.kernel.org/all/cover.1722357745.git.pabeni@redhat.com/
> 
> Given the simplicity of code implementation and ease of use for users in
> writing commands, I opted for tc-htb.

Hi Erni,

As someone who was involved with the design of net-shapers, I think it is
reasonable to instead use the Kernel API which appears to have been
designed specifically for this purpose: to control HW TX rate limiting.

If tooling isn't intuitive or otherwise doesn't meet user's needs
then that is something that can be addressed. But it's not a Kernel issue.

Re: [PATCH 2/3] net: mana: Add sched HTB offload support

[Erni Sri Satya Vennela]

On Thu, Apr 24, 2025 at 10:38:16AM +0100, Simon Horman wrote:
> On Fri, Apr 18, 2025 at 09:53:24AM -0700, Erni Sri Satya Vennela wrote:
> > On Thu, Apr 17, 2025 at 05:00:52PM -0700, Jakub Kicinski wrote:
> > > On Thu, 17 Apr 2025 12:47:27 -0700 Erni Sri Satya Vennela wrote:
> > > > > A single leaf is just Token Bucket Filter (TBF).
> > > > > Are you just trying to support some vendor config?  
> > > > TBF does not support hardware offloading.
> > > 
> > > Did you take a look at net_shapers? Will it not let you set a global
> > > config the way you intend?
> > Yes, Jakub. I have reviewed net-shapers and noted that it is not
> > integrated into the kernel like tc. I mean there isn't a standard,
> > general-purpose command for net-shaper in Linux. It is used by other
> > tools or potentially device-specific drivers that want to leverage the
> > NIC's hardware shaping capabilities.
> > 
> > To configure shaping with net-shapers, users would need to execute a
> > command similar to:
> > 
> > ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/shaper.yaml
> > --do set --json '{"ifindex":'$IFINDEX', 
> > 		  "shaper": {"handle": 
> > 			    {"scope": "node", "id":'$NODEID' },
> > 		  "bw-max": 2000000}}'
> > 
> > Ref: https://lore.kernel.org/all/cover.1722357745.git.pabeni@redhat.com/
> > 
> > Given the simplicity of code implementation and ease of use for users in
> > writing commands, I opted for tc-htb.
> 
> Hi Erni,
> 
> As someone who was involved with the design of net-shapers, I think it is
> reasonable to instead use the Kernel API which appears to have been
> designed specifically for this purpose: to control HW TX rate limiting.
> 
> If tooling isn't intuitive or otherwise doesn't meet user's needs
> then that is something that can be addressed. But it's not a Kernel issue.

Thank you, Simon and Jakub, for helping me understand the advantages of
using net-shapers over tc-htb. I will work on it and send the next
version using net-shapers in a few weeks, as I will be on leave for 
the next two weeks.

- Vennela

[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: Shradha Gupta <shradhagupta@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@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 1ba49602089b..58cfd247d8c5 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -871,6 +871,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 5b62f1443716..cb6a583b32d9 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -871,6 +871,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;
@@ -1269,6 +1272,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;
 	}
@@ -1313,6 +1320,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

[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

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