bond: merge branch 'th/slb-bond-no-counters'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1623

7 files changed, 223 insertions, 30 deletions

diff --git a/src/core/nm-bond-manager.c b/src/core/nm-bond-manager.c
index 2d15b0b5a0..9985fccf11 100644
--- a/src/core/nm-bond-manager.c
+++ b/src/core/nm-bond-manager.c
@@ -438,6 +438,7 @@ _nft_call(NMBondManager     *self,
 {
     gs_unref_bytes GBytes     *stdin_buf             = NULL;
     gs_free const char *const *previous_members_strv = NULL;
+    gboolean                   with_counters;
 
     if (up) {
         gs_unref_ptrarray GPtrArray *arr = NULL;
@@ -480,11 +481,16 @@ _nft_call(NMBondManager     *self,
         }
     }
 
+    /* counters in the nft rules are convenient for debugging, but have a performance overhead.
+     * Enable counters based on whether NM logging is enabled. */
+    with_counters = _NMLOG_ENABLED(LOGL_TRACE);
+
     stdin_buf = nm_firewall_nft_stdio_mlag(up,
                                            bond_ifname,
                                            bond_ifnames_down,
                                            active_members,
-                                           previous_members_strv);
+                                           previous_members_strv,
+                                           with_counters);
 
     nm_clear_g_cancellable(&self->cancellable);
     self->cancellable = g_cancellable_new();
diff --git a/src/core/nm-firewall-utils.c b/src/core/nm-firewall-utils.c
index 03f1a9a5eb..ac12d3e432 100644
--- a/src/core/nm-firewall-utils.c
+++ b/src/core/nm-firewall-utils.c
@@ -763,13 +763,15 @@ nm_firewall_nft_stdio_mlag(gboolean           up,
                            const char        *bond_ifname,
                            const char *const *bond_ifnames_down,
                            const char *const *active_members,
-                           const char *const *previous_members)
+                           const char *const *previous_members,
+                           gboolean           with_counters)
 {
     nm_auto_str_buf NMStrBuf strbuf_table_name =
         NM_STR_BUF_INIT_A(NM_UTILS_GET_NEXT_REALLOC_SIZE_32, FALSE);
     nm_auto_str_buf NMStrBuf strbuf = NM_STR_BUF_INIT(NM_UTILS_GET_NEXT_REALLOC_SIZE_1000, FALSE);
     const char              *table_name;
     gsize                    i;
+    const char *const        s_counter = with_counters ? " counter" : "";
 
     if (NM_MORE_ASSERTS > 10 && active_members) {
         /* No duplicates. We make certain assumptions here, and we don't
@@ -876,9 +878,10 @@ nm_firewall_nft_stdio_mlag(gboolean           up,
             _append(&strbuf,
                     "add rule netdev %s %s pkttype {"
                     " broadcast, multicast "
-                    "} counter drop",
+                    "}%s drop",
                     table_name,
-                    chain_name);
+                    chain_name,
+                    s_counter);
         }
 
         /* OVS SLB rule 2
@@ -905,15 +908,17 @@ nm_firewall_nft_stdio_mlag(gboolean           up,
                 table_name,
                 bond_ifname);
         _append(&strbuf,
-                "add rule netdev %s tx-snoop-source-mac set update ether saddr . vlan id"
-                " timeout 5s @macset-tagged counter return"
+                "add rule netdev %s tx-snoop-source-mac set update ether saddr . vlan id "
+                "timeout 5s @macset-tagged%s return"
                 "", /* tagged */
-                table_name);
+                table_name,
+                s_counter);
         _append(&strbuf,
-                "add rule netdev %s tx-snoop-source-mac set update ether saddr"
-                " timeout 5s @macset-untagged counter"
+                "add rule netdev %s tx-snoop-source-mac set update ether saddr timeout 5s "
+                "@macset-untagged%s"
                 "", /* untagged*/
-                table_name);
+                table_name,
+                s_counter);
 
         _append(&strbuf,
                 "add chain netdev %s rx-drop-looped-packets {"
@@ -921,18 +926,20 @@ nm_firewall_nft_stdio_mlag(gboolean           up,
                 "}",
                 table_name,
                 bond_ifname);
+        _append(
+            &strbuf,
+            "add rule netdev %s rx-drop-looped-packets ether saddr . vlan id @macset-tagged%s drop",
+            table_name,
+            s_counter);
         _append(&strbuf,
-                "add rule netdev %s rx-drop-looped-packets ether saddr . vlan id"
-                " @macset-tagged counter drop",
-                table_name);
-        _append(&strbuf,
-                "add rule netdev %s rx-drop-looped-packets ether type vlan counter return"
+                "add rule netdev %s rx-drop-looped-packets ether type vlan%s return"
                 "", /* avoid looking up tagged packets in untagged table */
-                table_name);
+                table_name,
+                s_counter);
         _append(&strbuf,
-                "add rule netdev %s rx-drop-looped-packets ether saddr @macset-untagged"
-                " counter drop",
-                table_name);
+                "add rule netdev %s rx-drop-looped-packets ether saddr @macset-untagged%s drop",
+                table_name,
+                s_counter);
     }
 
 out:
