diff mbox series

[ovs-dev] do not send multicast packets to conntrack

Message ID 20211217141645.9931-1-dceara@redhat.com
State Accepted
Headers show
Series [ovs-dev] do not send multicast packets to conntrack | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Dumitru Ceara Dec. 17, 2021, 2:16 p.m. UTC
From: 张祖建 <zhangzujian.7@gmail.com>

Signed-off-by: 张祖建 <zhangzujian.7@gmail.com>
---
Originally posted at: https://github.com/ovn-org/ovn/pull/108
---
 northd/northd.c         | 10 ++++++++++
 northd/ovn-northd.8.xml | 44 ++++++++++++++++++++---------------------
 tests/ovn-northd.at     |  4 ++++
 3 files changed, 36 insertions(+), 22 deletions(-)

Comments

Dumitru Ceara Dec. 17, 2021, 2:20 p.m. UTC | #1
On 12/17/21 15:16, Dumitru Ceara wrote:
> From: 张祖建 <zhangzujian.7@gmail.com>
> 
> Signed-off-by: 张祖建 <zhangzujian.7@gmail.com>
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara <dceara@redhat.com>
Numan Siddique Dec. 17, 2021, 10:38 p.m. UTC | #2
On Fri, Dec 17, 2021 at 9:20 AM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On 12/17/21 15:16, Dumitru Ceara wrote:
> > From: 张祖建 <zhangzujian.7@gmail.com>
> >
> > Signed-off-by: 张祖建 <zhangzujian.7@gmail.com>
> > ---
>
> Looks good to me, thanks!
>
> Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks. I applied this patch to the main, branch-21.12 and
branch-21.09 branches.

It doesn't apply cleanly for branch-21.06.  If you want to backport to
other  branches,  please submit a branch specific patch or PR.

Regards
Numan

>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 5db6ff03d..c714227b2 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -5735,6 +5735,12 @@  build_pre_acls(struct ovn_datapath *od, const struct hmap *port_groups,
                       "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
                       "(udp && udp.src == 546 && udp.dst == 547)", "next;");
 
+        /* Do not send multicast packets to conntrack. */
+        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "eth.mcast",
+                      "next;");
+        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, "eth.mcast",
+                      "next;");
+
         /* Ingress and Egress Pre-ACL Table (Priority 100).
          *
          * Regardless of whether the ACL is "from-lport" or "to-lport",
@@ -5811,6 +5817,10 @@  build_empty_lb_event_flow(struct ovn_lb_vip *lb_vip,
 static void
 build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
 {
+    /* Do not send multicast packets to conntrack */
+    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, "eth.mcast", "next;");
+    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, "eth.mcast", "next;");
+
     /* Do not send ND packets to conntrack */
     ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
                   "nd || nd_rs || nd_ra || mldv1 || mldv2",
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index bd3c3aa26..79f35bc16 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -456,10 +456,10 @@ 
       <code>Pre-stateful</code> to send IP packets to the connection tracker
       before eventually advancing to ingress table <code>ACLs</code>. If
       special ports such as route ports or localnet ports can't use ct(), a
-      priority-110 flow is added to skip over stateful ACLs. IPv6 Neighbor
-      Discovery and MLD traffic also skips stateful ACLs. For "allow-stateless"
-      ACLs, a flow is added to bypass setting the hint for connection tracker
-      processing.
+      priority-110 flow is added to skip over stateful ACLs. Multicast, IPv6
+      Neighbor Discovery and MLD traffic also skips stateful ACLs. For
+      "allow-stateless" ACLs, a flow is added to bypass setting the hint for
+      connection tracker processing.
     </p>
 
     <p>
@@ -478,16 +478,16 @@ 
       This table prepares flows for possible stateful load balancing processing
       in ingress table <code>LB</code> and <code>Stateful</code>.  It contains
       a priority-0 flow that simply moves traffic to the next table. Moreover
-      it contains a priority-110 flow to move IPv6 Neighbor Discovery and MLD
-      traffic to the next table. If load balancing rules with virtual IP
-      addresses (and ports) are configured in <code>OVN_Northbound</code>
-      database for a logical switch datapath, a priority-100 flow is added
-      with the match <code>ip</code> to match on IP packets and sets the action
-      <code>reg0[2] = 1; next;</code> to act as a hint for table
-      <code>Pre-stateful</code> to send IP packets to the connection tracker
-      for packet de-fragmentation (and to possibly do DNAT for already
-      established load balanced traffic) before eventually advancing to ingress
-      table <code>Stateful</code>.
+      it contains two priority-110 flows to move multicast, IPv6 Neighbor
+      Discovery and MLD traffic to the next table. If load balancing rules with
+      virtual IP addresses (and ports) are configured in
+      <code>OVN_Northbound</code> database for a logical switch datapath, a
+      priority-100 flow is added with the match <code>ip</code> to match on IP
+      packets and sets the action <code>reg0[2] = 1; next;</code> to act as a
+      hint for table <code>Pre-stateful</code> to send IP packets to the
+      connection tracker for packet de-fragmentation (and to possibly do DNAT
+      for already established load balanced traffic) before eventually
+      advancing to ingress table <code>Stateful</code>.
       If controller_event has been enabled and load balancing rules with
       empty backends have been added in <code>OVN_Northbound</code>, a 130 flow
       is added to trigger ovn-controller events whenever the chassis receives a
