| Message ID | 1742473341-15262-3-git-send-email-ernis@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for speed in MANA ethtool | expand |
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
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
> > $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
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
> -----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
> 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
> -----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
> 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
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;