Message-ID: <20250807180740.GA31890@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date: Thu, 7 Aug 2025 11:07:40 -0700
From: Erni Sri Satya Vennela <ernis@...ux.microsoft.com>
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: dsahern@...il.com, netdev@...r.kernel.org, haiyangz@...rosoft.com,
	shradhagupta@...ux.microsoft.com, ssengar@...rosoft.com,
	dipayanroy@...rosoft.com, ernis@...rosoft.com
Subject: Re: [PATCH iproute2-next v2] iproute2: Add 'netshaper' command to
 'ip link' for netdev shaping

On Tue, Aug 05, 2025 at 11:03:31AM -0700, Stephen Hemminger wrote:
> On Wed, 30 Jul 2025 02:21:56 -0700
> Erni Sri Satya Vennela <ernis@...ux.microsoft.com> wrote:
> 
> > Add support for the netshaper Generic Netlink
> > family to iproute2. Introduce a new subcommand to `ip link` for
> > configuring netshaper parameters directly from userspace.
> > 
> > This interface allows users to set shaping attributes (such as speed)
> > which are passed to the kernel to perform the corresponding netshaper
> > operation.
> > 
> > Example usage:
> > $ip link netshaper { set | get | delete } dev DEVNAME \
> >                    handle scope SCOPE id ID \
> >                    [ speed SPEED ]
> > 
> > Internally, this triggers a kernel call to apply the shaping
> > configuration to the specified network device.
> > 
> > Currently, the tool supports the following functionalities:
> > - Setting speed in Mbps, enabling bandwidth clamping for
> >   a network device that support netshaper operations.
> > - Deleting the current configuration.
> > - Querying the existing configuration.
> > 
> > Additional netshaper operations will be integrated into the tool
> > as per requirement.
> > 
> > This change enables easy and scriptable configuration of bandwidth
> > shaping for  devices that use the netshaper Netlink family.
> > 
> > Corresponding net-next patches:
> > 1) https://lore.kernel.org/all/cover.1728460186.git.pabeni@redhat.com/
> > 2) https://lore.kernel.org/lkml/1750144656-2021-1-git-send-email-ernis@linux.microsoft.com/
> > 
> > Install pkg-config and libmnl* packages to print any kernel extack
> > errors to stdout.
> > 
> > Signed-off-by: Erni Sri Satya Vennela <ernis@...ux.microsoft.com>
> > ---
> > Please add include/uapi/linux/net_shaper.h from kernel source tree
> > for this patch.
> > ---
> > Changes in v2:
> > * Use color coding for printing device name in stdout.
> > * Use clang-format to format the code inline.
> > * Use __u64 for speed_bps.
> > * Remove include/uapi/linux/netshaper.h file.
> > ---
> >  ip/Makefile           |   2 +-
> >  ip/iplink.c           |  12 +++
> >  ip/iplink_netshaper.c | 190 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 203 insertions(+), 1 deletion(-)
> >  create mode 100644 ip/iplink_netshaper.c
> 
> Good start but changes needed.
> 
> > 
> > diff --git a/ip/Makefile b/ip/Makefile
> > index 3535ba78..18218c3b 100644
> > --- a/ip/Makefile
> > +++ b/ip/Makefile
> > @@ -4,7 +4,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
> >      ipmaddr.o ipmonitor.o ipmroute.o ipprefix.o iptuntap.o iptoken.o \
> >      ipxfrm.o xfrm_state.o xfrm_policy.o xfrm_monitor.o iplink_dummy.o \
> >      iplink_ifb.o iplink_nlmon.o iplink_team.o iplink_vcan.o iplink_vxcan.o \
> > -    iplink_vlan.o link_veth.o link_gre.o iplink_can.o iplink_xdp.o \
> > +    iplink_vlan.o iplink_netshaper.o link_veth.o link_gre.o iplink_can.o iplink_xdp.o \
> >      iplink_macvlan.o ipl2tp.o link_vti.o link_vti6.o link_xfrm.o \
> >      iplink_vxlan.o tcp_metrics.o iplink_ipoib.o ipnetconf.o link_ip6tnl.o \
> >      link_iptnl.o link_gre6.o iplink_bond.o iplink_bond_slave.o iplink_hsr.o \
> > diff --git a/ip/iplink.c b/ip/iplink.c
> > index 59e8caf4..9da6e304 100644
> > --- a/ip/iplink.c
> > +++ b/ip/iplink.c
> > @@ -1509,6 +1509,15 @@ static void do_help(int argc, char **argv)
> >  		usage();
> >  }
> >  
> > +static int iplink_netshaper(int argc, char **argv)
> > +{
> > +	struct link_util *lu;
> > +
> > +	lu = get_link_kind("netshaper");
> > +
> > +	return lu->parse_opt(lu, argc, argv, NULL);
> > +}
> > +
> >  int do_iplink(int argc, char **argv)
> >  {
> >  	if (argc < 1)
> > @@ -1545,6 +1554,9 @@ int do_iplink(int argc, char **argv)
> >  	if (matches(*argv, "property") == 0)
> >  		return iplink_prop(argc-1, argv+1);
> >  
> > +	if (matches(*argv, "netshaper") == 0)
> > +		return iplink_netshaper(argc-1, argv+1);
> > +
> 
> Matches() can cause issues, prefer strcmp() for new code.

Okay, I'll make this change in the the next version.
> 
> >  	if (matches(*argv, "help") == 0) {
> >  		do_help(argc-1, argv+1);
> >  		return 0;
> > diff --git a/ip/iplink_netshaper.c b/ip/iplink_netshaper.c
> > new file mode 100644
> > index 00000000..af7d5e09
> > --- /dev/null
> > +++ b/ip/iplink_netshaper.c
> > @@ -0,0 +1,190 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * iplink_netshaper.c netshaper H/W shaping support
> > + *
> > + * Authors:        Erni Sri Satya Vennela <ernis@...ux.microsoft.com>
> > + */
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <linux/genetlink.h>
> > +#include <linux/netlink.h>
> > +#include <linux/rtnetlink.h>
> > +#include <uapi/linux/netshaper.h>
> > +#include "utils.h"
> > +#include "ip_common.h"
> > +#include "libgenl.h"
> > +
> > +/* netlink socket */
> > +static struct rtnl_handle gen_rth = { .fd = -1 };
> > +static int genl_family = -1;
> > +
> > +static void usage(void)
> > +{
> > +	fprintf(stderr,
> > +		"Usage:	ip link netshaper set dev DEVNAME handle scope HANDLE_SCOPE id HANDLE_ID speed SPEED\n"
> > +		"	ip link netshaper delete dev DEVNAME handle scope HANDLE_SCOPE id HANDLE_ID\n"
> > +		"	ip link netshaper get dev DEVNAME handle scope HANDLE_SCOPE id HANDLE_ID\n"
> > +		"Where:	DEVNAME		:= STRING\n"
> > +		"	HANDLE_SCOPE	:= { netdev | queue | node }\n"
> > +		"	HANDLE_ID	:= UINT\n"
> > +		"	SPEED		:= UINT (Mega bits per second)\n");
> > +
> > +	exit(-1);
> > +}
> > +
> > +static void print_netshaper_attrs(struct nlmsghdr *answer)
> > +{
> > +	struct genlmsghdr *ghdr = NLMSG_DATA(answer);
> > +	int len = answer->nlmsg_len - NLMSG_LENGTH(GENL_HDRLEN);
> > +	struct rtattr *tb[NET_SHAPER_A_MAX + 1] = {};
> > +	__u32 speed_mbps;
> > +	__u64 speed_bps;
> > +	int ifindex;
> > +
> > +	parse_rtattr(tb, NET_SHAPER_A_MAX,
> > +		     (struct rtattr *)((char *)ghdr + GENL_HDRLEN), len);
> > +
> > +	for (int i = 1; i <= NET_SHAPER_A_MAX; ++i) {
> > +		if (!tb[i])
> > +			continue;
> > +		switch (i) {
> > +		case NET_SHAPER_A_BW_MAX:
> > +			speed_bps = rta_getattr_u64(tb[i]);
> > +			speed_mbps = (speed_bps / 1000000);
> > +			print_uint(PRINT_ANY, "speed", "speed: %u mbps\n",
> > +				   speed_mbps);
> > +			break;
> > +		case NET_SHAPER_A_IFINDEX:
> > +			ifindex = rta_getattr_u32(tb[i]);
> > +			print_color_string(PRINT_ANY, COLOR_IFNAME, "dev",
> > +					   "dev: %s\n",
> > +					   ll_index_to_name(ifindex));
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +	}
> > +}
> > +
> > +static int do_cmd(int argc, char **argv, struct nlmsghdr *n, int cmd)
> > +{
> > +	GENL_REQUEST(req, 1024, genl_family, 0, NET_SHAPER_FAMILY_VERSION, cmd,
> > +		     NLM_F_REQUEST | NLM_F_ACK);
> > +
> > +	struct nlmsghdr *answer;
> > +	__u32 speed_mbps = 0;
> > +	__u64 speed_bps = 0;
> > +	int ifindex = -1;
> > +	int handle_scope = NET_SHAPER_SCOPE_UNSPEC;
> > +	__u32 handle_id = 0;
> > +	bool handle_present = false;
> > +	int err;
> > +
> > +	while (argc > 0) {
> > +		if (matches(*argv, "dev") == 0) {
> > +			NEXT_ARG();
> > +			ifindex = ll_name_to_index(*argv);
> > +		} else if (matches(*argv, "speed") == 0) {
> 
> Iproute2 no longer allows shortcut matches on new commands.
> Shortcuts have lead to lots of confusion where there are multiple matches possible.

Thanks for the clarification. I'll will update my usage accordingly to
avoid ambiguity.
> 
> > +			NEXT_ARG();
> > +			if (get_unsigned(&speed_mbps, *argv, 10)) {
> > +				fprintf(stderr, "Invalid speed value\n");
> > +				return -1;
> > +			}
> 
> Could you change this code to use the get_rate() in lib/utils_math.c
> That routine handles wide variety of suffixes.
> 
> > +			/*Convert Mbps to Bps*/
> 
> Won't need this if you use get_rate() or get_rate64

Okay I'll make the changes accordingly.
> 
> > +			speed_bps = (((__u64)speed_mbps) * 1000000);
> > +		} else if (matches(*argv, "handle") == 0) {
> > +			handle_present = true;
> > +			NEXT_ARG();
> > +			if (matches(*argv, "scope") == 0) {
> > +				NEXT_ARG();
> > +				if (matches(*argv, "netdev") == 0) {
> > +					handle_scope = NET_SHAPER_SCOPE_NETDEV;
> > +				} else if (matches(*argv, "queue") == 0) {
> > +					handle_scope = NET_SHAPER_SCOPE_QUEUE;
> > +				} else if (matches(*argv, "node") == 0) {
> > +					handle_scope = NET_SHAPER_SCOPE_NODE;
> > +				} else {
> > +					fprintf(stderr, "Invalid scope\n");
> > +					return -1;
> > +				}
> > +
> > +				NEXT_ARG();
> > +				if (matches(*argv, "id") == 0) {
> > +					NEXT_ARG();
> > +					if (get_unsigned(&handle_id, *argv, 10)) {
> > +						fprintf(stderr,
> > +							"Invalid handle id\n");
> > +						return -1;
> > +					}
> > +				}
> > +			}
> > +		} else {
> > +			fprintf(stderr, "What is \"%s\"\n", *argv);
> > +			usage();
> > +		}
> > +		argc--;
> > +		argv++;
> > +	}
> > +
> > +	if (ifindex == -1)
> > +		missarg("dev");
> > +
> > +	if (!handle_present)
> > +		missarg("handle");
> > +
> > +	if (cmd == NET_SHAPER_CMD_SET && speed_mbps == 0)
> > +		missarg("speed");
> > +
> > +	addattr32(&req.n, sizeof(req), NET_SHAPER_A_IFINDEX, ifindex);
> > +
> > +	struct rtattr *handle = addattr_nest(&req.n, sizeof(req),
> > +					     NET_SHAPER_A_HANDLE | NLA_F_NESTED);
> > +	addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_SCOPE, handle_scope);
> > +	addattr32(&req.n, sizeof(req), NET_SHAPER_A_HANDLE_ID, handle_id);
> > +	addattr_nest_end(&req.n, handle);
> > +
> > +	if (cmd == NET_SHAPER_CMD_SET)
> > +		addattr64(&req.n, sizeof(req), NET_SHAPER_A_BW_MAX, speed_bps);
> > +
> > +	err = rtnl_talk(&gen_rth, &req.n, &answer);
> > +	if (err < 0) {
> > +		printf("Kernel command failed: %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	if (cmd == NET_SHAPER_CMD_GET)
> > +		print_netshaper_attrs(answer);
> > +
> > +	return err;
> > +}
> > +
> > +static int netshaper_parse_opt(struct link_util *lu, int argc, char **argv,
> > +			       struct nlmsghdr *n)
> > +{
> > +	if (argc < 1)
> > +		usage();
> > +	if (matches(*argv, "help") == 0)
> > +		usage();
> > +
> > +	if (genl_init_handle(&gen_rth, NET_SHAPER_FAMILY_NAME, &genl_family))
> > +		exit(1);
> > +
> > +	if (matches(*argv, "set") == 0)
> > +		return do_cmd(argc - 1, argv + 1, n, NET_SHAPER_CMD_SET);
> > +
> > +	if (matches(*argv, "delete") == 0)
> > +		return do_cmd(argc - 1, argv + 1, n, NET_SHAPER_CMD_DELETE);
> > +
> > +	if (matches(*argv, "get") == 0)
> > +		return do_cmd(argc - 1, argv + 1, n, NET_SHAPER_CMD_GET);
> 
> No matches shortcuts
Okay.
> 
> > +
> > +	fprintf(stderr,
> > +		"Command \"%s\" is unknown, try \"ip link netshaper help\".\n",
> > +		*argv);
> > +	exit(-1);
> > +}
> > +
> > +struct link_util netshaper_link_util = {
> > +	.id = "netshaper",
> > +	.parse_opt = netshaper_parse_opt,
> > +};
> 
> Where is the print function. You should be able to print current shaper
> state?
The current shaper state can be retrieved from the get opearation like:
"ip link netshaper get dev <interface> handle scope <scope> id <id>"

Powered by blists - more mailing lists