On Sun, Oct 23, 2016 at 11:19 AM, Mickey Spiegel <mickeys....@gmail.com> wrote:
> Acked-by: Mickey Spiegel <mickeys....@gmail.com> > > A few very minor nits below. > > On Fri, Oct 21, 2016 at 1:36 PM, Darrell Ball <dlu...@gmail.com> wrote: > >> There has been enough confusion regarding logical switch datapath >> arp responders in ovn to warrant some additional comments; >> hence add a general description regarding why they exist and >> document the special cases. >> >> Signed-off-by: Darrell Ball <dlu...@gmail.com> >> Signed-off-by: Ramu Ramamurthy <ramu.ramamur...@us.ibm.com> >> Co-authored-by: Ramu Ramamurthy <ramu.ramamur...@us.ibm.com> >> Acked-by: Han Zhou <zhou...@gmail.com> >> --- >> >> v4->v5: Splice in some rewording from review from multiple sources. >> >> v3->v4: Capitalization fixes. >> Reinstate comment regarding L2 learning confusion. >> >> v2->v3: Reword and further elaborate. >> >> v1->v2: Dropped RFC code change for logical switch router >> type ports. >> >> ovn/northd/ovn-northd.8.xml | 67 ++++++++++++++++++++++++++++++ >> +++++++++++---- >> 1 file changed, 61 insertions(+), 6 deletions(-) >> >> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml >> index df53d4c..930ebf4 100644 >> --- a/ovn/northd/ovn-northd.8.xml >> +++ b/ovn/northd/ovn-northd.8.xml >> @@ -435,20 +435,75 @@ >> <h3>Ingress Table 10: ARP/ND responder</h3> >> >> <p> >> - This table implements ARP/ND responder for known IPs. It contains >> these >> - logical flows: >> + This table implements ARP/ND responder in a logical switch for >> known >> + IPs. The advantage of the ARP responder flow is to limit ARP >> + broadcasts by locally responding to ARP requests without the need >> to >> + send to other hypervisors. One common case is when the inport is a >> + logical port associated with a VIF and the broadcast is responded >> to >> + on the local hypervisor rather than broadcast across the whole >> + network and responded to by the destination VM. This behavior is >> + proxy ARP. >> + </p> >> + >> + <p> >> + ARP requests arrive from VMs from a logical switch inport of type >> + default. For this case, the logical switch proxy ARP rules can be >> + for other VMs or logical router ports. Logical switch proxy ARP >> + rules may be programmed both for mac binding of IP addresses on >> + other logical switch VIF ports (which are of the default logical >> + switch port type, representing connectivity to VMs or containers), >> + and for mac binding of IP addresses on logical switch router type >> + ports, representing their logical router port peers. In order to >> + support proxy ARP for logical router ports, an IP address must be >> + configured on the logical switch router type port, with the same >> + value as the peer of the logical router port. The configured MAC >> > > Instead of "peer of the logical router port" (did you mean the logical > router port or the logical switch router type port?), perhaps just > "peer logical router port"? > "peer logical router port" is the intended meaning - good catch. > > >> + addresses must match as well. When a VM sends an ARP request for a >> + distributed logical router port and if the peer router type port >> of >> > > There is an extra space in "peer router". > thanks > > >> + the attached logical switch does not have an IP address configured, >> + the ARP request will be broadcast on the logical switch. One of >> the >> + copies of the ARP request will go through the logical switch router >> + type port to the logical router datapath, where the logical router >> ARP >> + responder will generate a reply. The mac binding in a VM for an >> + associated distributed logical router will be used for all >> + communication needing routing, hence the action of a VM re-arping >> for >> + the mac binding of the logical router port should be rare. >> > > There is a context switch going into the last sentence that can be a bit > confusing. How about: > > After the VM learns a MAC binding for an associated distributed logical > router, that MAC binding will be used for all communication needing > routing, hence ... > MAC binding is the subject of the sentence and I prefer the active voice. However, I partially folded in your suggestion: "The MAC binding of a distributed logical router, once learned by an associated VM, is used for all that VM's communication needing routing. Hence, the action of a VM re-arping for the mac binding of the logical router port should be rare." > > >> + </p> >> + >> + <p> >> + Logical switch ARP responder proxy ARP rules can also be hit when >> + receiving ARP requests externally on a L2 gateway port. In this >> case, >> + the hypervisor acting as an L2 gateway, responds to the ARP >> request on >> + behalf of a destination VM. >> + </p> >> + >> + <p> >> + Note that ARP requests received from <code>localnet</code> or >> + <code>vtep</code> logical inports can either go directly to VMs, in >> + which case the VM responds or can hit an ARP responder for a >> logical >> > > I prefer a comma between "responds" and "or". > Since the clause after the coordinating conjunction has a dependency, I believe the rule is that there is never a comma before the coordinating conjunction. > > >> + router port if the packet is used to resolve a logical router port >> + next hop address. In either case, logical switch ARP responder >> rules >> + will not be hit. It contains these logical flows: >> </p> >> >> <ul> >> <li> >> - Priority-100 flows to skip ARP responder if inport is of type >> - <code>localnet</code>, and advances directly to the next table. >> + Priority-100 flows to skip the ARP responder if inport is of type >> + <code>localnet</code> or <code>vtep</code> and advances directly >> + to the next table. ARP requests sent to <code>localnet</code> or >> + <code>vtep</code> ports can be received by multiple hypervisors. >> + Now, because the same mac binding rules are downloaded to all >> + hypervisors, each of the multiple hypervisors will respond. This >> + will confuse L2 learning on the source of the ARP requests. ARP >> + requests received on an inport of type <code>router</code> are >> not >> + expected to hit any logical switch ARP responder flows. However, >> + no skip flows are installed for these packets, as there would be >> + some additional flow cost for this and the value appears limited. >> > > Change "will" to "would" in both places above, since this is part of the > hypothetical rationale and not an action that will actually be taken. > The premise is: "+ Now, because the same mac binding rules are downloaded to all + hypervisors," and if the premise is true or likely to be true, then "will" is correct not "would" as below "each of the multiple hypervisors will respond." Since the statement below is cascading from a likely true premise, "will" is also preferred. "+ This will confuse L2 learning on the source of the ARP requests. + since the premise is shared" > > >> </li> >> >> <li> >> <p> >> Priority-50 flows that match ARP requests to each known IP >> address >> - <var>A</var> of every logical router port, and respond with ARP >> + <var>A</var> of every logical switch port, and respond with ARP >> replies directly with corresponding Ethernet address >> <var>E</var>: >> </p> >> >> @@ -475,7 +530,7 @@ output; >> <p> >> Priority-50 flows that match IPv6 ND neighbor solicitations to >> each known IP address <var>A</var> (and <var>A</var>'s >> - solicited node address) of every logical router port, and >> + solicited node address) of every logical switch port, and >> respond with neighbor advertisements directly with >> corresponding Ethernet address <var>E</var>: >> </p> >> -- >> 1.9.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev >> > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