@@ -1750,14 +1750,14 @@  output;
     <p>
       This table is similar to ingress table <code>Pre-LB</code>.  It
       contains a priority-0 flow that simply moves traffic to the next table.
-      Moreover it contains a priority-110 flow to move IPv6 Neighbor Discovery
-      traffic to the next table. If any load balancing rules exist for the
-      datapath, a priority-100 flow is added with a match of <code>ip</code>
-      and action of <code>reg0[2] = 1; next;</code> to act as a hint for
-      table <code>Pre-stateful</code> to send IP packets to the connection
-      tracker for packet de-fragmentation and possibly DNAT the destination
-      VIP to one of the selected backend for already commited load balanced
-      traffic.
+      Moreover it contains two priority-110 flows to move multicast, IPv6
+      Neighbor Discovery and MLD traffic to the next table. If any load
+      balancing rules exist for the datapath, a priority-100 flow is added with
+      a match of <code>ip</code> and action of <code>reg0[2] = 1; next;</code>
+      to act as a hint for table <code>Pre-stateful</code> to send IP packets
+      to the connection tracker for packet de-fragmentation and possibly DNAT
+      the destination VIP to one of the selected backend for already commited
+      load balanced traffic.
     </p>
 
     <p>
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 839ab62e7..652903761 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3872,6 +3872,7 @@  check_stateful_flows() {
   table=6 (ls_in_pre_lb       ), priority=0    , match=(1), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=100  , match=(ip), action=(reg0[[2]] = 1; next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(eth.dst == $svc_monitor_mac), action=(next;)
+  table=6 (ls_in_pre_lb       ), priority=110  , match=(eth.mcast), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(ip && inport == "sw0-lr0"), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(nd || nd_rs || nd_ra || mldv1 || mldv2), action=(next;)
 ])
@@ -3899,6 +3900,7 @@  check_stateful_flows() {
     AT_CHECK([grep "ls_out_pre_lb" sw0flows | sort], [0], [dnl
   table=0 (ls_out_pre_lb      ), priority=0    , match=(1), action=(next;)
   table=0 (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[2]] = 1; next;)
+  table=0 (ls_out_pre_lb      ), priority=110  , match=(eth.mcast), action=(next;)
   table=0 (ls_out_pre_lb      ), priority=110  , match=(eth.src == $svc_monitor_mac), action=(next;)
   table=0 (ls_out_pre_lb      ), priority=110  , match=(ip && outport == "sw0-lr0"), action=(next;)
   table=0 (ls_out_pre_lb      ), priority=110  , match=(nd || nd_rs || nd_ra || mldv1 || mldv2), action=(next;)
@@ -3938,6 +3940,7 @@  AT_CAPTURE_FILE([sw0flows])
 AT_CHECK([grep "ls_in_pre_lb" sw0flows | sort], [0], [dnl
   table=6 (ls_in_pre_lb       ), priority=0    , match=(1), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(eth.dst == $svc_monitor_mac), action=(next;)
+  table=6 (ls_in_pre_lb       ), priority=110  , match=(eth.mcast), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(ip && inport == "sw0-lr0"), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(nd || nd_rs || nd_ra || mldv1 || mldv2), action=(next;)
 ])
@@ -3962,6 +3965,7 @@  AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl
 
 AT_CHECK([grep "ls_out_pre_lb" sw0flows | sort], [0], [dnl
   table=0 (ls_out_pre_lb      ), priority=0    , match=(1), action=(next;)
+  table=0 (ls_out_pre_lb      ), priority=110  , match=(eth.mcast), action=(next;)
   table=0 (ls_out_pre_lb      ), priority=110  , match=(eth.src == $svc_monitor_mac), action=(next;)
   table=0 (ls_out_pre_lb      ), priority=110  , match=(ip && outport == "sw0-lr0"), action=(next;)
   table=0 (ls_out_pre_lb      ), priority=110  , match=(nd || nd_rs || nd_ra || mldv1 || mldv2), action=(next;)