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

Han Zhou zhouhan at gmail.com
Thu Jun 28 18:16:50 UTC 2018


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> 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>
>> Signed-off-by: Darrell Ball <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
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list