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

Guru Shetty guru at ovn.org
Thu Jun 28 20:30:07 UTC 2018


>
>
>
> After a bit of chewing on the finer grained filtering question, I take back
> the above suggestion regarding  "lb_force_snat_ip", as it will not work
> in all cases. Implementing finer grained filtering would involve checking
> several pieces of information; I am not sure it is worth the added cost
> and complexity.
>

It looks to me that we need to skip the protocol port unreachable message
for the following 2 cases (unless I am missing some other case)

1. If any SNAT, DNAT or LB rules exist for the router in question where the
router IP is used for NATting.
2. If there is a dnat_force_snat_ip or lb_force_snat_ip set for the router.

But my personal opinion is that it need not be part of this patch as this
one fixes a regression and should come
as a patch that enhances ICMP port unreachability feature.


>
>
>
> >
> >
> >
> >>
> >>
> >>>
> >>>
> >>> 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