[PATCH 3/3] [TC/U32] Infrastructure for pretty printing

[PATCH 3/3] [TC/U32] Infrastructure for pretty printing

[jamal]

[-- Attachment #1: Type: text/plain, Size: 35 bytes --]

And last for now ..

cheers,
jamal

[-- Attachment #2: 0003-TC-U32-Infrastructure-for-pretty-printing.txt --]
[-- Type: text/plain, Size: 3811 bytes --]

[PATCH 3/3] [TC/U32] Infrastructure for pretty printing

This patch makes it easy to add pretty printers of different protocols.
For starters it makes use of ipv4 and raw printers.
Add more later ...

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
---
 tc/f_u32.c |   57 +++++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/tc/f_u32.c b/tc/f_u32.c
index db492fe..7ead576 100644
--- a/tc/f_u32.c
+++ b/tc/f_u32.c
@@ -21,6 +21,7 @@
 #include <arpa/inet.h>
 #include <string.h>
 #include <linux/if.h>
+#include <linux/if_ether.h>
 
 #include "utils.h"
 #include "tc_util.h"
@@ -789,27 +790,24 @@ static int parse_hashkey(int *argc_p, char ***argv_p, struct tc_u32_sel *sel)
 	return 0;
 }
 
-static void show_key(FILE *f, const struct tc_u32_key *key)
+static void print_ipv4(FILE *f, const struct tc_u32_key *key)
 {
 	char abuf[256];
 
-	if (!show_cooked)
-		goto raw;
-
 	switch (key->off) {
 	case 0:
 		switch (ntohl(key->mask)) {
 		case 0x0f000000:
-			fprintf(f, "\n ihl %u", ntohl(key->val) >> 24);
+			fprintf(f, "\n  match IP ihl %u", ntohl(key->val) >> 24);
 			return;
 		case 0x00ff0000:
-			fprintf(f, "\n dsfield %#x", ntohl(key->val) >> 16);
+			fprintf(f, "\n  match IP dsfield %#x", ntohl(key->val) >> 16);
 			return;
 		}
 		break;
 	case 8:
 		if (ntohl(key->mask) == 0x00ff0000) {
-			fprintf(f, "\n protocol %u", ntohl(key->val) >> 16);
+			fprintf(f, "\n  match IP protocol %d", ntohl(key->val) >> 16);
 			return;
 		}
 		break;
@@ -818,7 +816,7 @@ static void show_key(FILE *f, const struct tc_u32_key *key)
 			int bits = mask2bits(key->mask);
 			if (bits >= 0) {
 				fprintf(f, "\n  %s %s/%d", 
-					key->off == 12 ? "src" : "dst",
+					key->off == 12 ? "match IP src" : "match IP dst",
 					inet_ntop(AF_INET, &key->val,
 						  abuf, sizeof(abuf)),
 					bits);
@@ -830,23 +828,26 @@ static void show_key(FILE *f, const struct tc_u32_key *key)
 	case 20:
 		switch (ntohl(key->mask)) {
 		case 0x0000ffff:
-			fprintf(f, "\n  sport %u", 
+			fprintf(f, "\n  match sport %u",
 				ntohl(key->val) & 0xffff);
 			return;
 		case 0xffff0000:
-			fprintf(f, "\n  dport %u", 
+			fprintf(f, "\n  match dport %u",
 				ntohl(key->val) >> 16);
 			return;
 		case 0xffffffff:
-			fprintf(f, "\n  sport %u, dport %u", 
+			fprintf(f, "\n  match sport %u, match dport %u",
 				ntohl(key->val) & 0xffff,
 				ntohl(key->val) >> 16);
 
 			return;
 		}
+		/* XXX: Default print_raw */
 	}
+}
 
-raw:
+static void print_raw(FILE *f, const struct tc_u32_key *key)
+{
 	fprintf(f, "\n  match %08x/%08x at %s%d",
 		(unsigned int)ntohl(key->val),
 		(unsigned int)ntohl(key->mask),
@@ -854,7 +855,35 @@ raw:
 		key->off);
 }
 
-static int u32_parse_opt(struct filter_util *qu, char *handle, 
+static struct {
+	__u16 proto;
+	__u16 pad;
+	void (*pprinter)(FILE *f, const struct tc_u32_key *key);
+	} u32_pprinters[] = {
+		{0, 0, print_raw},
+		{ETH_P_IP, 0, print_ipv4},
+};
+
+static void show_keys(FILE *f, const struct tc_u32_key *key)
+{
+	int i = 0;
+
+	if (!show_cooked)
+		goto show_k;
+
+	for (i = 0; i < sizeof(u32_pprinters) / sizeof(u32_pprinters[0]); i++) {
+		if (u32_pprinters[i].proto == ntohs(f_proto)) {
+show_k:
+			u32_pprinters[i].pprinter(f, key);
+			return;
+		}
+	}
+
+	i = 0;
+	goto show_k;
+}
+
+static int u32_parse_opt(struct filter_util *qu, char *handle,
 			 int argc, char **argv, struct nlmsghdr *n)
 {
 	struct {
@@ -1129,7 +1158,7 @@ static int u32_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt,
 		if (sel->nkeys) {
 			int i;
 			for (i=0; i<sel->nkeys; i++) {
-				show_key(f, sel->keys + i);
+				show_keys(f, sel->keys + i);
 				if (show_stats && NULL != pf)
 					fprintf(f, " (success %llu ) ",
 						(unsigned long long) pf->kcnts[i]);
-- 
1.4.4.1.gaed4

Re: [PATCH 3/3] [TC/U32] Infrastructure for pretty printing

[Patrick McHardy]

jamal wrote:
>  		case 0x0f000000:
> -			fprintf(f, "\n ihl %u", ntohl(key->val) >> 24);
> +			fprintf(f, "\n  match IP ihl %u", ntohl(key->val) >> 24);
>  			return;
>  		case 0x00ff0000:
> -			fprintf(f, "\n dsfield %#x", ntohl(key->val) >> 16);
> +			fprintf(f, "\n  match IP dsfield %#x", ntohl(key->val) >> 16);
>  			return;
>  		}

So far the dump output can be cut-and-pasted and is
properly parsed again (useful for example for deletion).
Does that still work with pretty printing?

Re: [PATCH 3/3] [TC/U32] Infrastructure for pretty printing

[jamal]

On Mon, 2008-21-04 at 13:52 +0200, Patrick McHardy wrote:

> So far the dump output can be cut-and-pasted and is
> properly parsed again (useful for example for deletion).
> Does that still work with pretty printing?

great idea;-> 
It doesnt do that at the moment - all it tries to do is be equivalent to
the raw format (example the prefix "match" etc).
I think it is worth it though to try what you suggest and i will look
into it and provide an additional patch on top.
IINMistaken you are thinking of a save/restore feature?

The main challenge would be the cruft that dumps at the begining i.e the
sort of:
--
filter protocol ip pref 10 u32
filter protocol ip pref 10 u32 fh 802: ht divisor 1
filter protocol ip pref 10 u32 fh 802::800 order 2048 key ht 802 bkt 0
flowid 1:12
---

I am wondering in the case of -c if we should also go all the way and
dump just sufficient info to be able to reuse it as is to re-insert or
alternatively dump as it is today (for the lines above) and the restore
script would extract enough details out of the above.
Thoughts?

cheers,
jamal

Re: [PATCH 3/3] [TC/U32] Infrastructure for pretty printing

[Patrick McHardy]

jamal wrote:
> On Mon, 2008-21-04 at 13:52 +0200, Patrick McHardy wrote:
> 
>> So far the dump output can be cut-and-pasted and is
>> properly parsed again (useful for example for deletion).
>> Does that still work with pretty printing?
> 
> great idea;-> 
> It doesnt do that at the moment - all it tries to do is be equivalent to
> the raw format (example the prefix "match" etc).
> I think it is worth it though to try what you suggest and i will look
> into it and provide an additional patch on top.
> IINMistaken you are thinking of a save/restore feature?

Something like that. But it used to already work the
way I described, at least for all parts where I tested
it so far (IIRC routes/rules/addresses/qdiscs/classes/filters).

> The main challenge would be the cruft that dumps at the begining i.e the
> sort of:
> --
> filter protocol ip pref 10 u32
> filter protocol ip pref 10 u32 fh 802: ht divisor 1
> filter protocol ip pref 10 u32 fh 802::800 order 2048 key ht 802 bkt 0
> flowid 1:12
> ---

Yes, the first part needs to be stripped (filter/qdisc/class/...).
I usually do something like

"ip link/route/addr del <paste dump without first word here>".

Similar for tc.

> I am wondering in the case of -c if we should also go all the way and
> dump just sufficient info to be able to reuse it as is to re-insert or
> alternatively dump as it is today (for the lines above) and the restore
> script would extract enough details out of the above.
> Thoughts?


It would be nice to dump it in a format that can be piped
into batch mode without further changes, so it would have
to insert "add" between "filter" and "protocol".

Re: [PATCH 3/3] [TC/U32] Infrastructure for pretty printing

[jamal]

On Mon, 2008-21-04 at 15:29 +0200, Patrick McHardy wrote:

> Something like that. But it used to already work the
> way I described, at least for all parts where I tested
> it so far (IIRC routes/rules/addresses/qdiscs/classes/filters).

ok, missed that - i dont recall ever using it like that. But i will test
and make sure it works. Now i get more of what you are saying with the
env variables.

> Yes, the first part needs to be stripped (filter/qdisc/class/...).
> I usually do something like
> 
> "ip link/route/addr del <paste dump without first word here>".
> 
> Similar for tc.

Ok, just tried it on my laptop; u32 (and maybe the rest of the filters)
doesnt work in this mode. Essentially inputting is of the form
"match u{8,16,32} value mask" and output is of form "match
32bitval/32bitmask"
I think it should be easier to do with cooked mode and i will look into
it.

> It would be nice to dump it in a format that can be piped
> into batch mode without further changes, so it would have
> to insert "add" between "filter" and "protocol".

An additional processing may be needed in the save script.

cheers,
jamal

Re: [PATCH 3/3] [TC/U32] Infrastructure for pretty printing

[jamal]

On Mon, 2008-21-04 at 09:59 -0400, jamal wrote:
> On Mon, 2008-21-04 at 15:29 +0200, Patrick McHardy wrote:
> 
> > Something like that. But it used to already work the
> > way I described, at least for all parts where I tested
> > it so far (IIRC routes/rules/addresses/qdiscs/classes/filters).
> 
> ok, missed that - i dont recall ever using it like that. But i will test
> and make sure it works. Now i get more of what you are saying with the
> env variables.
> 
> > Yes, the first part needs to be stripped (filter/qdisc/class/...).
> > I usually do something like
> > 
> > "ip link/route/addr del <paste dump without first word here>".
> > 
> > Similar for tc.
> 
> Ok, just tried it on my laptop; u32 (and maybe the rest of the filters)
> doesnt work in this mode. Essentially inputting is of the form
> "match u{8,16,32} value mask" and output is of form "match
> 32bitval/32bitmask"

Actually it is easy to fix but it may break existing scripts again.
Certainly will break mine. The simple solution would be:
Change output which goes like:

-------
match 00000000/00ff0000 at 4
-----

to

---------
match 00000000 00ff0000 at 4
---

So maybe just make sure the cooked mode works in the future and ignore
all the rest. Or for the raw format (when someone explicitly calls it
with -raw) output it with the space instead of /.

cheers,
jamal