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
