[ovs-dev] [PATCH v3] ovn-northd: no logical router icmp response for directed broadcasts

[Justin Pettit]

Thanks, Flavio.  I pushed this patch.  I made a couple of small tweaks:

	- I updated the Reported-by link to the beginning of the thread that mentions the different options.
	- I removed a paragraph from the ovn-northd man page that talked about handling pings to 255.255.255.255.

--Justin


> On Jun 20, 2016, at 1:57 PM, Flavio Fernandes <flavio at flaviof.com> wrote:
> 
> 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/021619.html
> Signed-off-by: Flavio Fernandes <flavio at flaviof.com>
> ---
> Changes v1->v2:
>  - Rebase.
>  - Use Reported-at label for proper referencing.
> Changes v2->v3:
>  - Rebase.
>  - Update documentation in ovn-northd affected by this change.
> 
> ovn/northd/ovn-northd.8.xml | 17 +++++++++--------
> ovn/northd/ovn-northd.c     |  5 ++---
> 2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 0e59125..65f64c7 100644
> --- 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>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index d53fca9..752e032 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1957,9 +1957,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"; "
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev