ebtables-save: Fix counter formatting

The initial problem was 'ebtables-save -c' printing iptables-style
counters but at the same time not disabling ebtables-style counter
output (which was even printed in wrong format for ebtables-save).

The code around counter output was complicated enough to motivate a
larger rework:

* Make FMT_C_COUNTS indicate the appended counter style for ebtables.

* Use FMT_EBT_SAVE to distinguish between '-c' style counters and the
  legacy pcnt/bcnt ones.

Consequently, ebtables-save sets format to:

FMT_NOCOUNTS			- for no counters
FMT_EBT_SAVE			- for iptables-style counters
FMT_EBT_SAVE | FMT_C_COUNTS	- for '-c' style counters

For regular ebtables, list_rules() always sets FMT_C_COUNTS
(iptables-style counters are never used there) and FMT_NOCOUNTS if no
counters are requested.

The big plus is if neither FMT_NOCOUNTS nor FMT_C_COUNTS is set,
iptables-style counters are to be printed - both in iptables and
ebtables. This allows to drop the ebtables-specific 'save_counters'
callback.

Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index ddfbee165da937c827e8d416aececd1b1224cb7f..2e4b309b86135946b36611c34e2efe69b7883fde 100644 (file)
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -21,8 +21,6 @@
 #include "nft-bridge.h"
 #include "nft.h"
 
-static bool ebt_legacy_counter_fmt;
-
 void ebt_cs_clean(struct iptables_command_state *cs)
 {
        struct ebt_match *m, *nm;
@@ -422,22 +420,6 @@ static void print_protocol(uint16_t ethproto, bool invert, unsigned int bitmask)
                printf("%s ", ent->e_name);
 }
 
-static void nft_bridge_save_counters(const void *data)
-{
-       const char *ctr;
-
-       if (ebt_legacy_counter_fmt)
-               return;
-
-       ctr = getenv("EBTABLES_SAVE_COUNTER");
-       if (ctr) {
-               ebt_legacy_counter_fmt = true;
-               return;
-       }
-
-       save_counters(data);
-}
-
 static void nft_bridge_save_rule(const void *data, unsigned int format)
 {
        const struct iptables_command_state *cs = data;
@@ -474,15 +456,16 @@ static void nft_bridge_save_rule(const void *data, unsigned int format)
                cs->target->print(&cs->fw, cs->target->t, format & FMT_NUMERIC);
        }
 
-       if (format & FMT_EBT_SAVE)
-               printf(" -c %"PRIu64" %"PRIu64"",
-                      (uint64_t)cs->counters.pcnt,
-                      (uint64_t)cs->counters.bcnt);
-
-       if (!(format & FMT_NOCOUNTS))
-               printf(" , pcnt = %"PRIu64" -- bcnt = %"PRIu64"",
-                      (uint64_t)cs->counters.pcnt,
-                      (uint64_t)cs->counters.bcnt);
+       if ((format & (FMT_NOCOUNTS | FMT_C_COUNTS)) == FMT_C_COUNTS) {
+               if (format & FMT_EBT_SAVE)
+                       printf(" -c %"PRIu64" %"PRIu64"",
+                              (uint64_t)cs->counters.pcnt,
+                              (uint64_t)cs->counters.bcnt);
+               else
+                       printf(" , pcnt = %"PRIu64" -- bcnt = %"PRIu64"",
+                              (uint64_t)cs->counters.pcnt,
+                              (uint64_t)cs->counters.bcnt);
+       }
 
        if (!(format & FMT_NONEWLINE))
                fputc('\n', stdout);
