diff mbox series

[iproute2] tc: qdisc: filter qdisc's by parent/handle specification

Message ID 20200615192928.6667-1-littlesmilingcloud@gmail.com
State Changes Requested
Delegated to: stephen hemminger
Headers show
Series [iproute2] tc: qdisc: filter qdisc's by parent/handle specification | expand

Commit Message

Anton Danilov June 15, 2020, 7:29 p.m. UTC 
There wasn't a way to get a qdisc info by handle or parent, only full
dump of qdisc's with following grep/sed usage.

The 'qdisc get' command have been added.

tc qdisc { show | get } [ dev STRING ] [ QDISC_ID ] [ invisible ]
  QDISC_ID := { root | ingress | handle QHANDLE | parent CLASSID }

This change doesn't require any changes in the kernel.

Signed-off-by: Anton Danilov <littlesmilingcloud@gmail.com>
---
 man/man8/tc.8 |   8 +++-
 tc/tc_qdisc.c | 109 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 82 insertions(+), 35 deletions(-)

Comments

Stephen Hemminger June 15, 2020, 11:12 p.m. UTC | #1 
On Mon, 15 Jun 2020 22:29:28 +0300
Anton Danilov <littlesmilingcloud@gmail.com> wrote:

> There wasn't a way to get a qdisc info by handle or parent, only full
> dump of qdisc's with following grep/sed usage.
> 
> The 'qdisc get' command have been added.
> 
> tc qdisc { show | get } [ dev STRING ] [ QDISC_ID ] [ invisible ]
>   QDISC_ID := { root | ingress | handle QHANDLE | parent CLASSID }
> 
> This change doesn't require any changes in the kernel.
> 
> Signed-off-by: Anton Danilov <littlesmilingcloud@gmail.com>

The idea looks good, but you need to conform to the kernel style.
Look at these checkpatch warnings.

ERROR: space required before the open parenthesis '('
#210: FILE: tc/tc_qdisc.c:397:
+			if(get_tc_classid(&handle, *argv)) {

WARNING: else is not generally useful after a break or return
#214: FILE: tc/tc_qdisc.c:401:
+				break;
+			} else {

ERROR: space required before the open parenthesis '('
#224: FILE: tc/tc_qdisc.c:411:
+			if(get_qdisc_handle(&handle, *argv)) {

WARNING: else is not generally useful after a break or return
#228: FILE: tc/tc_qdisc.c:415:
+				break;
+			} else {

ERROR: space prohibited before that close parenthesis ')'
#292: FILE: tc/tc_qdisc.c:478:
+	    || matches(*argv, "lst") == 0 || matches(*argv, "get") == 0 )

total: 3 errors, 2 warnings, 182 lines che
diff mbox series

Patch

diff --git a/man/man8/tc.8 b/man/man8/tc.8
index e8e0cd0f..8753088f 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -77,9 +77,13 @@  tc \- show / manipulate traffic control settings
 .B tc
 .RI "[ " OPTIONS " ]"
 .RI "[ " FORMAT " ]"
-.B qdisc show [ dev
+.B qdisc { show | get } [ dev
 \fIDEV\fR
-.B ]
+.B ] [ root | ingress | handle
+\fIQHANDLE\fR
+.B | parent
+\fICLASSID\fR
+.B ] [ invisible ]
 .P
 .B tc
 .RI "[ " OPTIONS " ]"
diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
index 181fe2f0..45b78462 100644
--- a/tc/tc_qdisc.c
+++ b/tc/tc_qdisc.c
@@ -35,11 +35,12 @@  static int usage(void)
 		"       [ ingress_block BLOCK_INDEX ] [ egress_block BLOCK_INDEX ]\n"
 		"       [ [ QDISC_KIND ] [ help | OPTIONS ] ]\n"
 		"\n"
-		"       tc qdisc show [ dev STRING ] [ ingress | clsact ] [ invisible ]\n"
+		"       tc qdisc { show | get } [ dev STRING ] [ QDISC_ID ] [ invisible ]\n"
 		"Where:\n"
 		"QDISC_KIND := { [p|b]fifo | tbf | prio | cbq | red | etc. }\n"
 		"OPTIONS := ... try tc qdisc add <desired QDISC_KIND> help\n"
-		"STAB_OPTIONS := ... try tc qdisc add stab help\n");
+		"STAB_OPTIONS := ... try tc qdisc add stab help\n"
+		"QDISC_ID := { root | ingress | handle QHANDLE | parent CLASSID }\n");
 	return -1;
 }
 
@@ -212,6 +213,8 @@  static int tc_qdisc_modify(int cmd, unsigned int flags, int argc, char **argv)
 }
 
 static int filter_ifindex;