diff --git a/src/core/nm-firewall-utils.h b/src/core/nm-firewall-utils.h
index ca138ccf78..9f13a5127e 100644
--- a/src/core/nm-firewall-utils.h
+++ b/src/core/nm-firewall-utils.h
@@ -39,6 +39,7 @@ GBytes *nm_firewall_nft_stdio_mlag(gboolean           up,
                                    const char        *bond_ifname,
                                    const char *const *bond_ifnames_down,
                                    const char *const *active_members,
-                                   const char *const *previous_members);
+                                   const char *const *previous_members,
+                                   gboolean           with_counters);
 
 #endif /* __NM_FIREWALL_UTILS_H__ */
diff --git a/src/core/tests/test-core.c b/src/core/tests/test-core.c
index bd6425d95c..a30cc3babc 100644
--- a/src/core/tests/test-core.c
+++ b/src/core/tests/test-core.c
@@ -18,6 +18,7 @@
 
 #include "dns/nm-dns-manager.h"
 #include "nm-connectivity.h"
+#include "nm-firewall-utils.h"
 
 #include "nm-test-utils-core.h"
 
@@ -2582,6 +2583,125 @@ test_connectivity_state_cmp(void)
 
 /*****************************************************************************/
 
+static void
+test_nm_firewall_nft_stdio_mlag(void)
+{
+#define _T(up,                                               \
+           bond_ifname,                                      \
+           bond_ifnames_down,                                \
+           active_members,                                   \
+           previous_members,                                 \
+           with_counters,                                    \
+           expected)                                         \
+    G_STMT_START                                             \
+    {                                                        \
+        gs_unref_bytes GBytes *_b = NULL;                    \
+                                                             \
+        _b = nm_firewall_nft_stdio_mlag((up),                \
+                                        (bond_ifname),       \
+                                        (bond_ifnames_down), \
+                                        (active_members),    \
+                                        (previous_members),  \
+                                        (with_counters));    \
+                                                             \
+        g_assert(_b);                                        \
+        nmtst_assert_cmpmem(expected,                        \
+                            NM_STRLEN(expected),             \
+                            g_bytes_get_data(_b, NULL),      \
+                            g_bytes_get_size(_b));           \
+    }                                                        \
+    G_STMT_END
+
+    _T(TRUE,
+       "bond0",
+       NM_MAKE_STRV("eth0"),
+       NM_MAKE_STRV("eth1"),
+       NM_MAKE_STRV("eth2"),
+       TRUE,
+       "add table netdev nm-mlag-eth0\012delete table netdev nm-mlag-eth0\012add table netdev "
+       "nm-mlag-bond0\012flush table netdev nm-mlag-bond0\012add chain netdev nm-mlag-bond0 "
+       "rx-drop-bc-mc-eth2 { type filter hook ingress device eth2 priority filter; }\012delete "
+       "chain netdev nm-mlag-bond0 rx-drop-bc-mc-eth2\012add chain netdev nm-mlag-bond0 "
+       "rx-drop-bc-mc-eth1 { type filter hook ingress device eth1 priority filter; }\012delete "
+       "chain netdev nm-mlag-bond0 rx-drop-bc-mc-eth1\012add set netdev nm-mlag-bond0 "
+       "macset-tagged { typeof ether saddr . vlan id; flags dynamic,timeout; }\012add set netdev "
+       "nm-mlag-bond0 macset-untagged { typeof ether saddr; flags dynamic,timeout; }\012add chain "
+       "netdev nm-mlag-bond0 tx-snoop-source-mac { type filter hook egress device bond0 priority "
+       "filter; }\012add rule netdev nm-mlag-bond0 tx-snoop-source-mac set update ether saddr . "
+       "vlan id timeout 5s @macset-tagged counter return\012add rule netdev nm-mlag-bond0 "
+       "tx-snoop-source-mac set update ether saddr timeout 5s @macset-untagged counter\012add "
+       "chain netdev nm-mlag-bond0 rx-drop-looped-packets { type filter hook ingress device bond0 "
+       "priority filter; }\012add rule netdev nm-mlag-bond0 rx-drop-looped-packets ether saddr . "
+       "vlan id @macset-tagged counter drop\012add rule netdev nm-mlag-bond0 "
+       "rx-drop-looped-packets ether type vlan counter return\012add rule netdev nm-mlag-bond0 "
+       "rx-drop-looped-packets ether saddr @macset-untagged counter drop\012");
+
+    _T(TRUE,
+       "bond0",
+       NM_MAKE_STRV("eth0"),
+       NM_MAKE_STRV("eth1"),
+       NM_MAKE_STRV("eth2"),
+       FALSE,
+       "add table netdev nm-mlag-eth0\012delete table netdev nm-mlag-eth0\012add table netdev "
+       "nm-mlag-bond0\012flush table netdev nm-mlag-bond0\012add chain netdev nm-mlag-bond0 "
+       "rx-drop-bc-mc-eth2 { type filter hook ingress device eth2 priority filter; }\012delete "
+       "chain netdev nm-mlag-bond0 rx-drop-bc-mc-eth2\012add chain netdev nm-mlag-bond0 "
+       "rx-drop-bc-mc-eth1 { type filter hook ingress device eth1 priority filter; }\012delete "
+       "chain netdev nm-mlag-bond0 rx-drop-bc-mc-eth1\012add set netdev nm-mlag-bond0 "
+       "macset-tagged { typeof ether saddr . vlan id; flags dynamic,timeout; }\012add set netdev "
+       "nm-mlag-bond0 macset-untagged { typeof ether saddr; flags dynamic,timeout; }\012add chain "
+       "netdev nm-mlag-bond0 tx-snoop-source-mac { type filter hook egress device bond0 priority "
+       "filter; }\012add rule netdev nm-mlag-bond0 tx-snoop-source-mac set update ether saddr . "
+       "vlan id timeout 5s @macset-tagged return\012add rule netdev nm-mlag-bond0 "
+       "tx-snoop-source-mac set update ether saddr timeout 5s @macset-untagged\012add chain netdev "
+       "nm-mlag-bond0 rx-drop-looped-packets { type filter hook ingress device bond0 priority "
+       "filter; }\012add rule netdev nm-mlag-bond0 rx-drop-looped-packets ether saddr . vlan id "
+       "@macset-tagged drop\012add rule netdev nm-mlag-bond0 rx-drop-looped-packets ether type "
+       "vlan return\012add rule netdev nm-mlag-bond0 rx-drop-looped-packets ether saddr "
+       "@macset-untagged drop\012");
+
+    _T(TRUE,
+       "bond0",
+       NM_MAKE_STRV("eth0", "eth1"),
+       NM_MAKE_STRV("eth2", "eth3"),
+       NM_MAKE_STRV("eth4", "eth5"),
+       FALSE,
+       "add table netdev nm-mlag-eth0\012delete table netdev nm-mlag-eth0\012add table netdev "
+       "nm-mlag-eth1\012delete table netdev nm-mlag-eth1\012add table netdev "
+       "nm-mlag-bond0\012flush table netdev nm-mlag-bond0\012add chain netdev nm-mlag-bond0 "
+       "rx-drop-bc-mc-eth4 { type filter hook ingress device eth4 priority filter; }\012delete "
+       "chain netdev nm-mlag-bond0 rx-drop-bc-mc-eth4\012add chain netdev nm-mlag-bond0 "
+       "rx-drop-bc-mc-eth5 { type filter hook ingress device eth5 priority filter; }\012delete "
+       "chain netdev nm-mlag-bond0 rx-drop-bc-mc-eth5\012add chain netdev nm-mlag-bond0 "
+       "rx-drop-bc-mc-eth2 { type filter hook ingress device eth2 priority filter; }\012delete "
+       "chain netdev nm-mlag-bond0 rx-drop-bc-mc-eth2\012add chain netdev nm-mlag-bond0 "
+       "rx-drop-bc-mc-eth3 { type filter hook ingress device eth3 priority filter; }\012add rule "
+       "netdev nm-mlag-bond0 rx-drop-bc-mc-eth3 pkttype { broadcast, multicast } drop\012add set "
+       "netdev nm-mlag-bond0 macset-tagged { typeof ether saddr . vlan id; flags dynamic,timeout; "
+       "}\012add set netdev nm-mlag-bond0 macset-untagged { typeof ether saddr; flags "
+       "dynamic,timeout; }\012add chain netdev nm-mlag-bond0 tx-snoop-source-mac { type filter "
+       "hook egress device bond0 priority filter; }\012add rule netdev nm-mlag-bond0 "
+       "tx-snoop-source-mac set update ether saddr . vlan id timeout 5s @macset-tagged "
+       "return\012add rule netdev nm-mlag-bond0 tx-snoop-source-mac set update ether saddr timeout "
+       "5s @macset-untagged\012add chain netdev nm-mlag-bond0 rx-drop-looped-packets { type filter "
+       "hook ingress device bond0 priority filter; }\012add rule netdev nm-mlag-bond0 "
+       "rx-drop-looped-packets ether saddr . vlan id @macset-tagged drop\012add rule netdev "
+       "nm-mlag-bond0 rx-drop-looped-packets ether type vlan return\012add rule netdev "
+       "nm-mlag-bond0 rx-drop-looped-packets ether saddr @macset-untagged drop\012");
+
+    _T(FALSE,
+       "bond0",
+       NM_MAKE_STRV("eth0", "eth1"),
+       NM_MAKE_STRV("eth2", "eth3"),
+       NM_MAKE_STRV("eth4", "eth5"),
+       FALSE,
+       "add table netdev nm-mlag-eth0\012delete table netdev nm-mlag-eth0\012add table netdev "
+       "nm-mlag-eth1\012delete table netdev nm-mlag-eth1\012add table netdev "
+       "nm-mlag-bond0\012delete table netdev nm-mlag-bond0\012");
+}
+
+/*****************************************************************************/
+
 NMTST_DEFINE();
 
 int
