Re: [ovs-dev] [patch_v5 2/3] ovn: Add additional comments regarding arp responders.

2016-11-02 Thread Darrell Ball
On Sun, Oct 23, 2016 at 11:19 AM, Mickey Spiegel 
wrote:

> Acked-by: Mickey Spiegel 
>
> A few very minor nits below.
>
> On Fri, Oct 21, 2016 at 1:36 PM, Darrell Ball  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 
>> Signed-off-by: Ramu Ramamurthy 
>> Co-authored-by: Ramu Ramamurthy 
>> Acked-by: Han Zhou 
>> ---
>>
>> 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 @@
>>  Ingress Table 10: ARP/ND responder
>>
>>  
>> -  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.
>> +
>> +
>> +
>> +  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."



>
>
>> +
>> +
>> +
>> +  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.

Re: [ovs-dev] [patch_v5 2/3] ovn: Add additional comments regarding arp responders.

2016-10-23 Thread Mickey Spiegel
Acked-by: Mickey Spiegel 

A few very minor nits below.

On Fri, Oct 21, 2016 at 1:36 PM, Darrell Ball  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 
> Signed-off-by: Ramu Ramamurthy 
> Co-authored-by: Ramu Ramamurthy 
> Acked-by: Han Zhou 
> ---
>
> 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 @@
>  Ingress Table 10: ARP/ND responder
>
>  
> -  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.
> +
> +
> +
> +  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"?


> +  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".


> +  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 ...


> +
> +
> +
> +  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.
> +
> +
> +
> +  Note that ARP requests received from localnet or
> +  vtep 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".


> +  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:
>  
>
>  
>
> -Priority-100 flows to skip ARP responder if inport is of type
> -localnet, and advances directly to the next table.
> +