net: shell: iface: Add link speed settings command #90374

peileeli · 2025-05-23T07:57:30Z

Add this to set link speed through net_shell.
Current zephyr version doesn't have user interface to change speed, hence adding this to accomodate it.

Able to change link speed using command below:
net iface set_link
eg: net iface set_link 1 100

github-actions · 2025-05-23T07:58:11Z

Hello @peileeli, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

subsys/net/lib/shell/iface.c

jukkar · 2025-05-23T09:35:23Z

subsys/net/lib/shell/iface.c

+               PR_WARNING("No such interface in index %d\n", idx);
+               return -ENODEV;
+       }
+


We need to make sure that the interface is Ethernet one. So something like this (see similar checks in other parts of this file).

if (net_if_l2(iface) != &NET_L2_GET_NAME(ETHERNET)) { PR_WARNING("Interface %idx is not Ethernet type\n", idx); return -EINVAL; }

jukkar · 2025-05-23T09:38:43Z

subsys/net/lib/shell/iface.c

@@ -27,6 +27,7 @@ LOG_MODULE_DECLARE(net_shell);
 #endif


Please remove the subsys: prefix from commit subject, it is not needed.

@Vijayakannan , i have done the amendment accordingly

rlubos · 2025-05-26T08:19:06Z

subsys/net/lib/shell/iface.c

