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

Darrell Ball dlu998 at gmail.com
Fri Jun 29 14:37:34 UTC 2018


On Fri, Jun 29, 2018 at 3:31 AM, Lorenzo Bianconi <
lorenzo.bianconi at redhat.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
>
> Hi Darrell,
>
> thx for the info, I will always use it in subsequent patches.
> I implemented what is indicated in ovn-northd doc and just run
> ovn tests (not system ones). Thx for fixing it
>
> Regards,
> Lorenzo
>

No worries Lorenzo.
This is a pretty unexpected overlap in functionality.

Darrell



>
> >
> >
> > >
> > > 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>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> > >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list