@@ -763,7 +746,7 @@ struct nft_family_ops nft_family_ops_bridge = {
        .print_header           = nft_bridge_print_header,
        .print_rule             = nft_bridge_print_rule,
        .save_rule              = nft_bridge_save_rule,
-       .save_counters          = nft_bridge_save_counters,
+       .save_counters          = save_counters,
        .save_chain             = nft_bridge_save_chain,
        .post_parse             = NULL,
        .rule_to_cs             = nft_rule_to_ebtables_command_state,

diff --git a/iptables/tests/shell/testcases/ebtables/0004-save-counters_0 b/iptables/tests/shell/testcases/ebtables/0004-save-counters_0
new file mode 100755 (executable)
index 0000000..8348dc7
--- /dev/null
+++ b/iptables/tests/shell/testcases/ebtables/0004-save-counters_0
@@ -0,0 +1,67 @@
+#!/bin/bash
+
+set -e
+
+# there is no legacy backend to test
+[[ $XT_MULTI == */xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 0; }
+
+$XT_MULTI ebtables --init-table
+$XT_MULTI ebtables -A FORWARD -i nodev123 -o nodev432 -j ACCEPT
+$XT_MULTI ebtables -A FORWARD -i nodev432 -o nodev123 -j ACCEPT
+
+EXPECT='Bridge table: filter
+
+Bridge chain: FORWARD, entries: 2, policy: ACCEPT
+-i nodev123 -o nodev432 -j ACCEPT
+-i nodev432 -o nodev123 -j ACCEPT'
+
+echo "ebtables -L FORWARD"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables -L FORWARD)
+
+EXPECT='Bridge table: filter
+
+Bridge chain: FORWARD, entries: 2, policy: ACCEPT
+-i nodev123 -o nodev432 -j ACCEPT , pcnt = 0 -- bcnt = 0
+-i nodev432 -o nodev123 -j ACCEPT , pcnt = 0 -- bcnt = 0'
+
+echo "ebtables -L FORWARD --Lc"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables -L FORWARD --Lc)
+
+EXPECT='*filter
+:INPUT ACCEPT
+:FORWARD ACCEPT
+:OUTPUT ACCEPT
+-A FORWARD -i nodev123 -o nodev432 -j ACCEPT
+-A FORWARD -i nodev432 -o nodev123 -j ACCEPT
+'
+
+echo "ebtables-save"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save | grep -v '^#')
+
+EXPECT='*filter
+:INPUT ACCEPT
+:FORWARD ACCEPT
+:OUTPUT ACCEPT
+[0:0] -A FORWARD -i nodev123 -o nodev432 -j ACCEPT
+[0:0] -A FORWARD -i nodev432 -o nodev123 -j ACCEPT
+'
+
+echo "ebtables-save -c"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save -c | grep -v '^#')
+
+export EBTABLES_SAVE_COUNTER=yes
+
+# -c flag overrides EBTABLES_SAVE_COUNTER variable
+echo "EBTABLES_SAVE_COUNTER=yes ebtables-save -c"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save -c | grep -v '^#')
+
+EXPECT='*filter
+:INPUT ACCEPT
+:FORWARD ACCEPT
+:OUTPUT ACCEPT
+-A FORWARD -i nodev123 -o nodev432 -j ACCEPT -c 0 0
+-A FORWARD -i nodev432 -o nodev123 -j ACCEPT -c 0 0
+'
+
+echo "EBTABLES_SAVE_COUNTER=yes ebtables-save"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save | grep -v '^#')

diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index b8d89ad974a42eb20b676441c3f9bec359b7e423..121ecbecd0b640a7c31983425130a8687e14f09b 100644 (file)
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -410,7 +410,7 @@ static int list_rules(struct nft_handle *h, const char *chain, const char *table
 {
        unsigned int format;
 
-       format = FMT_OPTIONS;
+       format = FMT_OPTIONS | FMT_C_COUNTS;
        if (verbose)
                format |= FMT_VIA;
 

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index b8d19705771ed672415fff96503092dd99379b3e..491122f39bbb0d1d69c461b987156aba6a5abaa7 100644 (file)
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -274,7 +274,8 @@ static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters
        printf("*%s\n", tablename);
 
        if (counters)
-               format = ebt_legacy_counter_format ? FMT_EBT_SAVE : 0;
+               format = FMT_EBT_SAVE |
+                       (ebt_legacy_counter_format ? FMT_C_COUNTS : 0);
 
        /* Dump out chain names first,
         * thereby preventing dependency conflicts */