Omit ACLs for nd || nd_ra || nd_rs || mldv1 || mldv2

When setting flows for LS, OVN distinguishes between two states: where
there’s a stateful ACL present in its list (has_stateful == true *)
and when it’s missing (all ACLs are stateless).

When has_stateful == true, the following is done (among other things):
- ct handling flows are set;
- they are omitted by a higher priority flow for “service” protocols:
  NA, RA, MLD.

The latter is done because of a known issue in kernel ct
implementation for the protocols:

* https://bugzilla.kernel.org/show_bug.cgi?id=11797

The assumption is that by default OVN allows all traffic unless
explicitly forbidden, so omitting ct flows only avoids ct machinery
but doesn't affect functional behavior of flow tables for the
protocols.

But if an ACL that forbids these protocols is configured, because of
the ct omittance, this ACL is not in effect. (But only when
has_stateful == true.)

This behavior results in inconsistent and confusing behavior in
OpenStack Neutron where

(1) the default security group behavior is drop all IP traffic
(achieved with default "drop" Port_Group); and
(2) ports that have stateful and stateless ACLs configured can
co-exist in the same network.

In which case, depending on other "stateful" ports present in the
network, "stateless" ports may or may not observe RA / NA / MLD
traffic. Which affects their IPv6 address configuration.

In this patch, I suggest that we don't make RA / NA / MLD behavior
dependent on whether "stateful" ACLs are present. Instead, make the
protocols always allowed, regardless of ACLs configured (whether
stateful ACLs or ACLs that forbid packets of these protocols).

Note: an argument can be made that the same "always-on" behavior
should be guaranteed for ARP protocol that serves a similar goal in a
IPv4 network as RA / NA do for IPv6 networks. This scenario is not
directly related to the inconsistency between "purely stateless" and
"mixed-stateful" networks and hence is left for a follow-up patch.

Note: this patch carries a test case that utilizes scapy tool to
construct packets for the protocols under test. A proper backport may
demand backporting scapy related patches too.

Reported-At: https://bugs.launchpad.net/neutron/+bug/2006949
Reported-At: https://bugzilla.redhat.com/show_bug.cgi?id=2149731
Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>

NEWS

@@ -12,6 +12,8 @@ Post v23.03.0
     target mac address matches. Arp requests matching no Logical Router will
     only be forwarded to non-router ports. Default is true which keeps the
     existing behaviour of flooding these arp requests to all attached Ports.
+  - Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast
+    Listener Discovery protocols, regardless of ACLs defined.
 
 OVN v23.03.0 - 03 Mar 2023
 --------------------------

northd/northd.c

@@ -6799,6 +6799,8 @@ build_port_group_lswitches(
     }
 }
 
+#define IPV6_CT_OMIT_MATCH "nd || nd_ra || nd_rs || mldv1 || mldv2"
+
 static void
 build_acls(struct ovn_datapath *od, const struct chassis_features *features,
            struct hmap *lflows, const struct hmap *port_groups,
@@ -6946,20 +6948,26 @@ build_acls(struct ovn_datapath *od, const struct chassis_features *features,
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3,
                       ds_cstr(&match), ct_out_acl_action);
 
-        /* Ingress and Egress ACL Table (Priority 65532).
-         *
-         * Not to do conntrack on ND packets. */
-        ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3,
-                      "nd || nd_ra || nd_rs || mldv1 || mldv2", "next;");
-        ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3,
-                      "nd || nd_ra || nd_rs || mldv1 || mldv2", "next;");
-
         /* Reply and related traffic matched by an "allow-related" ACL
          * should be allowed in the ls_in_acl_after_lb stage too. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_AFTER_LB, UINT16_MAX - 3,
                       REGBIT_ACL_HINT_ALLOW_REL" == 1", "next;");
     }
 
+    /* Ingress and Egress ACL Table (Priority 65532).
+     *
+     * Always allow service IPv6 protocols regardless of other ACLs defined.
+     *
+     * Also, don't send them to conntrack because session tracking
+     * for these protocols is not working properly:
+     * https://bugzilla.kernel.org/show_bug.cgi?id=11797. */
+    ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3,
+                  IPV6_CT_OMIT_MATCH, "next;");
+    ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3,
+                  IPV6_CT_OMIT_MATCH, "next;");
+    ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_AFTER_LB, UINT16_MAX - 3,
+                  IPV6_CT_OMIT_MATCH, "next;");
+
     /* Ingress or Egress ACL Table (Various priorities). */
     for (size_t i = 0; i < od->nbs->n_acls; i++) {
         struct nbrec_acl *acl = od->nbs->acls[i];

northd/ovn-northd.8.xml

@@ -748,6 +748,12 @@
       drop behavior.
     </p>
 
+    <p>
+      A priority-65532 flow is added to allow IPv6 Neighbor solicitation,
+      Neighbor discover, Router solicitation, Router advertisement and MLD
+      packets regardless of other ACLs defined.
+    </p>
+
     <p>
       If the logical datapath has a stateful ACL or a load balancer with VIP
       configured, the following flows will also be added:
@@ -824,12 +830,6 @@
         in the request direction are skipped here to let a newly created
         ACL re-allow this connection.
       </li>
-
-      <li>
-        A priority-65532 flow that allows IPv6 Neighbor solicitation,
-        Neighbor discover, Router solicitation, Router advertisement and MLD
-        packets.
-      </li>
     </ul>
 
     <p>
@@ -2123,6 +2123,12 @@ output;
       <code>to-lport</code> ACLs.
     </p>
 
+    <p>
+      Similar to ingress table, a priority-65532 flow is added to allow IPv6
+      Neighbor solicitation, Neighbor discover, Router solicitation, Router
+      advertisement and MLD packets regardless of other ACLs defined.
+    </p>
+
     <p>
       In addition, the following flows are added.
     </p>