@@ -841,6 +889,9 @@ SHELL_STATIC_SUBCMD_SET_CREATE(net_cmd_iface,
 	SHELL_CMD(default, IFACE_DYN_CMD,
 		  "'net iface default [<index>]' displays or sets the default network interface.",
 		  cmd_net_default_iface),
+	SHELL_CMD(set_link, IFACE_DYN_CMD,
+		  "'net iface set_link <index> <Speed 10/100/1000>' sets link speed for the network interface.",


The help string is missing 2500/5000 speed variants.

Vijayakannan

please amend the commit and PR tittle to reflect exactly.

subsys/net/lib/shell/iface.c

@@ -27,6 +27,7 @@ LOG_MODULE_DECLARE(net_shell);
 #endif


maass-hamburg

The preferred way to change the Ethernet speed is, to go over the phy and not directly over the Ethernet driver. When the link is then changed by the phy will inform the Ethernet driver via the callback. The api, that is used here, is not implemented in most Ethernet drivers. I'm actually in favor of removing/deprecating this part of the api.

clamattia · 2025-05-26T11:47:11Z

The preferred way to change the Ethernet speed is, to go over the phy and not directly over the Ethernet driver. When the link is then changed by the phy will inform the Ethernet driver via the callback. The api, that is used here, is not implemented in most Ethernet drivers. I'm actually in favor of removing/deprecating this part of the api.

I tend to agree with this. To me it is unclear when which api should be used. For example, there is a lot of redundancy with get_phy and then using the phy interface vs. using the ethernet interface.

maass-hamburg · 2025-05-26T11:49:52Z

The preferred way to change the Ethernet speed is, to go over the phy and not directly over the Ethernet driver. When the link is then changed by the phy will inform the Ethernet driver via the callback. The api, that is used here, is not implemented in most Ethernet drivers. I'm actually in favor of removing/deprecating this part of the api.

I tend to agree with this. To me it is unclear when which api should be used. For example, there is a lot of redundancy with get_phy and then using the phy interface vs. using the ethernet interface.

That's mainly from the days when there was not separate phy api and they integrated the phy code into the ethernet driver/days.

peileeli · 2025-05-27T02:28:33Z

The preferred way to change the Ethernet speed is, to go over the phy and not directly over the Ethernet driver. When the link is then changed by the phy will inform the Ethernet driver via the callback. The api, that is used here, is not implemented in most Ethernet drivers. I'm actually in favor of removing/deprecating this part of the api.

@maass-hamburg what if user wants to change speed using shell?
Do you mean that shell directly call to phy_configure_link() instead of ethernet driver?

maass-hamburg · 2025-05-27T04:55:31Z

The preferred way to change the Ethernet speed is, to go over the phy and not directly over the Ethernet driver. When the link is then changed by the phy will inform the Ethernet driver via the callback. The api, that is used here, is not implemented in most Ethernet drivers. I'm actually in favor of removing/deprecating this part of the api.

@maass-hamburg what if user wants to change speed using shell?
Do you mean that shell directly call to phy_configure_link() instead of ethernet driver?

Yes exactly. You can use it together with the 'get_phy' from the Ethernet api.

peileeli · 2025-05-27T04:58:42Z

The preferred way to change the Ethernet speed is, to go over the phy and not directly over the Ethernet driver. When the link is then changed by the phy will inform the Ethernet driver via the callback. The api, that is used here, is not implemented in most Ethernet drivers. I'm actually in favor of removing/deprecating this part of the api.

@maass-hamburg what if user wants to change speed using shell?
Do you mean that shell directly call to phy_configure_link() instead of ethernet driver?

Yes exactly. You can use it together with the 'get_phy' from the Ethernet api.

ok, let me try that approach!

peileeli · 2025-05-30T11:36:08Z

@maass-hamburg @jukkar @rlubos
I have made amendment accordingly.
[1] use get_phy()
[2] use phy_configure_link()
[3] modify the printout to support (10/100/1000/2500/5000)
[4] check iface

Vijayakannan

few nitpicks.

subsys/net/lib/shell/iface.c

+
+	iface = net_if_get_by_index(idx);
+	if (net_if_l2(iface) != &NET_L2_GET_NAME(ETHERNET)) {
+		PR_WARNING("Interface %idx is not Ethernet type\n", idx);


peileeli · 2025-06-02T06:53:35Z

subsys/net/lib/shell/iface.c

@@ -841,6 +896,9 @@ SHELL_STATIC_SUBCMD_SET_CREATE(net_cmd_iface,
 	SHELL_CMD(default, IFACE_DYN_CMD,
 		  "'net iface default [<index>]' displays or sets the default network interface.",
 		  cmd_net_default_iface),
+	SHELL_CMD(set_link, IFACE_DYN_CMD,
+		  "'net iface set_link <index> <Speed 10/100/1000/2500/5000>' sets link speed for the network interface.",


i think for the message i will remain but split to 100 characters

subsys/net/lib/shell/iface.c

@@ -27,6 +27,7 @@ LOG_MODULE_DECLARE(net_shell);
 #endif


subsys/net/lib/shell/iface.c

+
+	phy_configure_link(phy_dev, speed);
+
+	return 0;


maass-hamburg · 2025-06-02T08:32:34Z

subsys/net/lib/shell/iface.c

+#if !defined(CONFIG_NET_L2_ETHERNET_MGMT)
+	PR_WARNING("Unsupported command, please enable CONFIG_NET_L2_ETHERNET_MGMT\n");
+	return -ENOEXEC;
+#else


CONFIG_NET_L2_ETHERNET_MGMT is not needed

maass-hamburg · 2025-06-02T08:40:28Z

subsys/net/lib/shell/iface.c

+		return -ENOTSUP;
+	}
+
+	return phy_configure_link(phy_dev, speed);


this also supports multiple speeds, maybe we could have something like
net iface set_link 1 100 10, which would allow LINK_FULL_100BASE and LINK_FULL_10BASE

this also supports multiple speeds, maybe we could have something like net iface set_link 1 100 10, which would allow LINK_FULL_100BASE and LINK_FULL_10BASE

The multiple speed command argument will overlap with the duplex command recommended below. So, can we interchange the command format as shown below to accommodate Multiple speed and Duplex.?

net iface set_link 1 half 100 10

we could loop through the arguments with argc as the max and check if the next argument exists and check if it is not a number then increase the iterator.

net iface set_link 1 100 100 half 10 ( = LINK_FULL_100BASE | LINK_HALF_100BASE | LINK_FULL_10BASE)
should be possible

ok, let me try this.

maass-hamburg · 2025-06-02T08:46:30Z

subsys/net/lib/shell/iface.c

+	case 10:
+		speed = LINK_FULL_10BASE;
+		break;


there are also half duplex variants, maybe check if there is a half or h in the next argument, so
net iface set_link 1 10 half would use LINK_HALF_10BASE.

half is very rarely used, so I would also be fine without it (Optional requirement). Could still be added later.

there are also half duplex variants, maybe check if there is a half or h in the next argument, so net iface set_link 1 10 half would use LINK_HALF_10BASE.

I agree with this overloading idea. Perhaps take note of "ETHERNET_CONFIG_TYPE_DUPLEX" for setting the existing duplex option, which might require to deprecate.

"ETHERNET_CONFIG_TYPE_DUPLEX" is irrelevant, when going directly over the phy, the phy will inform the ethernet driver. also see #90652

Add this to set link speed through net_shell. Current zephyr version doesn't have user interface to change speed, hence adding this to accomodate it. Able to change link speed using command below: net iface set_link <iface idx> <speed> <optional:duplex> eg: net iface set_link 1 100 half net iface set_link 1 10 full net iface set_link 1 1000 Able to set multiple link speed like below: net iface set_link <iface idx> <speed1> <speed2> <optional:duplex> eg: net iface set_link 1 10 100 half net iface set_link 1 1000 full 10 Signed-off-by: Ling Pei Lee <pei.lee.ling@intel.com>

peileeli · 2025-06-03T11:46:03Z

Changes Added:
[1] comments amendment
[2] Add multiple link speed support
[3] Add half/full duplex support

maass-hamburg · 2025-06-03T12:07:16Z

subsys/net/lib/shell/iface.c

+	for (int j = 2; j < argc; j++) {
+		if (isdigit(argv[j][0]) == 0) {
+			if (strcmp(argv[j], "half") == 0) {
+				full_duplex = false;
+				break;
+			}
+		}
+	}


this should be inside the next loop, so it could be set after each speed, also I would just check if the first letter is h, is more performant and the user don't have to write half every time.

sonarqubecloud · 2025-06-03T12:16:59Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions bot added the area: Networking label May 23, 2025

github-actions bot requested review from jukkar, pdgendt, rlubos and ssharks May 23, 2025 07:58

github-actions bot assigned rlubos and jukkar May 23, 2025

jukkar reviewed May 23, 2025

View reviewed changes

peileeli changed the title ~~subsys: net: shell: Enable set_config in net_shell~~ net: shell: Enable set_config in net_shell May 23, 2025

peileeli force-pushed the dev/peileeli/set_link_cmd branch from e7cc324 to fc6fb1c Compare May 26, 2025 03:24

github-actions bot added the area: Ethernet label May 26, 2025

github-actions bot requested review from clamattia, decsny, lmajewski and maass-hamburg May 26, 2025 03:25

peileeli force-pushed the dev/peileeli/set_link_cmd branch from fc6fb1c to 38f9418 Compare May 26, 2025 04:30

rlubos reviewed May 26, 2025

View reviewed changes

Vijayakannan reviewed May 26, 2025

View reviewed changes

subsys/net/lib/shell/iface.c

@@ -27,6 +27,7 @@ LOG_MODULE_DECLARE(net_shell);

#endif

This comment was marked as resolved.

Sign in to view

maass-hamburg requested changes May 26, 2025

View reviewed changes

peileeli force-pushed the dev/peileeli/set_link_cmd branch 2 times, most recently from 8031b4a to adb7b69 Compare May 30, 2025 11:32

Vijayakannan reviewed May 31, 2025

View reviewed changes

peileeli changed the title ~~net: shell: Enable set_config in net_shell~~ net: shell: iface: Add link speed settings command Jun 2, 2025

peileeli force-pushed the dev/peileeli/set_link_cmd branch from adb7b69 to 54aab3a Compare June 2, 2025 07:00

maass-hamburg requested changes Jun 2, 2025

View reviewed changes

peileeli force-pushed the dev/peileeli/set_link_cmd branch from 54aab3a to 3d375f9 Compare June 3, 2025 11:44

maass-hamburg requested changes Jun 3, 2025

View reviewed changes

net: shell: iface: Add link speed settings command #90374

Are you sure you want to change the base?

net: shell: iface: Add link speed settings command #90374

Conversation

peileeli commented May 23, 2025

Uh oh!

github-actions bot commented May 23, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vijayakannan left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

maass-hamburg left a comment

Choose a reason for hiding this comment

Uh oh!

clamattia commented May 26, 2025

Uh oh!

maass-hamburg commented May 26, 2025

Uh oh!

peileeli commented May 27, 2025

Uh oh!

maass-hamburg commented May 27, 2025

Uh oh!

peileeli commented May 27, 2025

Uh oh!

peileeli commented May 30, 2025

Uh oh!

Vijayakannan left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peileeli commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 3, 2025

Quality Gate passed

Uh oh!

Uh oh!