From 5e5b3008d1fbb2ed74ac94153d6b5db37a903ae1 Mon Sep 17 00:00:00 2001 From: "Samudrala, Sridhar" Date: Wed, 8 Jun 2016 16:16:01 -0700 Subject: [PATCH 01/17] tc: f_u32: Add support for skip_hw and skip_sw flags On devices that support TC U32 offloads, these flags enable a filter to be added only to HW or only to SW. skip_sw and skip_hw are mutually exclusive flags. By default without any flags, the filter is added to both HW and SW, but no error checks are done in case of failure to add to HW. With skip-sw, failure to add to HW is treated as an error. Here is a sample script that adds 2 filters, one with skip_sw and the other with skip_hw flag. # add ingress qdisc tc qdisc add dev p4p1 ingress # enable hw tc offload. ethtool -K p4p1 hw-tc-offload on # add u32 filter with skip-sw flag. tc filter add dev p4p1 parent ffff: protocol ip prio 99 \ handle 800:0:1 u32 ht 800: flowid 800:1 \ skip-sw \ match ip src 192.168.1.0/24 \ action drop # add u32 filter with skip-hw flag. tc filter add dev p4p1 parent ffff: protocol ip prio 99 \ handle 800:0:2 u32 ht 800: flowid 800:2 \ skip-hw \ match ip src 192.168.2.0/24 \ action drop Signed-off-by: Sridhar Samudrala --- tc/f_u32.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tc/f_u32.c b/tc/f_u32.c index 9424dc37..b6ae4d28 100644 --- a/tc/f_u32.c +++ b/tc/f_u32.c @@ -30,7 +30,7 @@ extern int show_pretty; static void explain(void) { - fprintf(stderr, "Usage: ... u32 [ match SELECTOR ... ] [ link HTID ] [ classid CLASSID ]\n"); + fprintf(stderr, "Usage: ... u32 [ match SELECTOR ... ] [ link HTID ] [ classid CLASSID ] [skip-hw | skip-sw]\n"); fprintf(stderr, " [ action ACTION_SPEC ] [ offset OFFSET_SPEC ]\n"); fprintf(stderr, " [ ht HTID ] [ hashkey HASHKEY_SPEC ]\n"); fprintf(stderr, " [ sample SAMPLE ]\n"); @@ -993,6 +993,7 @@ static int u32_parse_opt(struct filter_util *qu, char *handle, int sample_ok = 0; __u32 htid = 0; __u32 order = 0; + __u32 flags = 0; memset(&sel, 0, sizeof(sel)); @@ -1152,6 +1153,14 @@ static int u32_parse_opt(struct filter_util *qu, char *handle, } terminal_ok++; continue; + } else if (strcmp(*argv, "skip_hw") == 0) { + NEXT_ARG(); + flags |= TCA_CLS_FLAGS_SKIP_HW; + continue; + } else if (strcmp(*argv, "skip_sw") == 0) { + NEXT_ARG(); + flags |= TCA_CLS_FLAGS_SKIP_SW; + continue; } else if (strcmp(*argv, "help") == 0) { explain(); return -1; @@ -1182,6 +1191,15 @@ static int u32_parse_opt(struct filter_util *qu, char *handle, addattr_l(n, MAX_MSG, TCA_U32_SEL, &sel, sizeof(sel.sel) + sel.sel.nkeys * sizeof(struct tc_u32_key)); + if (flags) { + if (!(flags ^ (TCA_CLS_FLAGS_SKIP_HW | TCA_CLS_FLAGS_SKIP_SW))) { + fprintf(stderr, "skip_hw and skip_sw are mutually " + "exclusive flags. Only one can be set\n"); + return -1; + } + addattr_l(n, MAX_MSG, TCA_U32_FLAGS, &flags, 4); + } + tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail; return 0; } @@ -1240,6 +1258,15 @@ static int u32_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, b1)); } + if (tb[TCA_U32_FLAGS]) { + __u32 flags = rta_getattr_u32(tb[TCA_U32_FLAGS]); + + if (flags & TCA_CLS_FLAGS_SKIP_HW) + fprintf(f, "skip_hw "); + if (flags & TCA_CLS_FLAGS_SKIP_SW) + fprintf(f, "skip_sw "); + } + if (tb[TCA_U32_PCNT]) { if (RTA_PAYLOAD(tb[TCA_U32_PCNT]) < sizeof(*pf)) { fprintf(f, "Broken perf counters\n"); From 622812052a71e752d55f5348ad9679884cc239d9 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Wed, 8 Jun 2016 16:45:26 -0700 Subject: [PATCH 02/17] tc: f_u32 cleanup indentation and long lines Several long lines and too long messages here. --- tc/f_u32.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/tc/f_u32.c b/tc/f_u32.c index b6ae4d28..0926461d 100644 --- a/tc/f_u32.c +++ b/tc/f_u32.c @@ -30,16 +30,18 @@ extern int show_pretty; static void explain(void) { - fprintf(stderr, "Usage: ... u32 [ match SELECTOR ... ] [ link HTID ] [ classid CLASSID ] [skip-hw | skip-sw]\n"); - fprintf(stderr, " [ action ACTION_SPEC ] [ offset OFFSET_SPEC ]\n"); - fprintf(stderr, " [ ht HTID ] [ hashkey HASHKEY_SPEC ]\n"); - fprintf(stderr, " [ sample SAMPLE ]\n"); - fprintf(stderr, "or u32 divisor DIVISOR\n"); - fprintf(stderr, "\n"); - fprintf(stderr, "Where: SELECTOR := SAMPLE SAMPLE ...\n"); - fprintf(stderr, " SAMPLE := { ip | ip6 | udp | tcp | icmp | u{32|16|8} | mark } SAMPLE_ARGS [divisor DIVISOR]\n"); - fprintf(stderr, " FILTERID := X:Y:Z\n"); - fprintf(stderr, "\nNOTE: CLASSID is parsed at hexadecimal input.\n"); + fprintf(stderr, + "Usage: ... u32 [ match SELECTOR ... ] [ link HTID ] [ classid CLASSID ]\n" + " [ action ACTION_SPEC ] [ offset OFFSET_SPEC ]\n" + " [ ht HTID ] [ hashkey HASHKEY_SPEC ]\n" + " [ sample SAMPLE ] [skip-hw | skip-sw]\n" + "or u32 divisor DIVISOR\n" + "\n" + "Where: SELECTOR := SAMPLE SAMPLE ...\n" + " SAMPLE := { ip | ip6 | udp | tcp | icmp | u{32|16|8} | mark }\n" + " SAMPLE_ARGS [ divisor DIVISOR ]\n" + " FILTERID := X:Y:Z\n" + "\nNOTE: CLASSID is parsed at hexadecimal input.\n"); } static int get_u32_handle(__u32 *handle, const char *str) @@ -1192,9 +1194,10 @@ static int u32_parse_opt(struct filter_util *qu, char *handle, sizeof(sel.sel) + sel.sel.nkeys * sizeof(struct tc_u32_key)); if (flags) { - if (!(flags ^ (TCA_CLS_FLAGS_SKIP_HW | TCA_CLS_FLAGS_SKIP_SW))) { - fprintf(stderr, "skip_hw and skip_sw are mutually " - "exclusive flags. Only one can be set\n"); + if (!(flags ^ (TCA_CLS_FLAGS_SKIP_HW | + TCA_CLS_FLAGS_SKIP_SW))) { + fprintf(stderr, + "skip_hw and skip_sw are mutually exclusive\n"); return -1; } addattr_l(n, MAX_MSG, TCA_U32_FLAGS, &flags, 4); @@ -1220,9 +1223,9 @@ static int u32_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, SPRINT_BUF(b1); fprintf(f, "fh %s ", sprint_u32_handle(handle, b1)); } - if (TC_U32_NODE(handle)) { + + if (TC_U32_NODE(handle)) fprintf(f, "order %d ", TC_U32_NODE(handle)); - } if (tb[TCA_U32_SEL]) { if (RTA_PAYLOAD(tb[TCA_U32_SEL]) < sizeof(*sel)) @@ -1325,14 +1328,15 @@ static int u32_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, fprintf(f, "\n"); tc_print_police(f, tb[TCA_U32_POLICE]); } + if (tb[TCA_U32_INDEV]) { struct rtattr *idev = tb[TCA_U32_INDEV]; fprintf(f, "\n input dev %s\n", rta_getattr_str(idev)); } - if (tb[TCA_U32_ACT]) { + + if (tb[TCA_U32_ACT]) tc_print_action(f, tb[TCA_U32_ACT]); - } return 0; } From 24604eb2877a3be3f3167ca329b37cba178537f2 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Thu, 9 Jun 2016 19:20:36 +0200 Subject: [PATCH 03/17] ipaddress: Allow listing addresses by type Not sure why this was limited to ip-link before. It is semantically equal to the 'master' keyword, which is not restricted at all. The man page and help text adjustments include the 'master' keyword as well since that is also supported but wasn't documented before. Cc: Vadim Kochan Signed-off-by: Phil Sutter --- ip/ipaddress.c | 11 ++++++-- man/man8/ip-address.8.in | 57 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/ip/ipaddress.c b/ip/ipaddress.c index df363b07..8766530f 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -75,8 +75,11 @@ static void usage(void) fprintf(stderr, "Usage: ip address {add|change|replace} IFADDR dev IFNAME [ LIFETIME ]\n"); fprintf(stderr, " [ CONFFLAG-LIST ]\n"); fprintf(stderr, " ip address del IFADDR dev IFNAME [mngtmpaddr]\n"); - fprintf(stderr, " ip address {show|save|flush} [ dev IFNAME ] [ scope SCOPE-ID ]\n"); + fprintf(stderr, " ip address {save|flush} [ dev IFNAME ] [ scope SCOPE-ID ]\n"); fprintf(stderr, " [ to PREFIX ] [ FLAG-LIST ] [ label LABEL ] [up]\n"); + fprintf(stderr, " ip address [ show [ dev IFNAME ] [ scope SCOPE-ID ] [ master DEVICE ]\n"); + fprintf(stderr, " [ type TYPE ] [ to PREFIX ] [ FLAG-LIST ]\n"); + fprintf(stderr, " [ label LABEL ] [up] ]\n"); fprintf(stderr, " ip address {showdump|restore}\n"); fprintf(stderr, "IFADDR := PREFIX | ADDR peer PREFIX\n"); fprintf(stderr, " [ broadcast ADDR ] [ anycast ADDR ]\n"); @@ -90,6 +93,10 @@ static void usage(void) fprintf(stderr, "CONFFLAG := [ home | nodad | mngtmpaddr | noprefixroute | autojoin ]\n"); fprintf(stderr, "LIFETIME := [ valid_lft LFT ] [ preferred_lft LFT ]\n"); fprintf(stderr, "LFT := forever | SECONDS\n"); + fprintf(stderr, "TYPE := { vlan | veth | vcan | dummy | ifb | macvlan | macvtap |\n"); + fprintf(stderr, " bridge | bond | ipoib | ip6tnl | ipip | sit | vxlan |\n"); + fprintf(stderr, " gre | gretap | ip6gre | ip6gretap | vti | nlmon |\n"); + fprintf(stderr, " bond_slave | ipvlan | geneve | bridge_slave | vrf }\n"); exit(-1); } @@ -1613,7 +1620,7 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action) if (!ifindex) invarg("Device does not exist\n", *argv); filter.master = ifindex; - } else if (do_link && strcmp(*argv, "type") == 0) { + } else if (strcmp(*argv, "type") == 0) { NEXT_ARG(); filter.kind = *argv; } else { diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in index ff3fe0b9..ab0942d7 100644 --- a/man/man8/ip-address.8.in +++ b/man/man8/ip-address.8.in @@ -23,7 +23,7 @@ ip-address \- protocol address management .IB IFADDR " dev " IFNAME " [ " mngtmpaddr " ]" .ti -8 -.BR "ip address" " { " show " | " save " | " flush " } [ " dev +.BR "ip address" " { " save " | " flush " } [ " dev .IR IFNAME " ] [ " .B scope .IR SCOPE-ID " ] [ " @@ -32,6 +32,21 @@ ip-address \- protocol address management .B label .IR PATTERN " ] [ " up " ]" +.ti -8 +.BR "ip address" " [ " show " [ " dev +.IR IFNAME " ] [ " +.B scope +.IR SCOPE-ID " ] [ " +.B to +.IR PREFIX " ] [ " FLAG-LIST " ] [ " +.B label +.IR PATTERN " ] [ " +.B master +.IR DEVICE " ] [ " +.B type +.IR TYPE " ] [ " +.BR up " ] ]" + .ti -8 .BR "ip address" " { " showdump " | " restore " }" @@ -80,6 +95,34 @@ ip-address \- protocol address management .BR forever " |" .IR SECONDS " ]" +.ti -8 +.IR TYPE " := [ " +.BR bridge " | " +.BR bond " | " +.BR can " | " +.BR dummy " | " +.BR hsr " | " +.BR ifb " | " +.BR ipoib " |" +.BR macvlan " | " +.BR macvtap " | " +.BR vcan " | " +.BR veth " | " +.BR vlan " | " +.BR vxlan " |" +.BR ip6tnl " |" +.BR ipip " |" +.BR sit " |" +.BR gre " |" +.BR gretap " |" +.BR ip6gre " |" +.BR ip6gretap " |" +.BR vti " |" +.BR nlmon " |" +.BR ipvlan " |" +.BR lowpan " |" +.BR geneve " ]" + .SH "DESCRIPTION" The .B address @@ -229,6 +272,14 @@ only list addresses with labels matching the .I PATTERN is a usual shell style pattern. +.TP +.BI master " DEVICE" +only list interfaces enslaved to this master device. + +.TP +.BI type " TYPE" +only list interfaces of the given type. + .TP .B up only list running interfaces. @@ -280,8 +331,8 @@ This command flushes the protocol addresses selected by some criteria. .PP This command has the same arguments as -.B show. -The difference is that it does not run when no arguments are given. +.BR show " except that " type " and " master " selectors are not supported." +Another difference is that it does not run when no arguments are given. .PP .B Warning: From 9ba4126dc4d6abb8dc5c8c8d52177849e764a14e Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 14 Jun 2016 22:55:17 +0200 Subject: [PATCH 04/17] utils: fix hex digits parsing in hexstring_a2n() strtoul() only modifies errno on overflow, so if errno is not zero before calling the function its value is preserved and makes the function fail for valid inputs; initialize it. Signed-off-by: Beniamino Galvani --- lib/utils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/utils.c b/lib/utils.c index 70e85b75..7dceeb58 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -924,6 +924,7 @@ __u8 *hexstring_a2n(const char *str, __u8 *buf, int blen, unsigned int *len) strncpy(tmpstr, str, 2); tmpstr[2] = '\0'; + errno = 0; tmp = strtoul(tmpstr, &endptr, 16); if (errno != 0 || tmp > 0xFF || *endptr != '\0') return NULL; From 8e45e44b7923a30c97ea50e43f72db6688c6175e Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 16:39:50 +0200 Subject: [PATCH 05/17] man: ip-link: Document query_rss option Doc text shamelessly stolen from the introducing commit's message (6c55c8c4617c5 ['ip link set vf: Added "query_rss" command']). Signed-off-by: Phil Sutter --- man/man8/ip-link.8.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in index 8fcce5e5..d5673639 100644 --- a/man/man8/ip-link.8.in +++ b/man/man8/ip-link.8.in @@ -141,6 +141,8 @@ ip-link \- network device configuration .br .RB "[ " spoofchk " { " on " | " off " } ]" .br +.RB "[ " query_rss " { " on " | " off " } ]" +.br .RB "[ " state " { " auto " | " enable " | " disable " } ]" .br .RB "[ " trust " { " on " | " off " } ] ]" @@ -1159,6 +1161,9 @@ parameter must be specified. .BI spoofchk " on|off" - turn packet spoof checking on or off for the specified VF. .sp +.BI query_rss " on|off" +- toggle the ability of querying the RSS configuration of a specific VF. VF RSS information like RSS hash key may be considered sensitive on some devices where this information is shared between VF and PF and thus its querying may be prohibited by default. +.sp .BI state " auto|enable|disable" - set the virtual link state as seen by the specified VF. Setting to auto means a reflection of the PF link state, enable lets the VF to communicate with other VFs on From d8694a30a4027a3b5d9a2a14ca846b77abd02ea1 Mon Sep 17 00:00:00 2001 From: Jamal Hadi Salim Date: Sun, 12 Jun 2016 17:40:34 -0400 Subject: [PATCH 06/17] action pedit: stylistic changes More modern layout. Signed-off-by: Jamal Hadi Salim --- tc/m_pedit.c | 118 +++++++++++++++++++++++++++------------------------ 1 file changed, 62 insertions(+), 56 deletions(-) diff --git a/tc/m_pedit.c b/tc/m_pedit.c index a539b68b..d276ba04 100644 --- a/tc/m_pedit.c +++ b/tc/m_pedit.c @@ -32,8 +32,7 @@ static struct m_pedit_util *pedit_list; static int pedit_debug; -static void -explain(void) +static void explain(void) { fprintf(stderr, "Usage: ... pedit munge [CONTROL]\n"); fprintf(stderr, @@ -48,22 +47,24 @@ explain(void) } -static void -usage(void) +static void usage(void) { explain(); exit(-1); } -static int -pedit_parse_nopopt (int *argc_p, char ***argv_p, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) +static int pedit_parse_nopopt(int *argc_p, char ***argv_p, + struct tc_pedit_sel *sel, + struct tc_pedit_key *tkey) { int argc = *argc_p; char **argv = *argv_p; if (argc) { - fprintf(stderr, "Unknown action hence option \"%s\" is unparsable\n", *argv); - return -1; + fprintf(stderr, + "Unknown action hence option \"%s\" is unparsable\n", + *argv); + return -1; } return 0; @@ -75,7 +76,7 @@ static struct m_pedit_util *get_pedit_kind(const char *str) static void *pBODY; void *dlh; char buf[256]; - struct m_pedit_util *p; + struct m_pedit_util *p; for (p = pedit_list; p; p = p->next) { if (strcmp(p->id, str) == 0) @@ -107,15 +108,14 @@ noexist: p = malloc(sizeof(*p)); if (p) { memset(p, 0, sizeof(*p)); - strncpy(p->id, str, sizeof(p->id)-1); + strncpy(p->id, str, sizeof(p->id) - 1); p->parse_peopt = pedit_parse_nopopt; goto reg; } return p; } -int -pack_key(struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) +int pack_key(struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) { int hwm = sel->nkeys; @@ -137,9 +137,8 @@ pack_key(struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) return 0; } - -int -pack_key32(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) +int pack_key32(__u32 retain, struct tc_pedit_sel *sel, + struct tc_pedit_key *tkey) { if (tkey->off > (tkey->off & ~3)) { fprintf(stderr, @@ -152,11 +151,11 @@ pack_key32(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) return pack_key(sel, tkey); } -int -pack_key16(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) +int pack_key16(__u32 retain, struct tc_pedit_sel *sel, + struct tc_pedit_key *tkey) { int ind, stride; - __u32 m[4] = {0x0000FFFF, 0xFF0000FF, 0xFFFF0000}; + __u32 m[4] = { 0x0000FFFF, 0xFF0000FF, 0xFFFF0000 }; if (tkey->val > 0xFFFF || tkey->mask > 0xFFFF) { fprintf(stderr, "pack_key16 bad value\n"); @@ -177,19 +176,20 @@ pack_key16(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) tkey->off &= ~3; if (pedit_debug) - printf("pack_key16: Final val %08x mask %08x\n", tkey->val, tkey->mask); + printf("pack_key16: Final val %08x mask %08x\n", + tkey->val, tkey->mask); return pack_key(sel, tkey); } -int -pack_key8(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) +int pack_key8(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) { int ind, stride; - __u32 m[4] = {0x00FFFFFF, 0xFF00FFFF, 0xFFFF00FF, 0xFFFFFF00}; + __u32 m[4] = { 0x00FFFFFF, 0xFF00FFFF, 0xFFFF00FF, 0xFFFFFF00 }; if (tkey->val > 0xFF || tkey->mask > 0xFF) { - fprintf(stderr, "pack_key8 bad value (val %x mask %x\n", tkey->val, tkey->mask); + fprintf(stderr, "pack_key8 bad value (val %x mask %x\n", + tkey->val, tkey->mask); return -1; } @@ -202,12 +202,12 @@ pack_key8(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) tkey->off &= ~3; if (pedit_debug) - printf("pack_key8: Final word off %d val %08x mask %08x\n", tkey->off, tkey->val, tkey->mask); + printf("pack_key8: Final word off %d val %08x mask %08x\n", + tkey->off, tkey->val, tkey->mask); return pack_key(sel, tkey); } -int -parse_val(int *argc_p, char ***argv_p, __u32 *val, int type) +int parse_val(int *argc_p, char ***argv_p, __u32 * val, int type) { int argc = *argc_p; char **argv = *argv_p; @@ -216,7 +216,7 @@ parse_val(int *argc_p, char ***argv_p, __u32 *val, int type) return -1; if (type == TINT) - return get_integer((int *) val, *argv, 0); + return get_integer((int *)val, *argv, 0); if (type == TU32) return get_u32(val, *argv, 0); @@ -238,8 +238,8 @@ parse_val(int *argc_p, char ***argv_p, __u32 *val, int type) return -1; } -int -parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) +int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain, + struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) { __u32 mask = 0, val = 0; __u32 o = 0xFF; @@ -251,7 +251,8 @@ parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain, struct return -1; if (pedit_debug) - printf("parse_cmd argc %d %s offset %d length %d\n", argc, *argv, tkey->off, len); + printf("parse_cmd argc %d %s offset %d length %d\n", + argc, *argv, tkey->off, len); if (len == 2) o = 0xFFFF; @@ -271,13 +272,15 @@ parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain, struct return -1; } - argc--; argv++; + argc--; + argv++; if (argc && matches(*argv, "retain") == 0) { NEXT_ARG(); if (parse_val(&argc, &argv, &retain, TU32)) return -1; - argc--; argv++; + argc--; + argv++; } tkey->val = val; @@ -302,15 +305,16 @@ parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain, struct return -1; done: if (pedit_debug) - printf("parse_cmd done argc %d %s offset %d length %d\n", argc, *argv, tkey->off, len); + printf("parse_cmd done argc %d %s offset %d length %d\n", + argc, *argv, tkey->off, len); *argc_p = argc; *argv_p = argv; return res; } -int -parse_offset(int *argc_p, char ***argv_p, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) +int parse_offset(int *argc_p, char ***argv_p, struct tc_pedit_sel *sel, + struct tc_pedit_key *tkey) { int off; __u32 len, retain; @@ -331,7 +335,6 @@ parse_offset(int *argc_p, char ***argv_p, struct tc_pedit_sel *sel, struct tc_pe if (argc <= 0) return -1; - if (matches(*argv, "u32") == 0) { len = 4; retain = 0xFFFFFFFF; @@ -386,8 +389,7 @@ done: return res; } -static int -parse_munge(int *argc_p, char ***argv_p, struct tc_pedit_sel *sel) +static int parse_munge(int *argc_p, char ***argv_p, struct tc_pedit_sel *sel) { struct tc_pedit_key tkey; int argc = *argc_p; @@ -433,8 +435,8 @@ done: return res; } -int -parse_pedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n) +int parse_pedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, + struct nlmsghdr *n) { struct { struct tc_pedit_sel sel; @@ -459,13 +461,15 @@ parse_pedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, stru usage(); } else if (matches(*argv, "munge") == 0) { if (!ok) { - fprintf(stderr, "Illegal pedit construct (%s)\n", *argv); + fprintf(stderr, "Bad pedit construct (%s)\n", + *argv); explain(); return -1; } NEXT_ARG(); if (parse_munge(&argc, &argv, &sel.sel)) { - fprintf(stderr, "Illegal pedit construct (%s)\n", *argv); + fprintf(stderr, "Bad pedit construct (%s)\n", + *argv); explain(); return -1; } @@ -489,7 +493,7 @@ parse_pedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, stru sel.sel.action = TC_ACT_PIPE; NEXT_ARG(); } else if (matches(*argv, "drop") == 0 || - matches(*argv, "shot") == 0) { + matches(*argv, "shot") == 0) { sel.sel.action = TC_ACT_SHOT; NEXT_ARG(); } else if (matches(*argv, "continue") == 0) { @@ -517,16 +521,17 @@ parse_pedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, stru tail = NLMSG_TAIL(n); addattr_l(n, MAX_MSG, tca_id, NULL, 0); - addattr_l(n, MAX_MSG, TCA_PEDIT_PARMS, &sel, sizeof(sel.sel)+sel.sel.nkeys*sizeof(struct tc_pedit_key)); - tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail; + addattr_l(n, MAX_MSG, TCA_PEDIT_PARMS, &sel, + sizeof(sel.sel) + + sel.sel.nkeys * sizeof(struct tc_pedit_key)); + tail->rta_len = (void *)NLMSG_TAIL(n) - (void *)tail; *argc_p = argc; *argv_p = argv; return 0; } -int -print_pedit(struct action_util *au, FILE * f, struct rtattr *arg) +int print_pedit(struct action_util *au, FILE *f, struct rtattr *arg) { struct tc_pedit_sel *sel; struct rtattr *tb[TCA_PEDIT_MAX + 1]; @@ -544,8 +549,10 @@ print_pedit(struct action_util *au, FILE * f, struct rtattr *arg) } sel = RTA_DATA(tb[TCA_PEDIT_PARMS]); - fprintf(f, " pedit action %s keys %d\n ", action_n2a(sel->action, b1, sizeof (b1)), sel->nkeys); - fprintf(f, "\t index %d ref %d bind %d", sel->index, sel->refcnt, sel->bindcnt); + fprintf(f, " pedit action %s keys %d\n ", + action_n2a(sel->action, b1, sizeof(b1)), sel->nkeys); + fprintf(f, "\t index %d ref %d bind %d", sel->index, sel->refcnt, + sel->bindcnt); if (show_stats) { if (tb[TCA_PEDIT_TM]) { @@ -561,21 +568,20 @@ print_pedit(struct action_util *au, FILE * f, struct rtattr *arg) for (i = 0; i < sel->nkeys; i++, key++) { fprintf(f, "\n\t key #%d", i); fprintf(f, " at %d: val %08x mask %08x", - (unsigned int)key->off, - (unsigned int)ntohl(key->val), - (unsigned int)ntohl(key->mask)); + (unsigned int)key->off, + (unsigned int)ntohl(key->val), + (unsigned int)ntohl(key->mask)); } } else { - fprintf(f, "\npedit %x keys %d is not LEGIT", sel->index, sel->nkeys); + fprintf(f, "\npedit %x keys %d is not LEGIT", sel->index, + sel->nkeys); } - fprintf(f, "\n "); return 0; } -int -pedit_print_xstats(struct action_util *au, FILE *f, struct rtattr *xstats) +int pedit_print_xstats(struct action_util *au, FILE *f, struct rtattr *xstats) { return 0; } From 8b625177ba69985be6673e776c54f45ab8d40e58 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Tue, 14 Jun 2016 14:31:37 -0700 Subject: [PATCH 07/17] pedit: fix whitespace etc Minor changes from checkpatch --- tc/m_pedit.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tc/m_pedit.c b/tc/m_pedit.c index d276ba04..64533060 100644 --- a/tc/m_pedit.c +++ b/tc/m_pedit.c @@ -37,10 +37,12 @@ static void explain(void) fprintf(stderr, "Usage: ... pedit munge [CONTROL]\n"); fprintf(stderr, "Where: MUNGE := |\n" - "\t:= [ATC]\n \t\tOFFSETC:= offset \n " - "\t\tATC:= at offmask shift \n \t\tNOTE: offval is byte offset, must be multiple of 4\n " - "\t\tNOTE: maskval is a 32 bit hex number\n \t\tNOTE: shiftval is a is a shift value\n " - "\t\tCMD:= clear | invert | set | retain\n \t:= ip | ip6 \n " + "\t:= [ATC]\n \t\tOFFSETC:= offset \n" + "\t\tATC:= at offmask shift \n" + "\t\tNOTE: offval is byte offset, must be multiple of 4\n" + "\t\tNOTE: maskval is a 32 bit hex number\n \t\tNOTE: shiftval is a is a shift value\n" + "\t\tCMD:= clear | invert | set | retain\n" + "\t:= ip | ip6 \n" " \t\t| udp | tcp | icmp \n" "\tCONTROL:= reclassify | pipe | drop | continue | pass\n" "For Example usage look at the examples directory\n"); @@ -207,7 +209,7 @@ int pack_key8(__u32 retain, struct tc_pedit_sel *sel, struct tc_pedit_key *tkey) return pack_key(sel, tkey); } -int parse_val(int *argc_p, char ***argv_p, __u32 * val, int type) +int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type) { int argc = *argc_p; char **argv = *argv_p; @@ -224,16 +226,15 @@ int parse_val(int *argc_p, char ***argv_p, __u32 * val, int type) if (type == TIPV4) { inet_prefix addr; - if (get_prefix_1(&addr, *argv, AF_INET)) { + if (get_prefix_1(&addr, *argv, AF_INET)) return -1; - } + *val = addr.data[0]; return 0; } - if (type == TIPV6) { - /* not implemented yet */ - return -1; - } + + if (type == TIPV6) + return -1; /* not implemented yet */ return -1; } From 445745221a21ecb4111c19cfa7f4c2cf4796337f Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:00 +0200 Subject: [PATCH 08/17] tc: m_xt: Prevent segfault with standard targets Iptables standard targets like DROP or REJECT don't implement the print callback in libxtables. Hence the following command would segfault: | tc filter add dev d0 parent ffff: u32 match u32 0 0 action xt -j DROP With this patch standard targets still can't be used (and are not really useful anyway), but at least it doesn't crash anymore. Signed-off-by: Phil Sutter --- tc/m_xt.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index c3f866df..62ec6d7f 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -250,8 +250,12 @@ static int parse_ipt(struct action_util *a, int *argc_p, fprintf(stdout, "tablename: %s hook: %s\n ", tname, ipthooks[hook]); fprintf(stdout, "\ttarget: "); - if (m) - m->print(NULL, m->t, 0); + if (m) { + if (m->print) + m->print(NULL, m->t, 0); + else + printf("%s ", m->name); + } fprintf(stdout, " index %d\n", index); if (strlen(tname) > 16) { From 8eee75a8358c542d881d2a84e2c47cc0a9fa92ef Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:01 +0200 Subject: [PATCH 09/17] tc: m_xt: Fix segfault when adding multiple actions at once Without this, the following call to tc would segfault: | tc filter add dev d0 parent ffff: u32 match u32 0 0 \ | action xt -j MARK --set-mark 0x1 \ | action xt -j MARK --set-mark 0x1 The reason is basically the same as for 6e2e5ec28bad4 ("fix print_ipt: segfault if more then one filter with action -j MARK.") but in parse_ipt() instead of print_ipt(). Signed-off-by: Phil Sutter --- tc/m_xt.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index 62ec6d7f..45d86d66 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -133,7 +133,9 @@ static int parse_ipt(struct action_util *a, int *argc_p, __u32 hook = 0, index = 0; struct option *opts = NULL; - xtables_init_all(&tcipt_globals, NFPROTO_IPV4); + /* copy tcipt_globals because .opts will be modified by iptables */ + struct xtables_globals tmp_tcipt_globals = tcipt_globals; + xtables_init_all(&tmp_tcipt_globals, NFPROTO_IPV4); set_lib_dir(); { @@ -153,7 +155,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, } while (1) { - c = getopt_long(argc, argv, "j:", tcipt_globals.opts, NULL); + c = getopt_long(argc, argv, "j:", tmp_tcipt_globals.opts, NULL); if (c == -1) break; switch (c) { @@ -166,12 +168,12 @@ static int parse_ipt(struct action_util *a, int *argc_p, return -1; } #if (XTABLES_VERSION_CODE >= 6) - opts = xtables_options_xfrm(tcipt_globals.orig_opts, - tcipt_globals.opts, + opts = xtables_options_xfrm(tmp_tcipt_globals.orig_opts, + tmp_tcipt_globals.opts, m->x6_options, &m->option_offset); #else - opts = xtables_merge_options(tcipt_globals.opts, + opts = xtables_merge_options(tmp_tcipt_globals.opts, m->extra_opts, &m->option_offset); #endif @@ -179,7 +181,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, fprintf(stderr, " failed to find additional options for target %s\n\n", optarg); return -1; } else - tcipt_globals.opts = opts; + tmp_tcipt_globals.opts = opts; } else { fprintf(stderr, " failed to find target %s\n\n", optarg); return -1; From f1a7c7d8301e9dcead8f8c0d28e7f1587c0cd3cc Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:02 +0200 Subject: [PATCH 10/17] tc: m_xt: Fix indenting By exiting early if xtables_find_target() fails, one indenting level can be dropped. Some of the wrongly indented code then happens to sit at the right spot by accident which is why this patch is smaller than expected. Signed-off-by: Phil Sutter --- tc/m_xt.c | 54 ++++++++++++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index 45d86d66..5a629c44 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -161,12 +161,15 @@ static int parse_ipt(struct action_util *a, int *argc_p, switch (c) { case 'j': m = xtables_find_target(optarg, XTF_TRY_LOAD); - if (m != NULL) { + if (!m) { + fprintf(stderr, " failed to find target %s\n\n", optarg); + return -1; + } - if (build_st(m, NULL) < 0) { - printf(" %s error\n", m->name); - return -1; - } + if (build_st(m, NULL) < 0) { + printf(" %s error\n", m->name); + return -1; + } #if (XTABLES_VERSION_CODE >= 6) opts = xtables_options_xfrm(tmp_tcipt_globals.orig_opts, tmp_tcipt_globals.opts, @@ -182,22 +185,18 @@ static int parse_ipt(struct action_util *a, int *argc_p, return -1; } else tmp_tcipt_globals.opts = opts; - } else { - fprintf(stderr, " failed to find target %s\n\n", optarg); - return -1; - } ok++; break; default: memset(&fw, 0, sizeof(fw)); #if (XTABLES_VERSION_CODE >= 6) - if (m != NULL && m->x6_parse != NULL) { - xtables_option_tpcall(c, argv, 0, m, NULL); + if (m != NULL && m->x6_parse != NULL) { + xtables_option_tpcall(c, argv, 0, m, NULL); #else - if (m != NULL && m->parse != NULL) { - m->parse(c - m->option_offset, argv, 0, &m->tflags, - NULL, &m->t); + if (m != NULL && m->parse != NULL) { + m->parse(c - m->option_offset, argv, 0, + &m->tflags, NULL, &m->t); #endif } else { fprintf(stderr, "failed to find target %s\n\n", optarg); @@ -339,11 +338,15 @@ print_ipt(struct action_util *au, FILE * f, struct rtattr *arg) t = RTA_DATA(tb[TCA_IPT_TARG]); m = xtables_find_target(t->u.user.name, XTF_TRY_LOAD); - if (m != NULL) { - if (build_st(m, t) < 0) { - fprintf(stderr, " %s error\n", m->name); - return -1; - } + if (!m) { + fprintf(stderr, " failed to find target %s\n\n", + t->u.user.name); + return -1; + } + if (build_st(m, t) < 0) { + fprintf(stderr, " %s error\n", m->name); + return -1; + } #if (XTABLES_VERSION_CODE >= 6) opts = xtables_options_xfrm(tmp_tcipt_globals.orig_opts, @@ -355,16 +358,11 @@ print_ipt(struct action_util *au, FILE * f, struct rtattr *arg) m->extra_opts, &m->option_offset); #endif - if (opts == NULL) { - fprintf(stderr, " failed to find additional options for target %s\n\n", optarg); - return -1; - } else - tmp_tcipt_globals.opts = opts; - } else { - fprintf(stderr, " failed to find target %s\n\n", - t->u.user.name); + if (opts == NULL) { + fprintf(stderr, " failed to find additional options for target %s\n\n", optarg); return -1; - } + } else + tmp_tcipt_globals.opts = opts; fprintf(f, "\ttarget "); m->print(NULL, m->t, 0); if (tb[TCA_IPT_INDEX] == NULL) { From b45f9141c2602de790a9fc3f0f423ed72419da24 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:03 +0200 Subject: [PATCH 11/17] tc: m_xt: Get rid of one indentation level in parse_ipt() Signed-off-by: Phil Sutter --- tc/m_xt.c | 97 +++++++++++++++++++++++++++---------------------------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index 5a629c44..b3731086 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -298,6 +298,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, static int print_ipt(struct action_util *au, FILE * f, struct rtattr *arg) { + struct xtables_target *m; struct rtattr *tb[TCA_IPT_MAX + 1]; struct xt_entry_target *t = NULL; struct option *opts = NULL; @@ -333,62 +334,60 @@ print_ipt(struct action_util *au, FILE * f, struct rtattr *arg) if (tb[TCA_IPT_TARG] == NULL) { fprintf(f, "\t[NULL ipt target parameters ]\n"); return -1; - } else { - struct xtables_target *m = NULL; + } - t = RTA_DATA(tb[TCA_IPT_TARG]); - m = xtables_find_target(t->u.user.name, XTF_TRY_LOAD); - if (!m) { - fprintf(stderr, " failed to find target %s\n\n", - t->u.user.name); - return -1; - } - if (build_st(m, t) < 0) { - fprintf(stderr, " %s error\n", m->name); - return -1; - } + t = RTA_DATA(tb[TCA_IPT_TARG]); + m = xtables_find_target(t->u.user.name, XTF_TRY_LOAD); + if (!m) { + fprintf(stderr, " failed to find target %s\n\n", + t->u.user.name); + return -1; + } + if (build_st(m, t) < 0) { + fprintf(stderr, " %s error\n", m->name); + return -1; + } #if (XTABLES_VERSION_CODE >= 6) - opts = xtables_options_xfrm(tmp_tcipt_globals.orig_opts, - tmp_tcipt_globals.opts, - m->x6_options, - &m->option_offset); + opts = xtables_options_xfrm(tmp_tcipt_globals.orig_opts, + tmp_tcipt_globals.opts, + m->x6_options, + &m->option_offset); #else - opts = xtables_merge_options(tmp_tcipt_globals.opts, - m->extra_opts, - &m->option_offset); + opts = xtables_merge_options(tmp_tcipt_globals.opts, + m->extra_opts, + &m->option_offset); #endif - if (opts == NULL) { - fprintf(stderr, " failed to find additional options for target %s\n\n", optarg); - return -1; - } else - tmp_tcipt_globals.opts = opts; - fprintf(f, "\ttarget "); - m->print(NULL, m->t, 0); - if (tb[TCA_IPT_INDEX] == NULL) { - fprintf(f, " [NULL ipt target index ]\n"); - } else { - __u32 index; - - index = rta_getattr_u32(tb[TCA_IPT_INDEX]); - fprintf(f, "\n\tindex %d", index); - } - - if (tb[TCA_IPT_CNT]) { - struct tc_cnt *c = RTA_DATA(tb[TCA_IPT_CNT]); - - fprintf(f, " ref %d bind %d", c->refcnt, c->bindcnt); - } - if (show_stats) { - if (tb[TCA_IPT_TM]) { - struct tcf_t *tm = RTA_DATA(tb[TCA_IPT_TM]); - - print_tm(f, tm); - } - } - fprintf(f, "\n"); + if (opts == NULL) { + fprintf(stderr, " failed to find additional options for target %s\n\n", optarg); + return -1; + } else + tmp_tcipt_globals.opts = opts; + fprintf(f, "\ttarget "); + m->print(NULL, m->t, 0); + if (tb[TCA_IPT_INDEX] == NULL) { + fprintf(f, " [NULL ipt target index ]\n"); + } else { + __u32 index; + index = rta_getattr_u32(tb[TCA_IPT_INDEX]); + fprintf(f, "\n\tindex %d", index); } + + if (tb[TCA_IPT_CNT]) { + struct tc_cnt *c = RTA_DATA(tb[TCA_IPT_CNT]); + + fprintf(f, " ref %d bind %d", c->refcnt, c->bindcnt); + } + if (show_stats) { + if (tb[TCA_IPT_TM]) { + struct tcf_t *tm = RTA_DATA(tb[TCA_IPT_TM]); + + print_tm(f, tm); + } + } + fprintf(f, "\n"); + xtables_free_opts(1); return 0; From b0ba0185763ad2bae3335423221919ed47951885 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:04 +0200 Subject: [PATCH 12/17] tc: m_xt: Drop unused variable fw in parse_ipt() Signed-off-by: Phil Sutter --- tc/m_xt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index b3731086..c42f3bda 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -120,7 +120,6 @@ static int parse_ipt(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n) { struct xtables_target *m = NULL; - struct ipt_entry fw; struct rtattr *tail; int c; @@ -189,7 +188,6 @@ static int parse_ipt(struct action_util *a, int *argc_p, break; default: - memset(&fw, 0, sizeof(fw)); #if (XTABLES_VERSION_CODE >= 6) if (m != NULL && m->x6_parse != NULL) { xtables_option_tpcall(c, argv, 0, m, NULL); From ab8f52fc4ae1141ff9980d24244f2d9559e47968 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:05 +0200 Subject: [PATCH 13/17] tc: m_xt: Get rid of rargc in parse_ipt() No need to copy the passed parameter, it's changed only once right before function return. Signed-off-by: Phil Sutter --- tc/m_xt.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index c42f3bda..61fc437d 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -123,7 +123,6 @@ static int parse_ipt(struct action_util *a, int *argc_p, struct rtattr *tail; int c; - int rargc = *argc_p; char **argv = *argv_p; int argc = 0, iargc = 0; char k[16]; @@ -140,7 +139,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, { int i; - for (i = 0; i < rargc; i++) { + for (i = 0; i < *argc_p; i++) { if (NULL == argv[i] || 0 == strcmp(argv[i], "action")) { break; } @@ -149,7 +148,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, } if (argc <= 2) { - fprintf(stderr, "bad arguments to ipt %d vs %d\n", argc, rargc); + fprintf(stderr, "bad arguments to ipt %d vs %d\n", argc, *argc_p); return -1; } @@ -274,7 +273,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, argc -= optind; argv += optind; - *argc_p = rargc - iargc; + *argc_p -= iargc; *argv_p = argv; optind = 0; From 28432f370e6ad72a7eab9ba6bae67eabdd4b1c57 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:06 +0200 Subject: [PATCH 14/17] tc: m_xt: Get rid of iargc variable in parse_ipt() After dropping the unused decrement of argc in the function's tail, it can fully take over what iargc has been used for. Signed-off-by: Phil Sutter --- tc/m_xt.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index 61fc437d..55ebadf2 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -124,7 +124,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, int c; char **argv = *argv_p; - int argc = 0, iargc = 0; + int argc = 0; char k[16]; int size = 0; int iok = 0, ok = 0; @@ -144,7 +144,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, break; } } - iargc = argc = i; + argc = i; } if (argc <= 2) { @@ -205,7 +205,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, } } - if (iargc > optind) { + if (argc > optind) { if (matches(argv[optind], "index") == 0) { if (get_u32(&index, argv[optind + 1], 10)) { fprintf(stderr, "Illegal \"index\"\n"); @@ -271,9 +271,8 @@ static int parse_ipt(struct action_util *a, int *argc_p, addattr_l(n, MAX_MSG, TCA_IPT_TARG, m->t, m->t->u.target_size); tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail; - argc -= optind; argv += optind; - *argc_p -= iargc; + *argc_p -= argc; *argv_p = argv; optind = 0; From f6ddd9c5da4e552322baf237075aae6db17237d4 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:07 +0200 Subject: [PATCH 15/17] tc: m_xt: Simplify argc adjusting in parse_ipt() And while at it, also improve the error message in case too few parameters have been given. Signed-off-by: Phil Sutter --- tc/m_xt.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index 55ebadf2..f449c9d1 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -124,7 +124,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, int c; char **argv = *argv_p; - int argc = 0; + int argc; char k[16]; int size = 0; int iok = 0, ok = 0; @@ -136,19 +136,14 @@ static int parse_ipt(struct action_util *a, int *argc_p, xtables_init_all(&tmp_tcipt_globals, NFPROTO_IPV4); set_lib_dir(); - { - int i; - - for (i = 0; i < *argc_p; i++) { - if (NULL == argv[i] || 0 == strcmp(argv[i], "action")) { - break; - } - } - argc = i; + /* parse only up until the next action */ + for (argc = 0; argc < *argc_p; argc++) { + if (!argv[argc] || !strcmp(argv[argc], "action")) + break; } if (argc <= 2) { - fprintf(stderr, "bad arguments to ipt %d vs %d\n", argc, *argc_p); + fprintf(stderr, "too few arguments for xt, need at least '-j '\n"); return -1; } From 2ef4008585ec9184a0abf7534bf7f575ce6579d1 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 10 Jun 2016 13:42:08 +0200 Subject: [PATCH 16/17] tc: m_xt: Introduce get_xtables_target_opts() This pulls common code from parse_ipt() and print_ipt() functions together. While here, also fix for incorrect use of the global 'optarg' variable in print_ipt(). Signed-off-by: Phil Sutter --- tc/m_xt.c | 58 +++++++++++++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index f449c9d1..1224e4fe 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -116,6 +116,27 @@ static void set_lib_dir(void) } +static int get_xtables_target_opts(struct xtables_globals *globals, + struct xtables_target *m) +{ + struct option *opts; + +#if (XTABLES_VERSION_CODE >= 6) + opts = xtables_options_xfrm(globals->orig_opts, + globals->opts, + m->x6_options, + &m->option_offset); +#else + opts = xtables_merge_options(globals->opts, + m->extra_opts, + &m->option_offset); +#endif + if (!opts) + return -1; + globals->opts = opts; + return 0; +} + static int parse_ipt(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, struct nlmsghdr *n) { @@ -129,7 +150,6 @@ static int parse_ipt(struct action_util *a, int *argc_p, int size = 0; int iok = 0, ok = 0; __u32 hook = 0, index = 0; - struct option *opts = NULL; /* copy tcipt_globals because .opts will be modified by iptables */ struct xtables_globals tmp_tcipt_globals = tcipt_globals; @@ -163,21 +183,11 @@ static int parse_ipt(struct action_util *a, int *argc_p, printf(" %s error\n", m->name); return -1; } -#if (XTABLES_VERSION_CODE >= 6) - opts = xtables_options_xfrm(tmp_tcipt_globals.orig_opts, - tmp_tcipt_globals.opts, - m->x6_options, - &m->option_offset); -#else - opts = xtables_merge_options(tmp_tcipt_globals.opts, - m->extra_opts, - &m->option_offset); -#endif - if (opts == NULL) { + + if (get_xtables_target_opts(&tmp_tcipt_globals, m) < 0) { fprintf(stderr, " failed to find additional options for target %s\n\n", optarg); return -1; - } else - tmp_tcipt_globals.opts = opts; + } ok++; break; @@ -292,7 +302,6 @@ print_ipt(struct action_util *au, FILE * f, struct rtattr *arg) struct xtables_target *m; struct rtattr *tb[TCA_IPT_MAX + 1]; struct xt_entry_target *t = NULL; - struct option *opts = NULL; if (arg == NULL) return -1; @@ -339,21 +348,12 @@ print_ipt(struct action_util *au, FILE * f, struct rtattr *arg) return -1; } -#if (XTABLES_VERSION_CODE >= 6) - opts = xtables_options_xfrm(tmp_tcipt_globals.orig_opts, - tmp_tcipt_globals.opts, - m->x6_options, - &m->option_offset); -#else - opts = xtables_merge_options(tmp_tcipt_globals.opts, - m->extra_opts, - &m->option_offset); -#endif - if (opts == NULL) { - fprintf(stderr, " failed to find additional options for target %s\n\n", optarg); + if (get_xtables_target_opts(&tmp_tcipt_globals, m) < 0) { + fprintf(stderr, + " failed to find additional options for target %s\n\n", + t->u.user.name); return -1; - } else - tmp_tcipt_globals.opts = opts; + } fprintf(f, "\ttarget "); m->print(NULL, m->t, 0); if (tb[TCA_IPT_INDEX] == NULL) { From 4b83a08c280fcd14ed8e775adc7604ea13c1252f Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Tue, 14 Jun 2016 14:40:53 -0700 Subject: [PATCH 17/17] m_xt: whitespace cleanup Make it 99% checkpatch clean. --- tc/m_xt.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/tc/m_xt.c b/tc/m_xt.c index 1224e4fe..028bad64 100644 --- a/tc/m_xt.c +++ b/tc/m_xt.c @@ -39,8 +39,10 @@ #endif #ifndef __ALIGN_KERNEL -#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) -#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask)) +#define __ALIGN_KERNEL(x, a) \ + __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1) +#define __ALIGN_KERNEL_MASK(x, mask) \ + (((x) + (mask)) & ~(mask)) #endif #ifndef ALIGN @@ -51,7 +53,7 @@ static const char *tname = "mangle"; char *lib_dir; -static const char *ipthooks[] = { +static const char * const ipthooks[] = { "NF_IP_PRE_ROUTING", "NF_IP_LOCAL_IN", "NF_IP_FORWARD", @@ -153,6 +155,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, /* copy tcipt_globals because .opts will be modified by iptables */ struct xtables_globals tmp_tcipt_globals = tcipt_globals; + xtables_init_all(&tmp_tcipt_globals, NFPROTO_IPV4); set_lib_dir(); @@ -163,7 +166,8 @@ static int parse_ipt(struct action_util *a, int *argc_p, } if (argc <= 2) { - fprintf(stderr, "too few arguments for xt, need at least '-j '\n"); + fprintf(stderr, + "too few arguments for xt, need at least '-j '\n"); return -1; } @@ -175,7 +179,9 @@ static int parse_ipt(struct action_util *a, int *argc_p, case 'j': m = xtables_find_target(optarg, XTF_TRY_LOAD); if (!m) { - fprintf(stderr, " failed to find target %s\n\n", optarg); + fprintf(stderr, + " failed to find target %s\n\n", + optarg); return -1; } @@ -184,8 +190,11 @@ static int parse_ipt(struct action_util *a, int *argc_p, return -1; } - if (get_xtables_target_opts(&tmp_tcipt_globals, m) < 0) { - fprintf(stderr, " failed to find additional options for target %s\n\n", optarg); + if (get_xtables_target_opts(&tmp_tcipt_globals, + m) < 0) { + fprintf(stderr, + " failed to find additional options for target %s\n\n", + optarg); return -1; } ok++; @@ -198,10 +207,11 @@ static int parse_ipt(struct action_util *a, int *argc_p, #else if (m != NULL && m->parse != NULL) { m->parse(c - m->option_offset, argv, 0, - &m->tflags, NULL, &m->t); + &m->tflags, NULL, &m->t); #endif } else { - fprintf(stderr, "failed to find target %s\n\n", optarg); + fprintf(stderr, + "failed to find target %s\n\n", optarg); return -1; } @@ -297,7 +307,7 @@ static int parse_ipt(struct action_util *a, int *argc_p, } static int -print_ipt(struct action_util *au, FILE * f, struct rtattr *arg) +print_ipt(struct action_util *au, FILE *f, struct rtattr *arg) { struct xtables_target *m; struct rtattr *tb[TCA_IPT_MAX + 1]; @@ -350,7 +360,7 @@ print_ipt(struct action_util *au, FILE * f, struct rtattr *arg) if (get_xtables_target_opts(&tmp_tcipt_globals, m) < 0) { fprintf(stderr, - " failed to find additional options for target %s\n\n", + " failed to find additional options for target %s\n\n", t->u.user.name); return -1; }