[ovs-dev] [patch v2] ovn: Fix gateway load balancing.
Lorenzo Bianconi
lorenzo.bianconi at redhat.com
Fri Jun 29 10:31:22 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
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
>
>
> >
> > 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