| Message ID | 543e5a3a2d6595fe1f2487f34c2867b704b6a929.1618260354.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] ovn-nbctl: dump next-hop for router policies | expand |
Acked-by: Mark Michelson <mmichels@redhat.com> See below for one tiny nit that doesn't affect the ACK-edness of the patch. On 4/12/21 5:12 PM, Lorenzo Bianconi wrote: > Commit 35b00c7e79 ('northd: Add ECMP support to router policies.') > introduced 'nexthops' column in the Logical_Router_Policy table to > support ECMP for router policies however print_routing_policy() still > dumps legacy 'nexthop' column as policy next hop. > Fix the issue dumping 'nexthops' columns. > > Fixes: 35b00c7e79 ('northd: Add ECMP support to router policies.') > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > tests/ovn-northd.at | 9 ++++++++- > utilities/ovn-nbctl.c | 14 +++++++++----- > 2 files changed, 17 insertions(+), 6 deletions(-) > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 96476497d..4a12b7fd6 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2485,6 +2485,13 @@ check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public > > check ovn-nbctl --wait=sb lr-policy-add lr0 10 "ip4.src == 10.0.0.3" reroute 172.168.0.101,172.168.0.102 > > +ovn-nbctl lr-policy-list lr0 > policy-list > +AT_CAPTURE_FILE([policy-list]) > +AT_CHECK([cat policy-list], [0], [dnl > +Routing Policies > + 10 ip4.src == 10.0.0.3 reroute 172.168.0.101, 172.168.0.102 > +]) > + > ovn-sbctl dump-flows lr0 > lr0flows3 > AT_CAPTURE_FILE([lr0flows3]) > > @@ -2950,4 +2957,4 @@ wait_row_count FDB 0 > ovn-sbctl list FDB > > AT_CLEANUP > -]) > \ No newline at end of file > +]) This should probably be undone. > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index 184058356..042c21002 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -3865,11 +3865,15 @@ static void > print_routing_policy(const struct nbrec_logical_router_policy *policy, > struct ds *s) > { > - if (policy->nexthop != NULL) { > - char *next_hop = normalize_prefix_str(policy->nexthop); > - ds_put_format(s, "%10"PRId64" %50s %15s %25s", policy->priority, > - policy->match, policy->action, next_hop); > - free(next_hop); > + if (policy->n_nexthops) { > + ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority, > + policy->match, policy->action); > + for (int i = 0; i < policy->n_nexthops; i++) { > + char *next_hop = normalize_prefix_str(policy->nexthops[i]); > + char *fmt = i ? ", %s" : " %25s"; > + ds_put_format(s, fmt, next_hop); > + free(next_hop); > + } > } else { > ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority, > policy->match, policy->action); >
> Acked-by: Mark Michelson <mmichels@redhat.com> > > See below for one tiny nit that doesn't affect the ACK-edness of the patch. ack, I will fix it in v2. Regards, Lorenzo > > On 4/12/21 5:12 PM, Lorenzo Bianconi wrote: > > Commit 35b00c7e79 ('northd: Add ECMP support to router policies.') > > introduced 'nexthops' column in the Logical_Router_Policy table to > > support ECMP for router policies however print_routing_policy() still > > dumps legacy 'nexthop' column as policy next hop. > > Fix the issue dumping 'nexthops' columns. > > > > Fixes: 35b00c7e79 ('northd: Add ECMP support to router policies.') > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > --- > > tests/ovn-northd.at | 9 ++++++++- > > utilities/ovn-nbctl.c | 14 +++++++++----- > > 2 files changed, 17 insertions(+), 6 deletions(-) > > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 96476497d..4a12b7fd6 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -2485,6 +2485,13 @@ check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public > > check ovn-nbctl --wait=sb lr-policy-add lr0 10 "ip4.src == 10.0.0.3" reroute 172.168.0.101,172.168.0.102 > > +ovn-nbctl lr-policy-list lr0 > policy-list > > +AT_CAPTURE_FILE([policy-list]) > > +AT_CHECK([cat policy-list], [0], [dnl > > +Routing Policies > > + 10 ip4.src == 10.0.0.3 reroute 172.168.0.101, 172.168.0.102 > > +]) > > + > > ovn-sbctl dump-flows lr0 > lr0flows3 > > AT_CAPTURE_FILE([lr0flows3]) > > @@ -2950,4 +2957,4 @@ wait_row_count FDB 0 > > ovn-sbctl list FDB > > AT_CLEANUP > > -]) > > \ No newline at end of file > > +]) > > This should probably be undone. > > > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > > index 184058356..042c21002 100644 > > --- a/utilities/ovn-nbctl.c > > +++ b/utilities/ovn-nbctl.c > > @@ -3865,11 +3865,15 @@ static void > > print_routing_policy(const struct nbrec_logical_router_policy *policy, > > struct ds *s) > > { > > - if (policy->nexthop != NULL) { > > - char *next_hop = normalize_prefix_str(policy->nexthop); > > - ds_put_format(s, "%10"PRId64" %50s %15s %25s", policy->priority, > > - policy->match, policy->action, next_hop); > > - free(next_hop); > > + if (policy->n_nexthops) { > > + ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority, > > + policy->match, policy->action); > > + for (int i = 0; i < policy->n_nexthops; i++) { > > + char *next_hop = normalize_prefix_str(policy->nexthops[i]); > > + char *fmt = i ? ", %s" : " %25s"; > > + ds_put_format(s, fmt, next_hop); > > + free(next_hop); > > + } > > } else { > > ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority, > > policy->match, policy->action); > > >
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 96476497d..4a12b7fd6 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2485,6 +2485,13 @@ check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public check ovn-nbctl --wait=sb lr-policy-add lr0 10 "ip4.src == 10.0.0.3" reroute 172.168.0.101,172.168.0.102 +ovn-nbctl lr-policy-list lr0 > policy-list +AT_CAPTURE_FILE([policy-list]) +AT_CHECK([cat policy-list], [0], [dnl +Routing Policies + 10 ip4.src == 10.0.0.3 reroute 172.168.0.101, 172.168.0.102 +]) + ovn-sbctl dump-flows lr0 > lr0flows3 AT_CAPTURE_FILE([lr0flows3]) @@ -2950,4 +2957,4 @@ wait_row_count FDB 0 ovn-sbctl list FDB AT_CLEANUP -]) \ No newline at end of file +]) diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 184058356..042c21002 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -3865,11 +3865,15 @@ static void print_routing_policy(const struct nbrec_logical_router_policy *policy, struct ds *s) { - if (policy->nexthop != NULL) { - char *next_hop = normalize_prefix_str(policy->nexthop); - ds_put_format(s, "%10"PRId64" %50s %15s %25s", policy->priority, - policy->match, policy->action, next_hop); - free(next_hop); + if (policy->n_nexthops) { + ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority, + policy->match, policy->action); + for (int i = 0; i < policy->n_nexthops; i++) { + char *next_hop = normalize_prefix_str(policy->nexthops[i]); + char *fmt = i ? ", %s" : " %25s"; + ds_put_format(s, fmt, next_hop); + free(next_hop); + } } else { ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority, policy->match, policy->action);
Commit 35b00c7e79 ('northd: Add ECMP support to router policies.') introduced 'nexthops' column in the Logical_Router_Policy table to support ECMP for router policies however print_routing_policy() still dumps legacy 'nexthop' column as policy next hop. Fix the issue dumping 'nexthops' columns. Fixes: 35b00c7e79 ('northd: Add ECMP support to router policies.') Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- tests/ovn-northd.at | 9 ++++++++- utilities/ovn-nbctl.c | 14 +++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-)