On Thu, Jun 28, 2018 at 11:44 AM, Han Zhou <[email protected]> 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. > Here are tests: system-ovn 113: ovn -- 2 LRs connected via LS, gateway router, SNAT and DNAT ok 114: ovn -- 2 LRs connected via LS, gateway router, easy SNAT FAILED ( system-ovn.at:262) 115: ovn -- multiple gateway routers, SNAT and DNAT FAILED ( system-ovn.at:432) 116: ovn -- load-balancing ok 117: ovn -- load-balancing - same subnet. ok 118: ovn -- load balancing in gateway router ok 119: ovn -- multiple gateway routers, load-balancing FAILED ( system-ovn.at:1051) 120: ovn -- load balancing in router with gateway router port ok 121: ovn -- DNAT and SNAT on distributed router - N/S FAILED ( system-ovn.at:1344) 122: ovn -- DNAT and SNAT on distributed router - E/W FAILED ( system-ovn.at:1517) > > On Thu, Jun 28, 2018 at 11:29 AM, Mark Michelson <[email protected]> > 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 <[email protected] >>> <mailto:[email protected]>> 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 <[email protected] >>> <mailto:[email protected]>> >>> Signed-off-by: Darrell Ball <[email protected] >>> <mailto:[email protected]>> >>> >>> --- >>> 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 >>> [email protected] <mailto:[email protected]> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >>> >>> >>> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