@@ -2656,5 +2776,7 @@ main(int argc, char **argv)
     g_test_add_func("/core/general/test_kernel_cmdline_match_check",
                     test_kernel_cmdline_match_check);
 
+    g_test_add_func("/core/test_nm_firewall_nft_stdio_mlag", test_nm_firewall_nft_stdio_mlag);
+
     return g_test_run();
 }
diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c
index 4a822d12cd..a51ff765d7 100644
--- a/src/libnm-glib-aux/nm-shared-utils.c
+++ b/src/libnm-glib-aux/nm-shared-utils.c
@@ -2757,13 +2757,16 @@ nm_utils_buf_utf8safe_escape(gconstpointer           buf,
     if (g_utf8_validate(str, buflen, &p) && nul_terminated) {
         /* note that g_utf8_validate() does not allow NUL character inside @str. Good.
          * We can treat @str like a NUL terminated string. */
-        if (!NM_STRCHAR_ANY(str,
-                            ch,
-                            (ch == '\\'
-                             || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL)
-                                 && nm_ascii_is_ctrl_or_del(ch))
-                             || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII)
-                                 && nm_ascii_is_non_ascii(ch)))))
+        if (!NM_STRCHAR_ANY(
+                str,
+                ch,
+                (ch == '\\'
+                 || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL)
+                     && nm_ascii_is_ctrl_or_del(ch))
+                 || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII)
+                     && nm_ascii_is_non_ascii(ch))
+                 || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_DOUBLE_QUOTE)
+                     && ch == '"'))))
             return str;
     }
 
