| Message ID | b07f581f46ef50befd5a22875ab9a12ff540c0e9.1632491529.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] nbctl: allow multiple bfd sessions with same nexthop and different outports | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
Hi all,
Tested this PR on an OCP OVN-K cluster and it works as expected. I created
two pods on different nodes having the same nexthop and OVN created two BFD
sessions one from each of the GW routers on those nodes towards the
nexthop/dst_ip.
=======
sh-4.4# ovn-nbctl lr-route-list GR_ci-ln-xwgizmt-f76d1-f8gv9-worker-b-vshc4
IPv4 Routes
10.128.8.12 10.0.128.4 src-ip
rtoe-GR_ci-ln-xwgizmt-f76d1-f8gv9-worker-b-vshc4 ecmp-symmetric-reply bfd
10.128.0.0/16 100.64.0.1 dst-ip
0.0.0.0/0 10.0.128.1 dst-ip
rtoe-GR_ci-ln-xwgizmt-f76d1-f8gv9-worker-b-vshc4
sh-4.4# ovn-nbctl lr-route-list GR_ci-ln-xwgizmt-f76d1-f8gv9-worker-c-rrq85
IPv4 Routes
10.128.10.13 10.0.128.4 src-ip
rtoe-GR_ci-ln-xwgizmt-f76d1-f8gv9-worker-c-rrq85 ecmp-symmetric-reply bfd
10.128.0.0/16 100.64.0.1 dst-ip
0.0.0.0/0 10.0.128.1 dst-ip
rtoe-GR_ci-ln-xwgizmt-f76d1-f8gv9-worker-c-rrq85
sh-4.4# ovn-nbctl list bfd
_uuid : a646da00-f201-4048-923b-82bac426c91a
detect_mult : []
dst_ip : "10.0.128.4"
external_ids : {}
logical_port : rtoe-GR_ci-ln-xwgizmt-f76d1-f8gv9-worker-b-vshc4
min_rx : []
min_tx : []
options : {}
status : down
_uuid : 00c14ced-6c0c-4b9e-8772-4eff9411bad6
detect_mult : []
dst_ip : "10.0.128.4"
external_ids : {}
logical_port : rtoe-GR_ci-ln-xwgizmt-f76d1-f8gv9-worker-c-rrq85
min_rx : []
min_tx : []
options : {}
status : down
Would be good if we could move this patch along.
Thanks,
Surya.
On Fri, Sep 24, 2021 at 3:54 PM Lorenzo Bianconi <
lorenzo.bianconi@redhat.com> wrote:
> Allow CMS to create multiple bfd sessions with the same nexthop but
> different outports:
>
> ovn-nbctl --bfd --policy=src-ip --ecmp-symmetric-reply lr-route-add
> GR_ovn-worker 10.244.0.5/32 172.18.0.4 rtoe-GR_ovn-worker
> ovn-nbctl --bfd --policy=src-ip --ecmp-symmetric-reply lr-route-add
> GR_ovn-worker2 10.244.2.5/32 172.18.0.4 rtoe-GR_ovn-worker2
>
> https://bugzilla.redhat.com/show_bug.cgi?id=2007549
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
> tests/ovn-northd.at | 12 ++++++++++--
> utilities/ovn-nbctl.c | 31 +++++++++++++++++++------------
> 2 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 5de554455..c5400d806 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -3011,7 +3011,7 @@ AT_KEYWORDS([northd-bfd])
> ovn_start
>
> check ovn-nbctl --wait=sb lr-add r0
> -for i in $(seq 1 5); do
> +for i in $(seq 1 7); do
> check ovn-nbctl --wait=sb lrp-add r0 r0-sw$i 00:00:00:00:00:0$i
> 192.168.$i.1/24
> check ovn-nbctl --wait=sb ls-add sw$i
> check ovn-nbctl --wait=sb lsp-add sw$i sw$i-r0
> @@ -3052,12 +3052,20 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8
> 192.168.5.2 r0-sw5
> wait_column down bfd status logical_port=r0-sw5
> AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q
> bfd],[0])
>
> +check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32
> 192.168.10.10 r0-sw6
> +wait_column down bfd status logical_port=r0-sw6
> +AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q
> bfd],[0])
> +
> +check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32
> 192.168.10.10 r0-sw7
> +wait_column down bfd status logical_port=r0-sw7
> +AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q
> bfd],[0])
> +
> route_uuid=$(fetch_column nb:logical_router_static_route _uuid ip_prefix="
> 100.0.0.0/8")
> check ovn-nbctl clear logical_router_static_route $route_uuid bfd
> wait_column admin_down bfd status logical_port=r0-sw1
>
> ovn-nbctl destroy bfd $uuid
> -wait_row_count bfd 3
> +wait_row_count bfd 5
>
> AT_CLEANUP
> ])
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index 10217dcd5..e34bb65f7 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -4121,6 +4121,15 @@ nbctl_lr_route_add(struct ctl_context *ctx)
> }
> }
>
> + if (ctx->argc == 5) {
> + /* validate output port. */
> + error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &out_lrp);
> + if (error) {
> + ctx->error = error;
> + goto cleanup;
> + }
> + }
> +
> struct shash_node *bfd = shash_find(&ctx->options, "--bfd");
> const struct nbrec_bfd *nb_bt = NULL;
> if (bfd) {
> @@ -4136,20 +4145,18 @@ nbctl_lr_route_add(struct ctl_context *ctx)
> } else {
> const struct nbrec_bfd *iter;
> NBREC_BFD_FOR_EACH (iter, ctx->idl) {
> - if (!strcmp(iter->dst_ip, next_hop)) {
> - nb_bt = iter;
> - break;
> + /* match endpoint ip. */
> + if (strcmp(iter->dst_ip, next_hop)) {
> + continue;
> + }
> + /* match outport. */
> + if (out_lrp && strcmp(iter->logical_port, out_lrp->name))
> {
> + continue;
> }
> - }
> - }
> - }
>
> - if (ctx->argc == 5) {
> - /* validate output port. */
> - error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &out_lrp);
> - if (error) {
> - ctx->error = error;
> - goto cleanup;
> + nb_bt = iter;
> + break;
> + }
> }
> }
>
> --
> 2.31.1
>
>
Thanks Lorenzo, Acked-by: Mark Michelson In case it's not clear from the commit message, this capability is added as a bug fix. So when this is merged, it needs to go into the main branch as well as the 21.09 branch, and possibly the 21.06 and 21.03 branches since they also have BFD support. On 9/24/21 9:54 AM, Lorenzo Bianconi wrote: > Allow CMS to create multiple bfd sessions with the same nexthop but > different outports: > > ovn-nbctl --bfd --policy=src-ip --ecmp-symmetric-reply lr-route-add GR_ovn-worker 10.244.0.5/32 172.18.0.4 rtoe-GR_ovn-worker > ovn-nbctl --bfd --policy=src-ip --ecmp-symmetric-reply lr-route-add GR_ovn-worker2 10.244.2.5/32 172.18.0.4 rtoe-GR_ovn-worker2 > > https://bugzilla.redhat.com/show_bug.cgi?id=2007549 > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > tests/ovn-northd.at | 12 ++++++++++-- > utilities/ovn-nbctl.c | 31 +++++++++++++++++++------------ > 2 files changed, 29 insertions(+), 14 deletions(-) > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 5de554455..c5400d806 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -3011,7 +3011,7 @@ AT_KEYWORDS([northd-bfd]) > ovn_start > > check ovn-nbctl --wait=sb lr-add r0 > -for i in $(seq 1 5); do > +for i in $(seq 1 7); do > check ovn-nbctl --wait=sb lrp-add r0 r0-sw$i 00:00:00:00:00:0$i 192.168.$i.1/24 > check ovn-nbctl --wait=sb ls-add sw$i > check ovn-nbctl --wait=sb lsp-add sw$i sw$i-r0 > @@ -3052,12 +3052,20 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5 > wait_column down bfd status logical_port=r0-sw5 > AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0]) > > +check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32 192.168.10.10 r0-sw6 > +wait_column down bfd status logical_port=r0-sw6 > +AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd],[0]) > + > +check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32 192.168.10.10 r0-sw7 > +wait_column down bfd status logical_port=r0-sw7 > +AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q bfd],[0]) > + > route_uuid=$(fetch_column nb:logical_router_static_route _uuid ip_prefix="100.0.0.0/8") > check ovn-nbctl clear logical_router_static_route $route_uuid bfd > wait_column admin_down bfd status logical_port=r0-sw1 > > ovn-nbctl destroy bfd $uuid > -wait_row_count bfd 3 > +wait_row_count bfd 5 > > AT_CLEANUP > ]) > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index 10217dcd5..e34bb65f7 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -4121,6 +4121,15 @@ nbctl_lr_route_add(struct ctl_context *ctx) > } > } > > + if (ctx->argc == 5) { > + /* validate output port. */ > + error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &out_lrp); > + if (error) { > + ctx->error = error; > + goto cleanup; > + } > + } > + > struct shash_node *bfd = shash_find(&ctx->options, "--bfd"); > const struct nbrec_bfd *nb_bt = NULL; > if (bfd) { > @@ -4136,20 +4145,18 @@ nbctl_lr_route_add(struct ctl_context *ctx) > } else { > const struct nbrec_bfd *iter; > NBREC_BFD_FOR_EACH (iter, ctx->idl) { > - if (!strcmp(iter->dst_ip, next_hop)) { > - nb_bt = iter; > - break; > + /* match endpoint ip. */ > + if (strcmp(iter->dst_ip, next_hop)) { > + continue; > + } > + /* match outport. */ > + if (out_lrp && strcmp(iter->logical_port, out_lrp->name)) { > + continue; > } > - } > - } > - } > > - if (ctx->argc == 5) { > - /* validate output port. */ > - error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &out_lrp); > - if (error) { > - ctx->error = error; > - goto cleanup; > + nb_bt = iter; > + break; > + } > } > } > >
I merged this change to the main branch and branch-21.09. On 10/1/21 9:03 AM, Mark Michelson wrote: > Thanks Lorenzo, > > Acked-by: Mark Michelson > > In case it's not clear from the commit message, this capability is added > as a bug fix. So when this is merged, it needs to go into the main > branch as well as the 21.09 branch, and possibly the 21.06 and 21.03 > branches since they also have BFD support. > > On 9/24/21 9:54 AM, Lorenzo Bianconi wrote: >> Allow CMS to create multiple bfd sessions with the same nexthop but >> different outports: >> >> ovn-nbctl --bfd --policy=src-ip --ecmp-symmetric-reply lr-route-add >> GR_ovn-worker 10.244.0.5/32 172.18.0.4 rtoe-GR_ovn-worker >> ovn-nbctl --bfd --policy=src-ip --ecmp-symmetric-reply lr-route-add >> GR_ovn-worker2 10.244.2.5/32 172.18.0.4 rtoe-GR_ovn-worker2 >> >> https://bugzilla.redhat.com/show_bug.cgi?id=2007549 >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> >> --- >> tests/ovn-northd.at | 12 ++++++++++-- >> utilities/ovn-nbctl.c | 31 +++++++++++++++++++------------ >> 2 files changed, 29 insertions(+), 14 deletions(-) >> >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index 5de554455..c5400d806 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -3011,7 +3011,7 @@ AT_KEYWORDS([northd-bfd]) >> ovn_start >> check ovn-nbctl --wait=sb lr-add r0 >> -for i in $(seq 1 5); do >> +for i in $(seq 1 7); do >> check ovn-nbctl --wait=sb lrp-add r0 r0-sw$i 00:00:00:00:00:0$i >> 192.168.$i.1/24 >> check ovn-nbctl --wait=sb ls-add sw$i >> check ovn-nbctl --wait=sb lsp-add sw$i sw$i-r0 >> @@ -3052,12 +3052,20 @@ check ovn-nbctl --bfd lr-route-add r0 >> 240.0.0.0/8 192.168.5.2 r0-sw5 >> wait_column down bfd status logical_port=r0-sw5 >> AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q >> bfd],[0]) >> +check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32 >> 192.168.10.10 r0-sw6 >> +wait_column down bfd status logical_port=r0-sw6 >> +AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q >> bfd],[0]) >> + >> +check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32 >> 192.168.10.10 r0-sw7 >> +wait_column down bfd status logical_port=r0-sw7 >> +AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q >> bfd],[0]) >> + >> route_uuid=$(fetch_column nb:logical_router_static_route _uuid >> ip_prefix="100.0.0.0/8") >> check ovn-nbctl clear logical_router_static_route $route_uuid bfd >> wait_column admin_down bfd status logical_port=r0-sw1 >> ovn-nbctl destroy bfd $uuid >> -wait_row_count bfd 3 >> +wait_row_count bfd 5 >> AT_CLEANUP >> ]) >> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c >> index 10217dcd5..e34bb65f7 100644 >> --- a/utilities/ovn-nbctl.c >> +++ b/utilities/ovn-nbctl.c >> @@ -4121,6 +4121,15 @@ nbctl_lr_route_add(struct ctl_context *ctx) >> } >> } >> + if (ctx->argc == 5) { >> + /* validate output port. */ >> + error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &out_lrp); >> + if (error) { >> + ctx->error = error; >> + goto cleanup; >> + } >> + } >> + >> struct shash_node *bfd = shash_find(&ctx->options, "--bfd"); >> const struct nbrec_bfd *nb_bt = NULL; >> if (bfd) { >> @@ -4136,20 +4145,18 @@ nbctl_lr_route_add(struct ctl_context *ctx) >> } else { >> const struct nbrec_bfd *iter; >> NBREC_BFD_FOR_EACH (iter, ctx->idl) { >> - if (!strcmp(iter->dst_ip, next_hop)) { >> - nb_bt = iter; >> - break; >> + /* match endpoint ip. */ >> + if (strcmp(iter->dst_ip, next_hop)) { >> + continue; >> + } >> + /* match outport. */ >> + if (out_lrp && strcmp(iter->logical_port, >> out_lrp->name)) { >> + continue; >> } >> - } >> - } >> - } >> - if (ctx->argc == 5) { >> - /* validate output port. */ >> - error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &out_lrp); >> - if (error) { >> - ctx->error = error; >> - goto cleanup; >> + nb_bt = iter; >> + break; >> + } >> } >> } >> >
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 5de554455..c5400d806 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -3011,7 +3011,7 @@ AT_KEYWORDS([northd-bfd]) ovn_start check ovn-nbctl --wait=sb lr-add r0 -for i in $(seq 1 5); do +for i in $(seq 1 7); do check ovn-nbctl --wait=sb lrp-add r0 r0-sw$i 00:00:00:00:00:0$i 192.168.$i.1/24 check ovn-nbctl --wait=sb ls-add sw$i check ovn-nbctl --wait=sb lsp-add sw$i sw$i-r0 @@ -3052,12 +3052,20 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5 wait_column down bfd status logical_port=r0-sw5 AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0]) +check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.6.1/32 192.168.10.10 r0-sw6 +wait_column down bfd status logical_port=r0-sw6 +AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd],[0]) + +check ovn-nbctl --bfd --policy=src-ip lr-route-add r0 192.168.7.1/32 192.168.10.10 r0-sw7 +wait_column down bfd status logical_port=r0-sw7 +AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.7.1 | grep -q bfd],[0]) + route_uuid=$(fetch_column nb:logical_router_static_route _uuid ip_prefix="100.0.0.0/8") check ovn-nbctl clear logical_router_static_route $route_uuid bfd wait_column admin_down bfd status logical_port=r0-sw1 ovn-nbctl destroy bfd $uuid -wait_row_count bfd 3 +wait_row_count bfd 5 AT_CLEANUP ]) diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 10217dcd5..e34bb65f7 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -4121,6 +4121,15 @@ nbctl_lr_route_add(struct ctl_context *ctx) } } + if (ctx->argc == 5) { + /* validate output port. */ + error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &out_lrp); + if (error) { + ctx->error = error; + goto cleanup; + } + } + struct shash_node *bfd = shash_find(&ctx->options, "--bfd"); const struct nbrec_bfd *nb_bt = NULL; if (bfd) { @@ -4136,20 +4145,18 @@ nbctl_lr_route_add(struct ctl_context *ctx) } else { const struct nbrec_bfd *iter; NBREC_BFD_FOR_EACH (iter, ctx->idl) { - if (!strcmp(iter->dst_ip, next_hop)) { - nb_bt = iter; - break; + /* match endpoint ip. */ + if (strcmp(iter->dst_ip, next_hop)) { + continue; + } + /* match outport. */ + if (out_lrp && strcmp(iter->logical_port, out_lrp->name)) { + continue; } - } - } - } - if (ctx->argc == 5) { - /* validate output port. */ - error = lrp_by_name_or_uuid(ctx, ctx->argv[4], true, &out_lrp); - if (error) { - ctx->error = error; - goto cleanup; + nb_bt = iter; + break; + } } }
Allow CMS to create multiple bfd sessions with the same nexthop but different outports: ovn-nbctl --bfd --policy=src-ip --ecmp-symmetric-reply lr-route-add GR_ovn-worker 10.244.0.5/32 172.18.0.4 rtoe-GR_ovn-worker ovn-nbctl --bfd --policy=src-ip --ecmp-symmetric-reply lr-route-add GR_ovn-worker2 10.244.2.5/32 172.18.0.4 rtoe-GR_ovn-worker2 https://bugzilla.redhat.com/show_bug.cgi?id=2007549 Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- tests/ovn-northd.at | 12 ++++++++++-- utilities/ovn-nbctl.c | 31 +++++++++++++++++++------------ 2 files changed, 29 insertions(+), 14 deletions(-)