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

Darrell Ball dlu998 at gmail.com
Thu Jun 28 18:57:23 UTC 2018


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