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

Le 26 nov. 2018 à 18:54, David Ahern [off-list ref] a écrit :

On 11/26/18 9:32 AM, Alexis Bauvin wrote:

quoted

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>