On Fri, Jun 29, 2018 at 3:31 AM, Lorenzo Bianconi < [email protected]> wrote:
> > 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 > > Hi Darrell, > > thx for the info, I will always use it in subsequent patches. > I implemented what is indicated in ovn-northd doc and just run > ovn tests (not system ones). Thx for fixing it > > Regards, > Lorenzo > No worries Lorenzo. This is a pretty unexpected overlap in functionality. Darrell > > > > > > > > > > > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
