[ovs-dev] [PATCH v5 3/3] ovn: Gratuitous ARP for distributed NAT rules

[Mickey Spiegel]

On Wed, Mar 29, 2017 at 10:16 AM, Guru Shetty <guru at ovn.org> wrote:

>
>
> On 27 March 2017 at 18:34, Mickey Spiegel <mickeys.dev at gmail.com> wrote:
>
>> This patch extends gratuitous ARP support for NAT addresses so that it
>> applies to distributed NAT rules on a distributed logical router.
>> Distributed NAT rules have type "dnat_and_snat" and specify
>> 'external_mac' and 'logical_port'.
>>
>> Gratuitous ARP packets for distributed NAT rules are only generated on
>> the chassis where the 'logical_port' specified in the NAT rule resides.
>> Gratuitous ARPs are issued for the 'external_ip' address, resolving to
>> the 'external_mac'.
>>
>> Since the MAC address varies for each distributed NAT rule, a separate
>> 'nat_addresses' string must be generated for each distributed NAT rule.
>> For this reason, in the southbound 'Port_Binding',
>> 'options:nat-addresses' is replaced by a 'nat_addresses' column that
>> can have an unlimited number of instances.  In order to allow for
>> upgrades, pinctrl in the ovn-controller can work off either the
>> 'nat_addresses' column (if present), or 'options:nat-addresses'
>> otherwise.
>>
>> Signed-off-by: Mickey Spiegel <mickeys.dev at gmail.com>
>>
>
> I get a simple warning when I compile this:
> ovn/controller/pinctrl.c: In function ‘send_garp_update’:
> ovn/controller/pinctrl.c:1060:47: warning: suggest parentheses around
> assignment used as truth value [-Wparentheses]
>                                                binding_rec->logical_port))
> {
>

Added parentheses.


>
> If I run the test with valgrind enabled, the test fails. e.g:
> make check-valgrind TESTSUITEFLAGS="2312"
>
> ./ovn.at:6767: sort packets
> --- expout  2017-03-28 23:57:05.117974381 -0700
> +++ /root/git/openvswitch/tests/testsuite.dir/at-groups/2312/stdout
> 2017-03-28 23:57:05.117974381 -0700
> @@ -1,4 +1,4 @@
>  fffffffffffff0000000000108060001080006040001f00000000001c0a8
> 0001000000000000c0a80001
> +fffffffffffff0000000000108060001080006040001f00000000001c0a8
> 0001000000000000c0a80001
> +fffffffffffff0000000000108060001080006040001f00000000001c0a8
> 0002000000000000c0a80002
>  fffffffffffff0000000000108060001080006040001f00000000001c0a8
> 0002000000000000c0a80002
> -fffffffffffff0000000000308060001080006040001f00000000003c0a8
> 0003000000000000c0a80003
> -fffffffffffff0000000000408060001080006040001f00000000004c0a8
> 0004000000000000c0a80004
>

Race conditions. I think I have to clean up completely and start again to
make the timing more deterministic.

I hit a race condition with make check-valgrind on the previous patch. It
is running so slowly that it catches the first GARP packet and runs the
check before the second GARP packet arrives. This is fixed by simply
bumping up the wait condition from 50 bytes to 100 bytes.

Mickey