> On Oct 9, 2015, at 9:15 PM, Ben Pfaff <[email protected]> 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 && 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> && ip4.ttl == {0, 1} &&
> + !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 <
> + 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 < 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 <[email protected]>
--Justin
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