@@ -2783,7 +2786,9 @@ nm_utils_buf_utf8safe_escape(gconstpointer           buf,
             else if ((NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL)
                       && nm_ascii_is_ctrl_or_del(ch))
                      || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII)
-                         && nm_ascii_is_non_ascii(ch)))
+                         && nm_ascii_is_non_ascii(ch))
+                     || (NM_FLAGS_HAS(flags, NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_DOUBLE_QUOTE)
+                         && ch == '"'))
                 _str_buf_append_c_escape_octal(&strbuf, ch);
             else
                 nm_str_buf_append_c(&strbuf, ch);
diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h
index 7233d77d40..60735d3c8d 100644
--- a/src/libnm-glib-aux/nm-shared-utils.h
+++ b/src/libnm-glib-aux/nm-shared-utils.h
@@ -1252,12 +1252,16 @@ typedef enum {
      * It will backslash escape ascii characters according to nm_ascii_is_non_ascii(). */
     NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_NON_ASCII = 0x0002,
 
+    /* Escape '"' as ASCII "\\042". This is useful when escaping a string so that
+     * it can be unescaped with `echo -e $PASTE_TEXT`. */
+    NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_DOUBLE_QUOTE = 0x0004,
+
     /* This flag only has an effect during escaping to ensure we
      * don't leak secrets in memory. Note that during unescape we
      * know the maximum result size from the beginning, and no
      * reallocation happens. Thus, unescape always avoids leaking
      * secrets already. */
-    NM_UTILS_STR_UTF8_SAFE_FLAG_SECRET = 0x0004,
+    NM_UTILS_STR_UTF8_SAFE_FLAG_SECRET = 0x0008,
 
     /* This flag only has an effect during unescaping. It means
      * that non-escaped whitespaces (g_ascii_isspace()) will be
@@ -1265,7 +1269,7 @@ typedef enum {
      * this flag is only useful for gracefully accepting user input
      * with spaces. With this flag, escape and unescape may no longer
      * yield the original input. */
-    NM_UTILS_STR_UTF8_SAFE_UNESCAPE_STRIP_SPACES = 0x0008,
+    NM_UTILS_STR_UTF8_SAFE_UNESCAPE_STRIP_SPACES = 0x0010,
 } NMUtilsStrUtf8SafeFlags;
 
 const char *nm_utils_buf_utf8safe_escape(gconstpointer           buf,
diff --git a/src/libnm-glib-aux/nm-test-utils.h b/src/libnm-glib-aux/nm-test-utils.h
index 4d99e643e9..d4e1e4b1f4 100644
--- a/src/libnm-glib-aux/nm-test-utils.h
+++ b/src/libnm-glib-aux/nm-test-utils.h
@@ -203,6 +203,54 @@
     }                                                             \
     G_STMT_END
 
