[ovs-dev] [patch v2] ovn: Fix gateway load balancing.
Guru Shetty
guru at ovn.org
Thu Jun 28 18:45:02 UTC 2018
On 28 June 2018 at 11:44, 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.
>
We have a few test cases. The tests weren't run likely as they are system
tests.
>
> 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