[ovs-dev] [patch v2] ovn: Fix gateway load balancing.
Darrell Ball
dlu998 at gmail.com
Thu Jun 28 19:34:16 UTC 2018
On Thu, Jun 28, 2018 at 11:59 AM, Darrell Ball <dlu998 at gmail.com> wrote:
>
>
> On Thu, Jun 28, 2018 at 11:26 AM, Darrell Ball <dlu998 at gmail.com> wrote:
>
>> 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.
>>
>>
>>
>> Pls double check that we are in same page first.
>>
>> You can run the ovn system tests without the patch and take a look at one
>> of the 5 failures, for example
>>
>> AT_SETUP([ovn -- multiple gateway routers, load-balancing])
>>
>> The load balance VIP is "vips:30.0.0.1"
>>
>> One the the router port's of interest has IP 20.0.0.2
>>
>>
> Once you confirm that there is no overlap b/w VIP and router port IP,
>
> I think one possibility is to only exclude "lb_force_snat_ip" router port
> IPs; I did not give it too much thought yet, however, but seems doable.
> There would be a cost to this finer grained filtering, of course.
>
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.
>
>
>
>>
>>
>>>
>>>
>>> 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);
>>>> + }
>>>> }
>>>> }
>>>>
>>>>
>>>
>>>
>>
>
More information about the dev
mailing list