[ovs-dev] [PATCH v3 3/3] ovn: Send GARP for router port IPs of a router port connected to bridged logical switch
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 --- 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(&c_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(&c_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(&op->peer->nbrp->options, - "reside-on-redirect-chassis", false)) { +(smap_get_bool(&op->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(&garp_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(&garp_info, " %s", op->peer->lrp_networks.ipv4_addrs[i].addr_s); } -ds_put_format(&garp_info, " is_chassis_resident(%s)", - op->peer->od->l3redirect_port->json_key); + +if (op->peer->od->l3redirect_port) { +ds_put_format(&garp_info, " is_chassis_resident(%s)", + op->peer->od->l3redirect_port->json_key); +} sbrec_port_binding_update_nat_addresses_addvalue( op->sb, ds_cstr(&garp_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 -l`]) # Wait for packet to be received. OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 50]) @@ -6737,10 +6740,11 @@ trim_zeros() { sed 's/\(00\)\{1,\}$//' } $PYTHON "$top_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap | trim_zeros > packets -expected="f00108060001080006040001f001c0a80002c0a80002" +expected="f00108060001080006040001f001c0a80001c0a80001" echo $expected > expout +expected="f00108060001080006040001f001c0a80002c0a80002" +echo $expected >>
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(&c_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(&c_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(&op->peer->nbrp->options, > - "reside-on-redirect-chassis", false)) { > +(smap_get_bool(&op->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(&garp_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(&garp_info, " %s", > > op->peer->lrp_networks.ipv4_addrs[i].addr_s); > } > -ds_put_format(&garp_info, " is_chassis_resident(%s)", > - op->peer->od->l3redirect_port->json_key); > + > +if (op->peer->od->l3redirect_port) { > +ds_put_format(&garp_info, " is_chassis_resident(%s)", > + op->peer->od->l3redirect_port->json_key); > +} > > sbrec_port_binding_update_nat_addresses_addvalue( > op->sb, ds_cstr(&garp_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 "Por
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 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 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