netdev - Re: [RFC v4 3/5] vxlan: add support for underlay in non-default VRF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D2E9C9D6-CCD7-45B3-AFE6-D6C28AB1D8A5@scaleway.com>
Date:   Tue, 27 Nov 2018 01:41:28 +0100
From:   Alexis Bauvin <abauvin@...leway.com>
To:     David Ahern <dsa@...ulusnetworks.com>, roopa@...ulusnetworks.com
Cc:     netdev@...r.kernel.org, akherbouche@...leway.com,
        Alexis Bauvin <abauvin@...leway.com>
Subject: Re: [RFC v4 3/5] vxlan: add support for underlay in non-default VRF

Le 26 nov. 2018 à 18:54, David Ahern <dsa@...ulusnetworks.com> a écrit :
> On 11/26/18 9:32 AM, Alexis Bauvin wrote:
>> Thanks for the review. I’ll send a v5 if you have no other comment on
>> this version!
> 
> A few comments on the test script; see attached which has the changes.
> 
> Mainly the cleanup does not need to be called at the end since you setup
> the exit trap. The cleanup calls ip to delete veth-hv-1 and veth-tap but
> those are moved to other namespaces.

This was on purpose to be sure to cleanup the interfaces in case the
script crashes for some reason and left interfaces outside of the
namespace.

> 'ip netns exec NAME ip ...' is more
> efficiently done as 'ip -netns NAME ...'. The test results should align
> like this:
> 
> Checking HV connectivity                                          [ OK ]
> Check VM connectivity through VXLAN (underlay in the default VRF) [ OK ]
> Check VM connectivity through VXLAN (underlay in a VRF)           [ OK ]
> 
> So it is easy for users to see the PASS/FAIL.

Awesome, thanks!

> It would be good to copy the topology ascii art into the test script as
> well for future users.

Will include this:

+-------------------+                                      +-------------------+
|                   |                                      |                   |
|    vm-1 netns     |                                      |    vm-2 netns     |
|                   |                                      |                   |
|  +-------------+  |                                      |  +-------------+  |
|  |   veth-hv   |  |                                      |  |   veth-hv   |  |
|  | 10.0.0.1/24 |  |                                      |  | 10.0.0.2/24 |  |
|  +-------------+  |                                      |  +-------------+  |
|         .         |                                      |         .         |
+-------------------+                                      +-------------------+
          .                                                          .
          .                                                          .
          .                                                          .
+------------------------------------+   +-------------------------------------+
|         .                          |   |                           .         |
|   +----------+                     |   |                     +----------+    |
|   | veth-tap |                     |   |                     | veth-tap |    |
|   +----+-----+                     |   |                     +----+-----+    |
|        |                           |   |                          |          |
|     +--+--+      +--------------+  |   |  +--------------+     +--+--+       |
|     | br0 |      | vrf-underlay |  |   |  | vrf-underlay |     | br0 |       |
|     +--+--+      +-------+------+  |   |  +------+-------+     +--+--+       |
|        |                 |         |   |         |                |          |
|    +---+----+    +-------+-------+ |   | +-------+-------+    +---+----+     |
|    | vxlan0 |....|     veth0     |.|...|.|     veth0     |....| vxlan0 |     |
|    +--------+    | 172.16.0.1/24 | |   | | 172.16.0.2/24 |    +--------+     |
|                  +---------------+ |   | +---------------+                   |
|                                    |   |                                     |
|              hv-1 netns            |   |           hv-2 netns                |
|                                    |   |                                     |
+------------------------------------+   +-------------------------------------+

> Also, add the test as a separate patch at the end and include it in
> tools/testing/selftests/net/Makefile

Regarding the discussion on patch 5, it should be better to send it first
after patch 3, and remove the down/up from it after current patch 5,
right?

> Finally, I think you should drop the RFC and send it as a 'ready for
> inclusion’.

Great thanks!

> <test_vxlan_under_vrf.sh>