ovn-northd: no logical router icmp response for directed broadcasts

Responding to icmp queries where the L3 destination is a directed broadcast
was not being properly handled, causing the reply to be sent to all logical
ports except for the one port that should receive it.

This is a proposal for using choice B in the mail discussion; where icmp
queries to broadcast are simply not responded by the logical router.

Reported-at: http://openvswitch.org/pipermail/discuss/2016-June/021610.html
Signed-off-by: Flavio Fernandes <flavio@flaviof.com>
Signed-off-by: Justin Pettit <jpettit@ovn.org>

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 6d52f7e..260cc14 100644 (file)
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -489,14 +489,15 @@ output;
       <li>
         <p>
           ICMP echo reply.  These flows reply to ICMP echo requests received
-          for the router's IP address.  Let <var>A</var> be an IP address or
-          broadcast address owned by a router port.  Then, for each
-          <var>A</var>, a priority-90 flow matches on <code>ip4.dst ==
-          <var>A</var></code> and <code>icmp4.type == 8 &amp;&amp; icmp4.code
-          == 0</code> (ICMP echo request).  These flows use the following
-          actions where, if <var>A</var> is unicast, then <var>S</var> is
-          <var>A</var>, and if <var>A</var> is broadcast, <var>S</var> is the
-          router's IP address in <var>A</var>'s network:
+          for the router's IP address.  Let <var>A</var> be an IP address
+          owned by a router port.  Then, for each <var>A</var>, a priority-90
+          flow matches on <code>ip4.dst == <var>A</var></code> and <code>
+          icmp4.type == 8 &amp;&amp; icmp4.code == 0</code> (ICMP echo
+          request).  The port of the router that receives the echo request
+          does not matter. Also, the ip.ttl of the echo request packet is not
+          checked, so it complies with RFC 1812, section 4.2.2.9. These flows
+          use the following actions where <var>S</var> is the router's IP
+          address:
         </p>
 
         <pre>
@@ -507,12 +508,6 @@ icmp4.type = 0;
 inport = ""; /* Allow sending out inport. */
 next;
         </pre>
-
-        <p>
-          Similar flows match on <code>ip4.dst == 255.255.255.255</code> and
-          each individual <code>inport</code>, and use the same actions in
-          which <var>S</var> is a function of <code>inport</code>.
-        </p>
       </li>
 
       <li>

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 1599e18..c2cf15e 100644 (file)
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1960,9 +1960,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
          * (i.e. the incoming locally attached net) does not matter.
          * The ip.ttl also does not matter (RFC1812 section 4.2.2.9) */
         match = xasprintf(
-            "(ip4.dst == "IP_FMT" || ip4.dst == "IP_FMT") && "
-            "icmp4.type == 8 && icmp4.code == 0",
-            IP_ARGS(op->ip), IP_ARGS(op->bcast));
+            "ip4.dst == "IP_FMT" && icmp4.type == 8 && icmp4.code == 0",
+            IP_ARGS(op->ip));
         char *actions = xasprintf(
             "ip4.dst = ip4.src; "
             "ip4.src = "IP_FMT"; "