Re: [ovs-dev] [PATCH v3 3/3] ovn: Send GARP for router port IPs of a router port connected to bridged logical switch
On Tue, Jul 9, 2019 at 5:46 PM Numan Siddique wrote: > > > On Tue, Jul 9, 2019 at 4:37 PM Ilya Maximets > wrote: > >> On 01.07.2019 10:43, nusid...@redhat.com wrote: >> > From: Numan Siddique >> > >> > This patch handles sending GARPs for >> > >> > - router port IPs of a distributed router port >> > >> > - router port IPs of a router port which belongs to gateway router >> >(with the option - redirect-chassis set in Logical_Router.options) >> > >> > Signed-off-by: Numan Siddique >> > Acked-by: Dumitru Ceara >> > --- >> > ovn/northd/ovn-northd.c | 44 >> > tests/ovn.at| 89 +++-- >> > 2 files changed, 105 insertions(+), 28 deletions(-) >> > >> >> Hi. >> This patch triggers frequent TravisCI failures: >> https://travis-ci.org/openvswitch/ovs/jobs/556015141 >> >> checking packets in ext1/vif1-tx.pcap against ext1-vif1.expected: >> ovn.at:12: waiting until $PYTHON "$top_srcdir/utilities/ovs-pcap.in" >> $rcv_pcap > $rcv_text >> rcv_n=`wc -l < "$rcv_text"` >> echo "rcv_n=$rcv_n exp_n=$exp_n" >> test $rcv_n -ge $exp_n... >> rcv_n=1 exp_n=2 >> rcv_n=1 exp_n=2 >> rcv_n=1 exp_n=2 >> rcv_n=2 exp_n=2 >> ovn.at:12: wait succeeded after 2 seconds >> ../../tests/ovn.at:8593: sort $rcv_text >> --- expout 2019-07-05 19:09:16.471288908 + >> +++ >> /home/travis/build/openvswitch/ovs/openvswitch-2.11.90/_build/tests/testsuite.dir/at-groups/2662/stdout >>2019-07-05 19:09:16.475288910 + >> @@ -1,2 +1,2 @@ >> >> -f0010204020102030800451c3f110100c0a80102ac10010300350008 >> >> +020102030806000108000604000102010203ac100101ac100101 >> >> >> 020102030806000108000604000102010203ac100101ac100101 >> 2662. ovn.at:8422: 2662. ovn -- 4 HV, 1 LS, 1 LR, packet test with HA >> distributed router gateway port (ovn.at:8422): FAILED (ovn.at:8593) >> >> Could you, please, take a look? >> >> Some other tests are affected too, but re-check usually succeeds for them: >> 2664: ovn -- 4 HV, 3 LS, 2 LR, packet test with HA distributed router >> gateway port >> 2691: ovn -- router - check packet length - icmp defrag >> It'll be good to fix them too. >> >> > Hi Ilya, > > I will take a look at it. > Dumitru also mentioned about the same errors. I thought those are timing > related. > Let me take a look. > > I have submitted the patch to fix these test failures - https://patchwork.ozlabs.org/patch/1130867/ Thanks Numan > Thanks > Numan > > >> Best regards, Ilya Maximets. >> > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 3/3] ovn: Send GARP for router port IPs of a router port connected to bridged logical switch
On Tue, Jul 9, 2019 at 4:37 PM Ilya Maximets wrote: > On 01.07.2019 10:43, nusid...@redhat.com wrote: > > From: Numan Siddique > > > > This patch handles sending GARPs for > > > > - router port IPs of a distributed router port > > > > - router port IPs of a router port which belongs to gateway router > >(with the option - redirect-chassis set in Logical_Router.options) > > > > Signed-off-by: Numan Siddique > > Acked-by: Dumitru Ceara > > --- > > ovn/northd/ovn-northd.c | 44 > > tests/ovn.at| 89 +++-- > > 2 files changed, 105 insertions(+), 28 deletions(-) > > > > Hi. > This patch triggers frequent TravisCI failures: > https://travis-ci.org/openvswitch/ovs/jobs/556015141 > > checking packets in ext1/vif1-tx.pcap against ext1-vif1.expected: > ovn.at:12: waiting until $PYTHON "$top_srcdir/utilities/ovs-pcap.in" > $rcv_pcap > $rcv_text > rcv_n=`wc -l < "$rcv_text"` > echo "rcv_n=$rcv_n exp_n=$exp_n" > test $rcv_n -ge $exp_n... > rcv_n=1 exp_n=2 > rcv_n=1 exp_n=2 > rcv_n=1 exp_n=2 > rcv_n=2 exp_n=2 > ovn.at:12: wait succeeded after 2 seconds > ../../tests/ovn.at:8593: sort $rcv_text > --- expout 2019-07-05 19:09:16.471288908 + > +++ > /home/travis/build/openvswitch/ovs/openvswitch-2.11.90/_build/tests/testsuite.dir/at-groups/2662/stdout >2019-07-05 19:09:16.475288910 + > @@ -1,2 +1,2 @@ > > -f0010204020102030800451c3f110100c0a80102ac10010300350008 > > +020102030806000108000604000102010203ac100101ac100101 > > > 020102030806000108000604000102010203ac100101ac100101 > 2662. ovn.at:8422: 2662. ovn -- 4 HV, 1 LS, 1 LR, packet test with HA > distributed router gateway port (ovn.at:8422): FAILED (ovn.at:8593) > > Could you, please, take a look? > > Some other tests are affected too, but re-check usually succeeds for them: > 2664: ovn -- 4 HV, 3 LS, 2 LR, packet test with HA distributed router > gateway port > 2691: ovn -- router - check packet length - icmp defrag > It'll be good to fix them too. > > Hi Ilya, I will take a look at it. Dumitru also mentioned about the same errors. I thought those are timing related. Let me take a look. Thanks Numan > Best regards, Ilya Maximets. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 3/3] ovn: Send GARP for router port IPs of a router port connected to bridged logical switch
On 01.07.2019 10:43, nusid...@redhat.com wrote: > From: Numan Siddique > > This patch handles sending GARPs for > > - router port IPs of a distributed router port > > - router port IPs of a router port which belongs to gateway router >(with the option - redirect-chassis set in Logical_Router.options) > > Signed-off-by: Numan Siddique > Acked-by: Dumitru Ceara > --- > ovn/northd/ovn-northd.c | 44 > tests/ovn.at| 89 +++-- > 2 files changed, 105 insertions(+), 28 deletions(-) > Hi. This patch triggers frequent TravisCI failures: https://travis-ci.org/openvswitch/ovs/jobs/556015141 checking packets in ext1/vif1-tx.pcap against ext1-vif1.expected: ovn.at:12: waiting until $PYTHON "$top_srcdir/utilities/ovs-pcap.in" $rcv_pcap > $rcv_text rcv_n=`wc -l < "$rcv_text"` echo "rcv_n=$rcv_n exp_n=$exp_n" test $rcv_n -ge $exp_n... rcv_n=1 exp_n=2 rcv_n=1 exp_n=2 rcv_n=1 exp_n=2 rcv_n=2 exp_n=2 ovn.at:12: wait succeeded after 2 seconds ../../tests/ovn.at:8593: sort $rcv_text --- expout 2019-07-05 19:09:16.471288908 + +++ /home/travis/build/openvswitch/ovs/openvswitch-2.11.90/_build/tests/testsuite.dir/at-groups/2662/stdout 2019-07-05 19:09:16.475288910 + @@ -1,2 +1,2 @@ -f0010204020102030800451c3f110100c0a80102ac10010300350008 +020102030806000108000604000102010203ac100101ac100101 020102030806000108000604000102010203ac100101ac100101 2662. ovn.at:8422: 2662. ovn -- 4 HV, 1 LS, 1 LR, packet test with HA distributed router gateway port (ovn.at:8422): FAILED (ovn.at:8593) Could you, please, take a look? Some other tests are affected too, but re-check usually succeeds for them: 2664: ovn -- 4 HV, 3 LS, 2 LR, packet test with HA distributed router gateway port 2691: ovn -- router - check packet length - icmp defrag It'll be good to fix them too. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 3/3] ovn: Send GARP for router port IPs of a router port connected to bridged logical switch
On Mon, Jul 1, 2019 at 9:45 AM wrote: > > From: Numan Siddique > > This patch handles sending GARPs for > > - router port IPs of a distributed router port > > - router port IPs of a router port which belongs to gateway router >(with the option - redirect-chassis set in Logical_Router.options) > > Signed-off-by: Numan Siddique I tested the patches on my setup and the code looks good to me. A bit unrelated to this change, at least for me the ovn_port_update_sbrec is getting quite hard to follow. Maybe we can consider refactoring it a bit in the future so different functionalities are handled by smaller functions (i.e., router-port, switch-port connected to vif, switch-port connected to router). Acked-by: Dumitru Ceara > --- > ovn/northd/ovn-northd.c | 44 > tests/ovn.at| 89 +++-- > 2 files changed, 105 insertions(+), 28 deletions(-) > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index e0af234f8..e8cbc3534 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -1983,9 +1983,23 @@ get_nat_addresses(const struct ovn_port *op, size_t *n) > } > } else { > /* Centralized NAT rule, either on gateway router or distributed > - * router. */ > -ds_put_format(_addresses, " %s", nat->external_ip); > -central_ip_address = true; > + * router. > + * Check if external_ip is same as router ip. If so, then there > + * is no need to add this to the nat_addresses. The router IPs > + * will be added separately. */ > +bool is_router_ip = false; > +for (size_t j = 0; j < op->lrp_networks.n_ipv4_addrs; j++) { > +if (!strcmp(nat->external_ip, > +op->lrp_networks.ipv4_addrs[j].addr_s)) { > +is_router_ip = true; > +break; > +} > +} > + > +if (!is_router_ip) { > +ds_put_format(_addresses, " %s", nat->external_ip); > +central_ip_address = true; > +} > } > } > > @@ -2531,13 +2545,26 @@ ovn_port_update_sbrec(struct northd_context *ctx, > * - op->peer has 'reside-on-gateway-chassis' set and the > *the logical router datapath has distributed router port. > * > + * - op->peer is distributed gateway router port. > + * > + * - op->peer's router is a gateway router and op has a localnet > + *port. > + * > * Note: Port_Binding.nat_addresses column is also used for > * sending the GARPs for the router port IPs. > * */ > +bool add_router_port_garp = false; > if (op->peer && op->peer->nbrp && op->peer->od->l3dgw_port && > op->peer->od->l3redirect_port && > -smap_get_bool(>peer->nbrp->options, > - "reside-on-redirect-chassis", false)) { > +(smap_get_bool(>peer->nbrp->options, > + "reside-on-redirect-chassis", false) || > +op->peer == op->peer->od->l3dgw_port)) { > +add_router_port_garp = true; > +} else if (chassis && op->od->localnet_port) { > +add_router_port_garp = true; > +} > + > +if (add_router_port_garp) { > struct ds garp_info = DS_EMPTY_INITIALIZER; > ds_put_format(_info, "%s", op->peer->lrp_networks.ea_s); > for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs; > @@ -2545,8 +2572,11 @@ ovn_port_update_sbrec(struct northd_context *ctx, > ds_put_format(_info, " %s", > > op->peer->lrp_networks.ipv4_addrs[i].addr_s); > } > -ds_put_format(_info, " is_chassis_resident(%s)", > - op->peer->od->l3redirect_port->json_key); > + > +if (op->peer->od->l3redirect_port) { > +ds_put_format(_info, " is_chassis_resident(%s)", > + op->peer->od->l3redirect_port->json_key); > +} > > sbrec_port_binding_update_nat_addresses_addvalue( > op->sb, ds_cstr(_info)); > diff --git a/tests/ovn.at b/tests/ovn.at > index ea627e128..2e266d94a 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -6730,6 +6730,9 @@ AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown]) > AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet]) > AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1]) > > +# Wait until the patch ports are created in hv1 to connect br-int to br-eth0 > +OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-vsctl show | \ > +grep "Port patch-br-int-to-ln_port" | wc