+#define nmtst_assert_cmpmem(m1, l1, m2, l2)                                                     \
+    G_STMT_START                                                                                \
+    {                                                                                           \
+        const guint8 *const _m1 = (gpointer) (m1);                                              \
+        const guint8 *const _m2 = (gpointer) (m2);                                              \
+        const gsize         _l1 = (l1);                                                         \
+        const gsize         _l2 = (l2);                                                         \
+                                                                                                \
+        /* This is like g_assert_cmpmem(), however on failure it actually
+         * prints the compared buffer contents, which is useful for debugging
+         * the test failure. */                       \
+                                                                                                \
+        g_assert(_l1 == 0 || _m1);                                                              \
+        g_assert(_l2 == 0 || _m2);                                                              \
+                                                                                                \
+        if (_l1 != _l2 || (_l1 > 0 && memcmp(_m1, _m2, _l1) != 0)) {                            \
+            gs_free char *_s1 = NULL;                                                           \
+            gs_free char *_s2 = NULL;                                                           \
+                                                                                                \
+            g_error(                                                                            \
+                "ERROR: %s:%d : buffer [\"%s\" (%s, %zu bytes)] differs from [\"%s\" (%s, %zu " \
+                "bytes)]:\n"                                                                    \
+                "   a=[ \"%s\" ]\n"                                                             \
+                "   b=[ \"%s\" ]\n",                                                            \
+                __FILE__,                                                                       \
+                (int) __LINE__,                                                                 \
+                #m1,                                                                            \
+                #l1,                                                                            \
+                _l1,                                                                            \
+                #m2,                                                                            \
+                #l2,                                                                            \
+                _l2,                                                                            \
+                (_s1 = nm_utils_buf_utf8safe_escape_cp(                                         \
+                     _m1,                                                                       \
+                     _l1,                                                                       \
+                     NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL                                    \
+                         | NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_DOUBLE_QUOTE))                    \
+                    ?: "",                                                                      \
+                (_s2 = nm_utils_buf_utf8safe_escape_cp(                                         \
+                     _m2,                                                                       \
+                     _l2,                                                                       \
+                     NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_CTRL                                    \
+                         | NM_UTILS_STR_UTF8_SAFE_FLAG_ESCAPE_DOUBLE_QUOTE))                    \
+                    ?: "");                                                                     \
+        }                                                                                       \
+    }                                                                                           \
+    G_STMT_END
+
 /*****************************************************************************/
 
 /* Our nm-error error numbers use negative values to signal failure.