On 28 June 2018 at 11:44, 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. >
We have a few test cases. The tests weren't run likely as they are system tests. > > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
