[ovs-dev] [patch v2] ovn: Fix gateway load balancing.
Han Zhou
zhouhan at gmail.com
Thu Jun 28 18:44:07 UTC 2018
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.
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>
>>
>>
>>
>
More information about the dev
mailing list