From 2c3f7a2cd6fd8325b3a84e280cce945c6c20b87f Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Thu, 31 Jan 2019 16:12:54 +0100 Subject: arptables-nft-save: Fix position of -j option Legacy arptables-save (just like arptables itself) prints verdict as first option, then matches and finally any target options. To achieve this without introducing double/trailing spaces everywhere, integrate target ('-j') option printing into nft_arp_print_rule_details() and make it print separating whitespace before each option. In nft_arp_save_rule(), replace the call to save_matches_and_target() by by a direct call to cs->target->save() since the former prints '-j' option itself. Since there are no match extensions in arptables, any other code from that function is not needed. Signed-off-by: Phil Sutter Signed-off-by: Florian Westphal --- iptables/nft-arp.c | 65 +++++++++++++--------- .../arptables/0001-arptables-save-restore_0 | 32 +++++------ .../arptables/0002-arptables-restore-defaults_0 | 6 +- 3 files changed, 58 insertions(+), 45 deletions(-) diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c index b0405be8..3dc0b953 100644 --- a/iptables/nft-arp.c +++ b/iptables/nft-arp.c @@ -434,14 +434,21 @@ static void nft_arp_print_header(unsigned int format, const char *chain, } } -static void nft_arp_print_rule_details(const struct arpt_entry *fw, +static void nft_arp_print_rule_details(const struct iptables_command_state *cs, unsigned int format) { + const struct arpt_entry *fw = &cs->arp; char buf[BUFSIZ]; char iface[IFNAMSIZ+2]; + const char *sep = ""; int print_iface = 0; int i; + if (strlen(cs->jumpto)) { + printf("%s-j %s", sep, cs->jumpto); + sep = " "; + } + iface[0] = '\0'; if (fw->arp.iniface[0] != '\0') { @@ -453,9 +460,11 @@ static void nft_arp_print_rule_details(const struct arpt_entry *fw, if (format & FMT_NUMERIC) strcat(iface, "*"); else strcat(iface, "any"); } - if (print_iface) - printf("%s-i %s ", fw->arp.invflags & ARPT_INV_VIA_IN ? + if (print_iface) { + printf("%s%s-i %s", sep, fw->arp.invflags & ARPT_INV_VIA_IN ? "! " : "", iface); + sep = " "; + } print_iface = 0; iface[0] = '\0'; @@ -469,12 +478,14 @@ static void nft_arp_print_rule_details(const struct arpt_entry *fw, if (format & FMT_NUMERIC) strcat(iface, "*"); else strcat(iface, "any"); } - if (print_iface) - printf("%s-o %s ", fw->arp.invflags & ARPT_INV_VIA_OUT ? + if (print_iface) { + printf("%s%s-o %s", sep, fw->arp.invflags & ARPT_INV_VIA_OUT ? "! " : "", iface); + sep = " "; + } if (fw->arp.smsk.s_addr != 0L) { - printf("%s", fw->arp.invflags & ARPT_INV_SRCIP + printf("%s%s", sep, fw->arp.invflags & ARPT_INV_SRCIP ? "! " : ""); if (format & FMT_NUMERIC) sprintf(buf, "%s", addr_to_dotted(&(fw->arp.src))); @@ -482,7 +493,8 @@ static void nft_arp_print_rule_details(const struct arpt_entry *fw, sprintf(buf, "%s", addr_to_anyname(&(fw->arp.src))); strncat(buf, mask_to_dotted(&(fw->arp.smsk)), sizeof(buf) - strlen(buf) - 1); - printf("-s %s ", buf); + printf("-s %s", buf); + sep = " "; } for (i = 0; i < ARPT_DEV_ADDR_LEN_MAX; i++) @@ -490,16 +502,16 @@ static void nft_arp_print_rule_details(const struct arpt_entry *fw, break; if (i == ARPT_DEV_ADDR_LEN_MAX) goto after_devsrc; - printf("%s", fw->arp.invflags & ARPT_INV_SRCDEVADDR + printf("%s%s", sep, fw->arp.invflags & ARPT_INV_SRCDEVADDR ? "! " : ""); printf("--src-mac "); print_mac_and_mask((unsigned char *)fw->arp.src_devaddr.addr, (unsigned char *)fw->arp.src_devaddr.mask, ETH_ALEN); - printf(" "); + sep = " "; after_devsrc: if (fw->arp.tmsk.s_addr != 0L) { - printf("%s", fw->arp.invflags & ARPT_INV_TGTIP + printf("%s%s", sep, fw->arp.invflags & ARPT_INV_TGTIP ? "! " : ""); if (format & FMT_NUMERIC) sprintf(buf, "%s", addr_to_dotted(&(fw->arp.tgt))); @@ -507,7 +519,8 @@ after_devsrc: sprintf(buf, "%s", addr_to_anyname(&(fw->arp.tgt))); strncat(buf, mask_to_dotted(&(fw->arp.tmsk)), sizeof(buf) - strlen(buf) - 1); - printf("-d %s ", buf); + printf("-d %s", buf); + sep = " "; } for (i = 0; i arp.invflags & ARPT_INV_TGTDEVADDR + printf("%s%s", sep, fw->arp.invflags & ARPT_INV_TGTDEVADDR ? "! " : ""); printf("--dst-mac "); print_mac_and_mask((unsigned char *)fw->arp.tgt_devaddr.addr, (unsigned char *)fw->arp.tgt_devaddr.mask, ETH_ALEN); - printf(" "); + sep = " "; after_devdst: if (fw->arp.arhln_mask != 0) { - printf("%s", fw->arp.invflags & ARPT_INV_ARPHLN + printf("%s%s", sep, fw->arp.invflags & ARPT_INV_ARPHLN ? "! " : ""); printf("--h-length %d", fw->arp.arhln); if (fw->arp.arhln_mask != 255) printf("/%d", fw->arp.arhln_mask); - printf(" "); + sep = " "; } if (fw->arp.arpop_mask != 0) { int tmp = ntohs(fw->arp.arpop); - printf("%s", fw->arp.invflags & ARPT_INV_ARPOP + printf("%s%s", sep, fw->arp.invflags & ARPT_INV_ARPOP ? "! " : ""); if (tmp <= NUMOPCODES && !(format & FMT_NUMERIC)) printf("--opcode %s", arp_opcodes[tmp-1]); @@ -545,13 +558,13 @@ after_devdst: if (fw->arp.arpop_mask != 65535) printf("/%d", ntohs(fw->arp.arpop_mask)); - printf(" "); + sep = " "; } if (fw->arp.arhrd_mask != 0) { uint16_t tmp = ntohs(fw->arp.arhrd); - printf("%s", fw->arp.invflags & ARPT_INV_ARPHRD + printf("%s%s", sep, fw->arp.invflags & ARPT_INV_ARPHRD ? "! " : ""); if (tmp == 1 && !(format & FMT_NUMERIC)) printf("--h-type %s", "Ethernet"); @@ -559,13 +572,13 @@ after_devdst: printf("--h-type %u", tmp); if (fw->arp.arhrd_mask != 65535) printf("/%d", ntohs(fw->arp.arhrd_mask)); - printf(" "); + sep = " "; } if (fw->arp.arpro_mask != 0) { int tmp = ntohs(fw->arp.arpro); - printf("%s", fw->arp.invflags & ARPT_INV_ARPPRO + printf("%s%s", sep, fw->arp.invflags & ARPT_INV_ARPPRO ? "! " : ""); if (tmp == 0x0800 && !(format & FMT_NUMERIC)) printf("--proto-type %s", "IPv4"); @@ -573,7 +586,7 @@ after_devdst: printf("--proto-type 0x%x", tmp); if (fw->arp.arpro_mask != 65535) printf("/%x", ntohs(fw->arp.arpro_mask)); - printf(" "); + sep = " "; } } @@ -584,8 +597,10 @@ nft_arp_save_rule(const void *data, unsigned int format) format |= FMT_NUMERIC; - nft_arp_print_rule_details(&cs->arp, format); - save_matches_and_target(cs, false, &cs->arp, format); + nft_arp_print_rule_details(cs, format); + if (cs->target && cs->target->save) + cs->target->save(&cs->fw, cs->target->t); + printf("\n"); } static void @@ -598,9 +613,7 @@ nft_arp_print_rule(struct nftnl_rule *r, unsigned int num, unsigned int format) nft_rule_to_iptables_command_state(r, &cs); - if (strlen(cs.jumpto)) - printf("-j %s ", cs.jumpto); - nft_arp_print_rule_details(&cs.arp, format); + nft_arp_print_rule_details(&cs, format); print_matches_and_target(&cs, format); if (!(format & FMT_NOCOUNTS)) { diff --git a/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0 b/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0 index f8629551..0664e3b3 100755 --- a/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0 +++ b/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0 @@ -35,22 +35,22 @@ DUMP='*filter :INPUT ACCEPT :OUTPUT DROP :foo - --A INPUT -s 10.0.0.0/8 --h-length 6 --h-type 1 -j ACCEPT --A INPUT -d 192.168.123.1 --h-length 6 --h-type 1 -j ACCEPT --A INPUT --src-mac fe:ed:ba:be:00:01 --h-length 6 --h-type 1 -j ACCEPT --A INPUT --dst-mac fe:ed:ba:be:00:01 --h-length 6 --h-type 1 -j ACCEPT --A INPUT --h-length 6 --h-type 1 -j foo --A INPUT --h-length 6 --h-type 1 --A OUTPUT -o lo --h-length 6 --h-type 1 -j ACCEPT --A OUTPUT -o eth134 --h-length 6 --h-type 1 -j mangle --mangle-ip-s 10.0.0.1 --A OUTPUT -o eth432 --h-length 6 --h-type 1 -j CLASSIFY --set-class feed:babe --A OUTPUT -o eth432 --h-length 6 --opcode 1 --h-type 1 -j CLASSIFY --set-class feed:babe --A foo -i lo --h-length 6 --h-type 1 -j ACCEPT --A foo --h-length 6 --h-type 1 -j ACCEPT --A foo --h-length 6 --h-type 1 -j MARK --set-mark 12345 --A foo --h-length 6 --opcode 1 --h-type 1 -j ACCEPT --A foo --h-length 6 --h-type 1 --proto-type 0x800 -j ACCEPT --A foo -i lo --h-length 6 --opcode 1 --h-type 1 --proto-type 0x800 -j ACCEPT +-A INPUT -j ACCEPT -s 10.0.0.0/8 --h-length 6 --h-type 1 +-A INPUT -j ACCEPT -d 192.168.123.1 --h-length 6 --h-type 1 +-A INPUT -j ACCEPT --src-mac fe:ed:ba:be:00:01 --h-length 6 --h-type 1 +-A INPUT -j ACCEPT --dst-mac fe:ed:ba:be:00:01 --h-length 6 --h-type 1 +-A INPUT -j foo --h-length 6 --h-type 1 +-A INPUT --h-length 6 --h-type 1 +-A OUTPUT -j ACCEPT -o lo --h-length 6 --h-type 1 +-A OUTPUT -j mangle -o eth134 --h-length 6 --h-type 1 --mangle-ip-s 10.0.0.1 +-A OUTPUT -j CLASSIFY -o eth432 --h-length 6 --h-type 1 --set-class feed:babe +-A OUTPUT -j CLASSIFY -o eth432 --h-length 6 --opcode 1 --h-type 1 --set-class feed:babe +-A foo -j ACCEPT -i lo --h-length 6 --h-type 1 +-A foo -j ACCEPT --h-length 6 --h-type 1 +-A foo -j MARK --h-length 6 --h-type 1 --set-mark 12345 +-A foo -j ACCEPT --h-length 6 --opcode 1 --h-type 1 +-A foo -j ACCEPT --h-length 6 --h-type 1 --proto-type 0x800 +-A foo -j ACCEPT -i lo --h-length 6 --opcode 1 --h-type 1 --proto-type 0x800 ' diff -u <(echo -e "$DUMP") <($XT_MULTI arptables-save) diff --git a/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0 b/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0 index ee17da00..d742c3d5 100755 --- a/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0 +++ b/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0 @@ -11,7 +11,7 @@ set -e DUMP='*filter :OUTPUT ACCEPT -A OUTPUT -j mangle --mangle-ip-s 10.0.0.1 --A OUTPUT --h-length 6 --h-type 1 -j mangle --mangle-ip-d 10.0.0.2 +-A OUTPUT -j mangle --h-length 6 --h-type 1 --mangle-ip-d 10.0.0.2 ' # note how mangle-ip-s is unset in second rule @@ -19,8 +19,8 @@ DUMP='*filter EXPECT='*filter :INPUT ACCEPT :OUTPUT ACCEPT --A OUTPUT --h-length 6 --h-type 1 -j mangle --mangle-ip-s 10.0.0.1 --A OUTPUT --h-length 6 --h-type 1 -j mangle --mangle-ip-d 10.0.0.2 +-A OUTPUT -j mangle --h-length 6 --h-type 1 --mangle-ip-s 10.0.0.1 +-A OUTPUT -j mangle --h-length 6 --h-type 1 --mangle-ip-d 10.0.0.2 ' $XT_MULTI arptables -F -- cgit v1.2.3