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.
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