+static __u32 filter_parent;
+static __u32 filter_handle;
 
 int print_qdisc(struct nlmsghdr *n, void *arg)
 {
@@ -235,6 +238,12 @@  int print_qdisc(struct nlmsghdr *n, void *arg)
 	if (filter_ifindex && filter_ifindex != t->tcm_ifindex)
 		return 0;
 
+	if (filter_handle && filter_handle != t->tcm_handle)
+		return 0;
+
+	if (filter_parent && filter_parent != t->tcm_parent)
+		return 0;
+
 	parse_rtattr_flags(tb, TCA_MAX, TCA_RTA(t), len, NLA_F_NESTED);
 
 	if (tb[TCA_KIND] == NULL) {
@@ -344,21 +353,68 @@  int print_qdisc(struct nlmsghdr *n, void *arg)
 
 static int tc_qdisc_list(int argc, char **argv)
 {
-	struct tcmsg t = { .tcm_family = AF_UNSPEC };
+	struct {
+		struct nlmsghdr n;
+		struct tcmsg t;
+		char buf[256];
+	} req = {
+		.n.nlmsg_type = RTM_GETQDISC,
+		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
+		.t.tcm_family = AF_UNSPEC,
+	};
+
 	char d[IFNAMSIZ] = {};
+	bool arg_error = false;
 	bool dump_invisible = false;
+	__u32 handle;
 
-	while (argc > 0) {
+	while (argc > 0 && !arg_error) {
 		if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
 			strncpy(d, *argv, sizeof(d)-1);
+		} else if (strcmp(*argv, "root") == 0) {
+			if (filter_parent || filter_handle) {
+				fprintf(stderr, "only one of parent/handle/root/ingress can be specified\n");
+				arg_error = true;
+				break;
+			}
+			filter_parent = TC_H_ROOT;
 		} else if (strcmp(*argv, "ingress") == 0 ||
 			   strcmp(*argv, "clsact") == 0) {
-			if (t.tcm_parent) {
-				fprintf(stderr, "Duplicate parent ID\n");
-				usage();
+			if (filter_parent || filter_handle) {
+				fprintf(stderr, "only one of parent/handle/root/ingress can be specified\n");
+				arg_error = true;
+				break;
+			}
+			filter_parent = TC_H_INGRESS;
+		} else if (matches(*argv, "parent") == 0) {
+			if (filter_parent || filter_handle) {
+				fprintf(stderr, "only one of parent/handle/root/ingress can be specified\n");
+				arg_error = true;
+				break;
+			}
+			NEXT_ARG();
+			if(get_tc_classid(&handle, *argv)) {
+				invarg("invalid parent ID", *argv);
+				arg_error = true;
+				break;
+			} else {
+				filter_parent = handle;
+			}
+		} else if (matches(*argv, "handle") == 0) {
+			if (filter_parent || filter_handle) {
+				fprintf(stderr, "only one of parent/handle/root/ingress can be specified\n");
+				arg_error = true;
+				break;
+			}
+			NEXT_ARG();
+			if(get_qdisc_handle(&handle, *argv)) {
+				invarg("invalid handle ID", *argv);
+				arg_error = true;
+				break;
+			} else {
+				filter_handle = handle;
 			}
-			t.tcm_parent = TC_H_INGRESS;
 		} else if (matches(*argv, "help") == 0) {
 			usage();
 		} else if (strcmp(*argv, "invisible") == 0) {
@@ -371,35 +427,26 @@  static int tc_qdisc_list(int argc, char **argv)
 		argc--; argv++;
 	}
 
+	if (arg_error) {
+		/* argument error message should be already displayed above */
+		return -1;
+	}
+
 	ll_init_map(&rth);
 
 	if (d[0]) {
-		t.tcm_ifindex = ll_name_to_index(d);
-		if (!t.tcm_ifindex)
+		req.t.tcm_ifindex = ll_name_to_index(d);
+		if (!req.t.tcm_ifindex)
 			return -nodev(d);
-		filter_ifindex = t.tcm_ifindex;
+		filter_ifindex = req.t.tcm_ifindex;
 	}
 
 	if (dump_invisible) {
-		struct {
-			struct nlmsghdr n;
-			struct tcmsg t;
-			char buf[256];
-		} req = {
-			.n.nlmsg_type = RTM_GETQDISC,
-			.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
-		};
-
-		req.t.tcm_family = AF_UNSPEC;
-
 		addattr(&req.n, 256, TCA_DUMP_INVISIBLE);
-		if (rtnl_dump_request_n(&rth, &req.n) < 0) {
-			perror("Cannot send dump request");
-			return 1;
-		}
+	}
 
-	} else if (rtnl_dump_request(&rth, RTM_GETQDISC, &t, sizeof(t)) < 0) {
-		perror("Cannot send dump request");
+	if (rtnl_dump_request_n(&rth, &req.n) < 0) {
+		perror("Cannot send request");
 		return 1;
 	}
 
@@ -427,12 +474,8 @@  int do_qdisc(int argc, char **argv)
 		return tc_qdisc_modify(RTM_NEWQDISC, NLM_F_REPLACE, argc-1, argv+1);
 	if (matches(*argv, "delete") == 0)
 		return tc_qdisc_modify(RTM_DELQDISC, 0,  argc-1, argv+1);
-#if 0
-	if (matches(*argv, "get") == 0)
-		return tc_qdisc_get(RTM_GETQDISC, 0,  argc-1, argv+1);
-#endif
 	if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0
-	    || matches(*argv, "lst") == 0)
+	    || matches(*argv, "lst") == 0 || matches(*argv, "get") == 0 )
 		return tc_qdisc_list(argc-1, argv+1);
 	if (matches(*argv, "help") == 0) {
 		usage();