On Thu, Jun 28, 2018 at 11:55 AM, Han Zhou <[email protected]> wrote:
> Thanks Darrell! These tests weren't even listed in make check > TESTSUITEFLAGS="--list". What's the best way to run them? I'd like to run > them for each of my patch to make sure there is no regression. > sudo make check-kmod -C _gcc //kernel sudo make check-system-userspace -C _gcc //Userspace DP > > On Thu, Jun 28, 2018 at 11:46 AM, Darrell Ball <[email protected]> wrote: > >> >> >> 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
