diff mbox series

[ovs-dev,branch-23.09] logical-fields: Add missing multicast matches for MLD and IGMP.

Message ID 20240809153538.3717625-1-i.maximets@ovn.org
State Accepted
Headers show
Series [ovs-dev,branch-23.09] logical-fields: Add missing multicast matches for MLD and IGMP. | expand

Checks

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

Commit Message

Ilya Maximets Aug. 9, 2024, 3:35 p.m. UTC
MLD flows are added to pipelines unconditionally in order to avoid
sending such traffic through conntrack.  The problem is that these
matches turn into matches on ip6.dst that end up as exact matches in
datapath flows.  This means a separate datapath flow per destination
IP address.  This may cause significant performance issues in setups
where traffic for many different IP addresses is passing through.
Since network protocol is stored further in the packet, it is evaluated
after checking the IP addresses, and so having a match on ip.proto
doesn't save us in this scenario.

MLD packets are all supposed to be multicast packets and so they all
should have multicast destination ethernet addresses.  Add the missing
eth.mcast6 match to all such packets.  This ensures that non-multicast
traffic will quickly fail the OpenFlow lookup on such rules and the bits
from higher layers will not be added to the match criteria in datapath
flows.  This also ensures that OVN doesn't handle incorrect MLD packets.

There are still ND responder flows that can add extra matches for IPv6
addresses, but they can be disabled or handled by other means.

IGMP did not check for IP address being multicast for some reason, so
it didn't cause issues for IPv4 traffic.  But let's fix it as well.

Tests were using incorrect multicast addresses, fixed now.

Reported-at: https://issues.redhat.com/browse/FDP-728
Reported-by: Mike Pattrick <mkp@redhat.com>
Fixes: 677a3ba4d66b ("ovn: Add MLD support.")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit 43c34f2e6676af87e3ca80c5a16d56c73e685963)
---
 lib/logical-fields.c |  7 ++++---
 tests/ovn.at         | 23 +++++++++++++++++------
 2 files changed, 21 insertions(+), 9 deletions(-)
diff mbox series

Patch

diff --git a/lib/logical-fields.c b/lib/logical-fields.c
index 20219a67a..736f279ef 100644
--- a/lib/logical-fields.c
+++ b/lib/logical-fields.c
@@ -243,7 +243,7 @@  ovn_init_symtab(struct shash *symtab)
     expr_symtab_add_field(symtab, "icmp4.code", MFF_ICMPV4_CODE, "icmp4",
               false);
 
-    expr_symtab_add_predicate(symtab, "igmp", "ip4 && ip.proto == 2");
+    expr_symtab_add_predicate(symtab, "igmp", "ip4.mcast && ip.proto == 2");
 
     expr_symtab_add_field(symtab, "ip6.src", MFF_IPV6_SRC, "ip6", false);
     expr_symtab_add_field(symtab, "ip6.dst", MFF_IPV6_DST, "ip6", false);
@@ -317,11 +317,12 @@  ovn_init_symtab(struct shash *symtab)
      * (RFC 2710 and RFC 3810).
      */
     expr_symtab_add_predicate(symtab, "mldv1",
-                              "ip6.src == fe80::/10 && "
+                              "eth.mcastv6 && ip6.src == fe80::/10 && "
                               "icmp6.type == {130, 131, 132}");
     /* MLDv2 packets are sent to ff02::16 (RFC 3810, 5.2.14) */
     expr_symtab_add_predicate(symtab, "mldv2",
-                              "ip6.dst == ff02::16 && icmp6.type == 143");
+                              "eth.mcastv6 && ip6.dst == ff02::16 && "
+                              "icmp6.type == 143");
 
     expr_symtab_add_predicate(symtab, "tcp", "ip.proto == 6");
     expr_symtab_add_field(symtab, "tcp.src", MFF_TCP_SRC, "tcp", false);
diff --git a/tests/ovn.at b/tests/ovn.at
index be274d1ec..274e03734 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5770,13 +5770,24 @@  for i in 1 2 3; do
 
     # Test ICMPv6 MLD reports (v1 and v2) and NS for DAD
     sip=00000000000000000000000000000000
-    test_icmpv6 ${i}3 f00000000${i}${i}3 f00000000021 $sip ff020000000000000000000000160000 83 21
-    test_icmpv6 ${i}3 f00000000${i}${i}3 f00000000021 $sip ff020000000000000000000000160000 8f 21
-    test_icmpv6 ${i}3 f00000000${i}${i}3 f00000000021 $sip ff0200000000000000ea2aeafffe2800 87 21
+    # Multicast traffic is delivered to all ports, except for the source.
+    out_ports=
+    for j in 1 2 3; do
+        for k in 1 2 3; do
+            if test "${j}${k}" -eq "${i}3"; then
+                continue
+            else
+                out_ports="$out_ports ${j}${k}"
+            fi
+        done
+    done
+    test_icmpv6 ${i}3 f00000000${i}${i}3 333300160000 $sip ff020000000000000000000000160000 83 ${out_ports}
+    test_icmpv6 ${i}3 f00000000${i}${i}3 333300160000 $sip ff020000000000000000000000160000 8f ${out_ports}
+    test_icmpv6 ${i}3 f00000000${i}${i}3 3333fffe2800 $sip ff0200000000000000ea2aeafffe2800 87 ${out_ports}
     # Traffic to non-multicast traffic should be dropped
     test_icmpv6 ${i}3 f00000000${i}${i}3 f00000000021 $sip $tip 83
     # Traffic of other ICMPv6 types should be dropped
-    test_icmpv6 ${i}3 f00000000${i}${i}3 f00000000021 $sip ff020000000000000000000000160000 80
+    test_icmpv6 ${i}3 f00000000${i}${i}3 333300160000 $sip ff020000000000000000000000160000 80
 
     # should be dropped
     sip=ae80000000000000ea2aeafffe2800aa
@@ -14174,8 +14185,8 @@  test_mldv2() {
     local inport=$1 outport=$2 src_mac=$3 src_ip=$4
 
     packet=$(fmt_pkt "
-        Ether(dst='ff:ff:ff:ff:ff:ff', src='${src_mac}') /
-        IPv6(src='${src_ip}', dst='ff02::2') /
+        Ether(dst='33:33:00:00:00:01', src='${src_mac}') /
+        IPv6(src='${src_ip}', dst='ff02::1') /
         ICMPv6MLQuery2()
     ")
     as hv1 ovs-appctl netdev-dummy/receive vif${inport} $packet