[ovs-dev] [patch v2] ovn: Fix gateway load balancing.

Han Zhou zhouhan at gmail.com
Thu Jun 28 19:03:24 UTC 2018


Great. It works:
sudo make check-system-userspace TESTSUITEFLAGS="-k ovn"

Thanks again!

On Thu, Jun 28, 2018 at 11:57 AM, Darrell Ball <dlu998 at gmail.com> wrote:

>
>
> On Thu, Jun 28, 2018 at 11:55 AM, Han Zhou <zhouhan at gmail.com> wrote:
>
>> Thanks Darrell! These tests weren't even listed in make check
>> TESTSUITEFLAGS="--list". What's the best way to run them? I'd like to run
>> them for each of my patch to make sure there is no regression.
>>
>
>
>  sudo make check-kmod -C _gcc  //kernel
>  sudo make check-system-userspace -C _gcc  //Userspace DP
>
>
>>
>> On Thu, Jun 28, 2018 at 11:46 AM, Darrell Ball <dlu998 at gmail.com> wrote:
>>
>>>
>>>
>>> On Thu, Jun 28, 2018 at 11:44 AM, Han Zhou <zhouhan at gmail.com> wrote:
>>>
>>>> Hi Mark, what I meant is the test for the feature of LB in Gateway. If
>>>> we had a test case, the problem would have been noticed when Lorenzo is
>>>> working on ICMP feature.
>>>>
>>>
>>> Here are tests:
>>>
>>> system-ovn
>>>
>>> 113: ovn -- 2 LRs connected via LS, gateway router, SNAT and DNAT ok
>>> 114: ovn -- 2 LRs connected via LS, gateway router, easy SNAT FAILED (
>>> system-ovn.at:262)
>>> 115: ovn -- multiple gateway routers, SNAT and DNAT  FAILED (
>>> system-ovn.at:432)
>>> 116: ovn -- load-balancing                           ok
>>> 117: ovn -- load-balancing - same subnet.            ok
>>> 118: ovn -- load balancing in gateway router         ok
>>> 119: ovn -- multiple gateway routers, load-balancing FAILED (
>>> system-ovn.at:1051)
>>> 120: ovn -- load balancing in router with gateway router port ok
>>> 121: ovn -- DNAT and SNAT on distributed router - N/S FAILED (
>>> system-ovn.at:1344)
>>> 122: ovn -- DNAT and SNAT on distributed router - E/W FAILED (
>>> system-ovn.at:1517)
>>>
>>>
>>>
>>>
>>>>
>>>> On Thu, Jun 28, 2018 at 11:29 AM, Mark Michelson <mmichels at redhat.com>
>>>> wrote:
>>>>
>>>>> Lorenzo actually brought up in today's OVN IRC meeting that writing
>>>>> ICMP tests for IPv6 is problematic right now. IPv4 tests have an M4 macro
>>>>> called OVN_POPULATE_ARP that they call. This manually populates tables in
>>>>> OVS so that there are no ARP requests being sent during the test. It does
>>>>> this by calling `ovs-appct tnl/neigh/set` for each IP address-MAC pair.
>>>>>
>>>>> IPv6 does not have an equivalent. The result is that tests will
>>>>> sometimes succeed, but sometimes will fail because IPv6 ND requests will be
>>>>> sent during the test. I haven't looked into this myself, but Lorenzo
>>>>> mentioned that there would need to be changes made to OVS itself in order
>>>>> to have an IPv6 version of a working OVN_POPULATE_ARP.
>>>>>
>>>>> On 06/28/2018 02:16 PM, Han Zhou wrote:
>>>>>
>>>>>> Agree with Mark that ICMP is still needed for non VIP on gateway. But
>>>>>> LB feature on GW is also important.
>>>>>> Shall we have a test case to cover this particular scenario?
>>>>>>
>>>>>> On Thu, Jun 28, 2018 at 9:41 AM, Mark Michelson <mmichels at redhat.com
>>>>>> <mailto:mmichels at redhat.com>> wrote:
>>>>>>
>>>>>>     Hi Darrell,
>>>>>>
>>>>>>     I'm not 100% sure this is correct. I think this change would only
>>>>>> be
>>>>>>     correct if the router's IP addresses are also load balancer VIPs.
>>>>>> In
>>>>>>     the case where the VIPs do not overlap with the router IP
>>>>>> addresses,
>>>>>>     I think we'd want to have the ICMP responses still in place.
>>>>>>
>>>>>>
>>>>>>     On 06/28/2018 01:15 AM, Darrell Ball wrote:
>>>>>>
>>>>>>         Non-distributed and distributed gateway load balancing is
>>>>>> broken.
>>>>>>         Recent changes for port unreachable handling broke the
>>>>>> associated
>>>>>>         unsnat functionality.  The fix approach is check for gateway
>>>>>>         contexts and accept packets directed to gateway router IPs.
>>>>>>
>>>>>>         Fixes:  86558ac2e476 ("OVN: add UDP port unreachable support
>>>>>> to
>>>>>>         OVN logical router.")
>>>>>>         Fixes:  159932c9e4ea ("OVN: add TCP port unreachable support
>>>>>> to
>>>>>>         OVN logical router.")
>>>>>>         Fixes:  0e858e05f76b ("OVN: add protocol unreachable support
>>>>>> to
>>>>>>         OVN router ports.")
>>>>>>         CC: Lorenzo Bianconi <lorenzo.bianconi at redhat.com
>>>>>>         <mailto:lorenzo.bianconi at redhat.com>>
>>>>>>         Signed-off-by: Darrell Ball <dlu998 at gmail.com
>>>>>>         <mailto:dlu998 at gmail.com>>
>>>>>>
>>>>>>         ---
>>>>>>            ovn/northd/ovn-northd.8.xml |  17 ++++---
>>>>>>            ovn/northd/ovn-northd.c     | 106
>>>>>>         ++++++++++++++++++++++----------------------
>>>>>>            2 files changed, 64 insertions(+), 59 deletions(-)
>>>>>>
>>>>>>         diff --git a/ovn/northd/ovn-northd.8.xml
>>>>>>         b/ovn/northd/ovn-northd.8.xml
>>>>>>         index cfd3511..280efd0 100644
>>>>>>         --- a/ovn/northd/ovn-northd.8.xml
>>>>>>         +++ b/ovn/northd/ovn-northd.8.xml
>>>>>>         @@ -1310,8 +1310,9 @@ nd_na {
>>>>>>                    <p>
>>>>>>                      UDP port unreachable.  Priority-80 flows generate
>>>>>>         ICMP port
>>>>>>                      unreachable messages in reply to UDP datagrams
>>>>>>         directed to the
>>>>>>         -          router's IP address.  The logical router doesn't
>>>>>>         accept any UDP
>>>>>>         -          traffic so it always generates such a reply.
>>>>>>         +          router's IP address, except in the special case of
>>>>>>         gateways,
>>>>>>         +          which accept traffic directed to a router IP for
>>>>>> load
>>>>>>         balancing
>>>>>>         +          purposes.
>>>>>>                    </p>
>>>>>>                      <p>
>>>>>>         @@ -1321,10 +1322,10 @@ nd_na {
>>>>>>                    <li>
>>>>>>                    <p>
>>>>>>         -          TCP reset.  Priority-80 flows generate TCP reset
>>>>>>         messages in reply to
>>>>>>         -          TCP datagrams directed to the router's IP
>>>>>> address.         The logical
>>>>>>         -          router doesn't accept any TCP traffic so it always
>>>>>>         generates such a
>>>>>>         -          reply.
>>>>>>         +          TCP reset.  Priority-80 flows generate TCP reset
>>>>>>         messages in reply
>>>>>>         +          to TCP datagrams directed to the router's IP
>>>>>> address,
>>>>>>         except in
>>>>>>         +          the special case of gateways, which accept traffic
>>>>>>         directed to a
>>>>>>         +          router IP for load balancing purposes.
>>>>>>                    </p>
>>>>>>                      <p>
>>>>>>         @@ -1336,7 +1337,9 @@ nd_na {
>>>>>>                    <p>
>>>>>>                      Protocol unreachable.  Priority-70 flows generate
>>>>>>         ICMP protocol
>>>>>>                      unreachable messages in reply to packets directed
>>>>>>         to the router's IP
>>>>>>         -          address on IP protocols other than UDP, TCP, and
>>>>>> ICMP.
>>>>>>         +          address on IP protocols other than UDP, TCP, and
>>>>>>         ICMP, except in the
>>>>>>         +          special case of gateways, which accept traffic
>>>>>>         directed to a router
>>>>>>         +          IP for load balancing purposes.
>>>>>>                    </p>
>>>>>>                      <p>
>>>>>>         diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
>>>>>>         index 72fe4e7..7648bce 100644
>>>>>>         --- a/ovn/northd/ovn-northd.c
>>>>>>         +++ b/ovn/northd/ovn-northd.c
>>>>>>         @@ -5141,48 +5141,49 @@ build_lrouter_flows(struct hmap
>>>>>>         *datapaths, struct hmap *ports,
>>>>>>                                      ds_cstr(&match),
>>>>>> ds_cstr(&actions));
>>>>>>                    }
>>>>>>            -        /* UDP/TCP port unreachable */
>>>>>>         -        for (int i = 0; i < op->lrp_networks.n_ipv4_addrs;
>>>>>> i++) {
>>>>>>         -            const char *action;
>>>>>>         -
>>>>>>         -            ds_clear(&match);
>>>>>>         -            ds_put_format(&match,
>>>>>>         -                          "ip4 && ip4.dst == %s &&
>>>>>>         !ip.later_frag && udp",
>>>>>>         -                          op->lrp_networks.ipv4_addrs[i]
>>>>>> .addr_s);
>>>>>>         -            action = "icmp4 {"
>>>>>>         -                        "eth.dst <-> eth.src; "
>>>>>>         -                        "ip4.dst <-> ip4.src; "
>>>>>>         -                        "ip.ttl = 255; "
>>>>>>         -                        "icmp4.type = 3; "
>>>>>>         -                        "icmp4.code = 3; "
>>>>>>         -                        "next; };";
>>>>>>         -            ovn_lflow_add(lflows, op->od,
>>>>>> S_ROUTER_IN_IP_INPUT, 80,
>>>>>>         -                          ds_cstr(&match), action);
>>>>>>         +        if (!smap_get(&op->od->nbr->options, "chassis")
>>>>>>         +            && !op->od->l3dgw_port) {
>>>>>>         +            /* UDP/TCP port unreachable. */
>>>>>>         +            for (int i = 0; i <
>>>>>> op->lrp_networks.n_ipv4_addrs;
>>>>>>         i++) {
>>>>>>         +                ds_clear(&match);
>>>>>>         +                ds_put_format(&match,
>>>>>>         +                              "ip4 && ip4.dst == %s &&
>>>>>>         !ip.later_frag && udp",
>>>>>>         +
>>>>>>  op->lrp_networks.ipv4_addrs[i].addr_s);
>>>>>>         +                const char *action = "icmp4 {"
>>>>>>         +                                     "eth.dst <-> eth.src; "
>>>>>>         +                                     "ip4.dst <-> ip4.src; "
>>>>>>         +                                     "ip.ttl = 255; "
>>>>>>         +                                     "icmp4.type = 3; "
>>>>>>         +                                     "icmp4.code = 3; "
>>>>>>         +                                     "next; };";
>>>>>>         +                ovn_lflow_add(lflows, op->od,
>>>>>>         S_ROUTER_IN_IP_INPUT, 80,
>>>>>>         +                              ds_cstr(&match), action);
>>>>>>            -            ds_clear(&match);
>>>>>>         -            ds_put_format(&match,
>>>>>>         -                          "ip4 && ip4.dst == %s &&
>>>>>>         !ip.later_frag && tcp",
>>>>>>         -                          op->lrp_networks.ipv4_addrs[i]
>>>>>> .addr_s);
>>>>>>         -            action = "tcp_reset {"
>>>>>>         -                        "eth.dst <-> eth.src; "
>>>>>>         -                        "ip4.dst <-> ip4.src; "
>>>>>>         -                        "next; };";
>>>>>>         -            ovn_lflow_add(lflows, op->od,
>>>>>> S_ROUTER_IN_IP_INPUT, 80,
>>>>>>         -                          ds_cstr(&match), action);
>>>>>>         +                ds_clear(&match);
>>>>>>         +                ds_put_format(&match,
>>>>>>         +                              "ip4 && ip4.dst == %s &&
>>>>>>         !ip.later_frag && tcp",
>>>>>>         +
>>>>>>  op->lrp_networks.ipv4_addrs[i].addr_s);
>>>>>>         +                action = "tcp_reset {"
>>>>>>         +                         "eth.dst <-> eth.src; "
>>>>>>         +                         "ip4.dst <-> ip4.src; "
>>>>>>         +                         "next; };";
>>>>>>         +                ovn_lflow_add(lflows, op->od,
>>>>>>         S_ROUTER_IN_IP_INPUT, 80,
>>>>>>         +                              ds_cstr(&match), action);
>>>>>>            -            ds_clear(&match);
>>>>>>         -            ds_put_format(&match,
>>>>>>         -                          "ip4 && ip4.dst == %s &&
>>>>>> !ip.later_frag",
>>>>>>         -                          op->lrp_networks.ipv4_addrs[i]
>>>>>> .addr_s);
>>>>>>         -            action = "icmp4 {"
>>>>>>         -                        "eth.dst <-> eth.src; "
>>>>>>         -                        "ip4.dst <-> ip4.src; "
>>>>>>         -                        "ip.ttl = 255; "
>>>>>>         -                        "icmp4.type = 3; "
>>>>>>         -                        "icmp4.code = 2; "
>>>>>>         -                        "next; };";
>>>>>>         -            ovn_lflow_add(lflows, op->od,
>>>>>> S_ROUTER_IN_IP_INPUT, 70,
>>>>>>         -                          ds_cstr(&match), action);
>>>>>>         +                ds_clear(&match);
>>>>>>         +                ds_put_format(&match,
>>>>>>         +                              "ip4 && ip4.dst == %s &&
>>>>>>         !ip.later_frag",
>>>>>>         +
>>>>>>  op->lrp_networks.ipv4_addrs[i].addr_s);
>>>>>>         +                action = "icmp4 {"
>>>>>>         +                         "eth.dst <-> eth.src; "
>>>>>>         +                         "ip4.dst <-> ip4.src; "
>>>>>>         +                         "ip.ttl = 255; "
>>>>>>         +                         "icmp4.type = 3; "
>>>>>>         +                         "icmp4.code = 2; "
>>>>>>         +                         "next; };";
>>>>>>         +                ovn_lflow_add(lflows, op->od,
>>>>>>         S_ROUTER_IN_IP_INPUT, 70,
>>>>>>         +                              ds_cstr(&match), action);
>>>>>>         +            }
>>>>>>                    }
>>>>>>                      ds_clear(&match);
>>>>>>         @@ -5306,19 +5307,20 @@ build_lrouter_flows(struct hmap
>>>>>>         *datapaths, struct hmap *ports,
>>>>>>                    }
>>>>>>                      /* TCP port unreachable */
>>>>>>         -        for (int i = 0; i < op->lrp_networks.n_ipv6_addrs;
>>>>>> i++) {
>>>>>>         -            const char *action;
>>>>>>         -
>>>>>>         -            ds_clear(&match);
>>>>>>         -            ds_put_format(&match,
>>>>>>         -                          "ip6 && ip6.dst == %s &&
>>>>>>         !ip.later_frag && tcp",
>>>>>>         -                          op->lrp_networks.ipv6_addrs[i]
>>>>>> .addr_s);
>>>>>>         -            action = "tcp_reset {"
>>>>>>         -                        "eth.dst <-> eth.src; "
>>>>>>         -                        "ip6.dst <-> ip6.src; "
>>>>>>         -                        "next; };";
>>>>>>         -            ovn_lflow_add(lflows, op->od,
>>>>>> S_ROUTER_IN_IP_INPUT, 80,
>>>>>>         +        if (!smap_get(&op->od->nbr->options, "chassis")
>>>>>>         +            && !op->od->l3dgw_port) {
>>>>>>         +            for (int i = 0; i <
>>>>>> op->lrp_networks.n_ipv6_addrs;
>>>>>>         i++) {
>>>>>>         +                ds_clear(&match);
>>>>>>         +                ds_put_format(&match,
>>>>>>         +                              "ip6 && ip6.dst == %s &&
>>>>>>         !ip.later_frag && tcp",
>>>>>>         +
>>>>>>  op->lrp_networks.ipv6_addrs[i].addr_s);
>>>>>>         +                const char *action = "tcp_reset {"
>>>>>>         +                                     "eth.dst <-> eth.src; "
>>>>>>         +                                     "ip6.dst <-> ip6.src; "
>>>>>>         +                                     "next; };";
>>>>>>         +                ovn_lflow_add(lflows, op->od,
>>>>>>         S_ROUTER_IN_IP_INPUT, 80,
>>>>>>                                      ds_cstr(&match), action);
>>>>>>         +            }
>>>>>>                    }
>>>>>>                }
>>>>>>
>>>>>>
>>>>>>     _______________________________________________
>>>>>>     dev mailing list
>>>>>>     dev at openvswitch.org <mailto:dev at openvswitch.org>
>>>>>>     https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>>>>     <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>


More information about the dev mailing list