[ovs-dev] [PATCH 06/23] ovn: Update TODO, ovn-northd flow table design, ovn-architecture for L3.

Justin Pettit jpettit at nicira.com
Fri Oct 16 21:46:55 UTC 2015


> On Oct 9, 2015, at 9:15 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> +*** Allow output to ingress port
> +
> +Sometimes when a packet ingresses into a router, it has to egress the
> +same port.  One example is a "one-armed" router that has multiple
> +routes on a single port (or in which a host is (mis)configured to send
> +every IP packet to the router, e.g. due to a bad netmask).  Another is
> +when a router needs to send an ICMP reply to a ingressing packet.

s/a/an/

> +        <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-210 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:
> +        </p>

I don't believe this is actually implemented in patch 23.  It might be nice to put a bolded "future" or something in the descriptions of things that aren't yet implemented.

> +        <p>
> +          UDP port unreachable.  These flows generate ICMP port unreachable
> +          messages in reply to UDP datagrams directed to the router's IP
> +          address.  The logical router doesn't accept any UDP traffic so it
> +          always generates such a reply.
> +        </p>
> ...
> +          TCP reset.  These flows generate TCP reset messages in reply to TCP
> +          datagrams directed to the router's IP address.  The logical router
> +          doesn't accept any TCP traffic so it always generates such a reply.
> +        </p>
> ...
> +        <p>
> +          Protocol unreachable.  These flows generate ICMP protocol unreachable
> +          messages in reply to packets directed to the router's IP address on
> +          IP protocols other than UDP, TCP, and ICMP.
> +        </p>

Did you want to specify a priority for these flows?  The ping and ARP processing have priority-210 and the drop everything else shows priority-200, so it may be worth throwing in there.

> +        Drop IP multicast.  A priority-190 flow with match
> +        <code>ip4.dst[28..31] == 0xe</code> drops IP multicast traffic.

Do you want to use "ip.mcast"?

> +          ICMP time exceeded.  For each router port <var>P</var>, whose IP
> +          address is <var>A</var>, a priority-180 flow with match <code>inport
> +          == <var>P</var> &amp;&amp; ip4.ttl == {0, 1} &amp;&amp;
> +          !ip.later_frag</code> matches packets whose TTL has expired, with the
> +          following actions to send an ICMP time exceeded reply:

The "ICMP time exceeded" and "TTL discard" flows match on an exceeded TTL differently.  It might be nice to use the same match in both. 

> +        TTL discard.  A priority-170 flow with match <code>ip4.ttl &lt;
> +        2</code> and actions <code>drop;</code> drops other packets whose TTL
> +        has expired, that should not receive a ICMP error reply.

It might be nice to explain that this handles fragments.

> +    <h3>Ingress Table 2: IP Routing</h3>

Is there anything in the Table 1 description that indicates the common case of moving to this table?

> +          Unknown MAC bindings.  For each non-gateway route to IPv4 network
> +          <var>N</var> with netmask <var>M</var> on router port <var>P</var>
> +          that owns IP address <var>A</var> and Ethernet address <var>E</var>,
> +          a logical flow with match <code>ip4.dst ==
> +          <var>N</var>/<var>M</var></code>, whose priority is the number of
> +          1-bits in <var>M</var>, has the following actions:
> ...
> +arp {
> +    eth.dst = ff:ff:ff:ff:ff:ff;
> +    eth.src = <var>E</var>;

I think you may want to set "eth.type".

> diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml
> index 47dfc2a..a7ff674 100644
> --- a/ovn/ovn-architecture.7.xml
> +++ b/ovn/ovn-architecture.7.xml
> @@ -596,7 +596,7 @@
>     </li>
>   </ol>
> 
> -  <h2>Life Cycle of a Packet</h2>
> +  <h2>Architectural Life Cycle of a Packet</h2>

There's the very similar sounding "Architectural Logical Life Cycle of a Packet" in ovn-sb, which I think could be confusing.  What do you think about throwing a "Physical" in there?

>       This following description focuses on the life cycle of a packet through
>       a logical datapath, ignoring physical details of the implementation.
> -      Please refer to <em>Life Cycle of a Packet</em> in
> +      Please refer to <em>Architectural Life Cycle of a Packet</em> in

If you change it, this would need to be updated, too, of course.

> +        <dt><code>ip.ttl--;</code></dt>
> +        <dd>
> +          <p>
> +            Decrements the IPv4 or IPv6 TTL.  If this would make the TTL zero
> +            or negative, then processing of the packet halts; no further
> +            actions are processed.  (To properly handle such cases, a
> +            higher-priority flow should match on <code>ip.ttl &lt; 2</code>.)
> +          </p>

This is put in the "to be implemented later"category of actions, but I believe "ip.ttl--" got implemented a few patches ago.

> +        <dt><code>icmp4 { <var>action</var>; </code>...<code> };</code></dt>
> ...
> +          <p>
> +            XXX need to explain exactly how the ICMP packet is constructed
> +          </p>

I don't understand what this means.

Acked-by: Justin Pettit <jpettit at nicira.com>

--Justin





More information about the dev mailing list