Message ID | 20220309093851.4070-1-dceara@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev] northd: Properly warn for NAT on LR with multiple gw ports. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Wed, Mar 9, 2022 at 4:39 AM Dumitru Ceara <dceara@redhat.com> wrote: > > This is not supported and ovn-northd was not logging it properly. > The previous attempt tried to log in init_nat_entries() but there > the logical router port configuration has not yet been parsed. > > Move the check and log instead to where we install the NAT flows. > Also add a test. > > Fixes: 15348b7b806f ("ovn-northd: Multiple distributed gateway port support.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> Thanks. Applied to the main branch. Numan > --- > northd/northd.c | 22 ++++++++++++---------- > tests/ovn-northd.at | 6 ++++++ > 2 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index b264fb850b..7f309098e1 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -763,16 +763,6 @@ init_nat_entries(struct ovn_datapath *od) > return; > } > > - if (od->n_l3dgw_ports > 1) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "NAT is configured on logical router %s, which has %" > - PRIuSIZE" distributed gateway ports. NAT is not supported" > - " yet when there is more than one distributed gateway " > - "port on the router.", > - od->nbr->name, od->n_l3dgw_ports); > - return; > - } > - > od->nat_entries = xmalloc(od->nbr->n_nat * sizeof *od->nat_entries); > > for (size_t i = 0; i < od->nbr->n_nat; i++) { > @@ -13243,6 +13233,18 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, > return; > } > > + /* NAT rules are not currently supported on logical routers with multiple > + * distributed gateway ports. */ > + if (od->n_l3dgw_ports > 1) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "NAT is configured on logical router %s, which has %" > + PRIuSIZE" distributed gateway ports. NAT is not supported" > + " yet when there is more than one distributed gateway " > + "port on the router.", > + od->nbr->name, od->n_l3dgw_ports); > + return; > + } > + > struct sset nat_entries = SSET_INITIALIZER(&nat_entries); > > bool dnat_force_snat_ip = > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 60d91a7717..17d4f31b39 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -5804,6 +5804,12 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep cr-DR | sed 's/table=../table=?? > table=??(lr_in_gw_redirect ), priority=50 , match=(outport == "DR-S3"), action=(outport = "cr-DR-S3"; next;) > ]) > > +# Check that ovn-northd logs a warning when trying to configure NAT > +# on the router with multiple distributed gw ports. Such configurations are > +# not supported yet. > +check ovn-nbctl lr-nat-add DR dnat_and_snat 42.42.42.1 20.0.0.2 > +AT_CHECK([grep -q 'NAT is configured on logical router DR, which has 2 distributed gateway ports. NAT is not supported yet when there is more than one distributed gateway port on the router.' northd/ovn-northd.log], [0]) > + > AT_CLEANUP > ]) > > -- > 2.27.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Thu, Mar 17, 2022 at 11:46 AM Numan Siddique <numans@ovn.org> wrote: > > On Wed, Mar 9, 2022 at 4:39 AM Dumitru Ceara <dceara@redhat.com> wrote: > > > > This is not supported and ovn-northd was not logging it properly. > > The previous attempt tried to log in init_nat_entries() but there > > the logical router port configuration has not yet been parsed. > > > > Move the check and log instead to where we install the NAT flows. > > Also add a test. > > > > Fixes: 15348b7b806f ("ovn-northd: Multiple distributed gateway port support.") > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > Thanks. Applied to the main branch. Also backported to branch-22.03 and branch-21.12. Numan > > Numan > > > > --- > > northd/northd.c | 22 ++++++++++++---------- > > tests/ovn-northd.at | 6 ++++++ > > 2 files changed, 18 insertions(+), 10 deletions(-) > > > > diff --git a/northd/northd.c b/northd/northd.c > > index b264fb850b..7f309098e1 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -763,16 +763,6 @@ init_nat_entries(struct ovn_datapath *od) > > return; > > } > > > > - if (od->n_l3dgw_ports > 1) { > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > - VLOG_WARN_RL(&rl, "NAT is configured on logical router %s, which has %" > > - PRIuSIZE" distributed gateway ports. NAT is not supported" > > - " yet when there is more than one distributed gateway " > > - "port on the router.", > > - od->nbr->name, od->n_l3dgw_ports); > > - return; > > - } > > - > > od->nat_entries = xmalloc(od->nbr->n_nat * sizeof *od->nat_entries); > > > > for (size_t i = 0; i < od->nbr->n_nat; i++) { > > @@ -13243,6 +13233,18 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, > > return; > > } > > > > + /* NAT rules are not currently supported on logical routers with multiple > > + * distributed gateway ports. */ > > + if (od->n_l3dgw_ports > 1) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > + VLOG_WARN_RL(&rl, "NAT is configured on logical router %s, which has %" > > + PRIuSIZE" distributed gateway ports. NAT is not supported" > > + " yet when there is more than one distributed gateway " > > + "port on the router.", > > + od->nbr->name, od->n_l3dgw_ports); > > + return; > > + } > > + > > struct sset nat_entries = SSET_INITIALIZER(&nat_entries); > > > > bool dnat_force_snat_ip = > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 60d91a7717..17d4f31b39 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -5804,6 +5804,12 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep cr-DR | sed 's/table=../table=?? > > table=??(lr_in_gw_redirect ), priority=50 , match=(outport == "DR-S3"), action=(outport = "cr-DR-S3"; next;) > > ]) > > > > +# Check that ovn-northd logs a warning when trying to configure NAT > > +# on the router with multiple distributed gw ports. Such configurations are > > +# not supported yet. > > +check ovn-nbctl lr-nat-add DR dnat_and_snat 42.42.42.1 20.0.0.2 > > +AT_CHECK([grep -q 'NAT is configured on logical router DR, which has 2 distributed gateway ports. NAT is not supported yet when there is more than one distributed gateway port on the router.' northd/ovn-northd.log], [0]) > > + > > AT_CLEANUP > > ]) > > > > -- > > 2.27.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/northd/northd.c b/northd/northd.c index b264fb850b..7f309098e1 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -763,16 +763,6 @@ init_nat_entries(struct ovn_datapath *od) return; } - if (od->n_l3dgw_ports > 1) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "NAT is configured on logical router %s, which has %" - PRIuSIZE" distributed gateway ports. NAT is not supported" - " yet when there is more than one distributed gateway " - "port on the router.", - od->nbr->name, od->n_l3dgw_ports); - return; - } - od->nat_entries = xmalloc(od->nbr->n_nat * sizeof *od->nat_entries); for (size_t i = 0; i < od->nbr->n_nat; i++) { @@ -13243,6 +13233,18 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, return; } + /* NAT rules are not currently supported on logical routers with multiple + * distributed gateway ports. */ + if (od->n_l3dgw_ports > 1) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "NAT is configured on logical router %s, which has %" + PRIuSIZE" distributed gateway ports. NAT is not supported" + " yet when there is more than one distributed gateway " + "port on the router.", + od->nbr->name, od->n_l3dgw_ports); + return; + } + struct sset nat_entries = SSET_INITIALIZER(&nat_entries); bool dnat_force_snat_ip = diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 60d91a7717..17d4f31b39 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -5804,6 +5804,12 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep cr-DR | sed 's/table=../table=?? table=??(lr_in_gw_redirect ), priority=50 , match=(outport == "DR-S3"), action=(outport = "cr-DR-S3"; next;) ]) +# Check that ovn-northd logs a warning when trying to configure NAT +# on the router with multiple distributed gw ports. Such configurations are +# not supported yet. +check ovn-nbctl lr-nat-add DR dnat_and_snat 42.42.42.1 20.0.0.2 +AT_CHECK([grep -q 'NAT is configured on logical router DR, which has 2 distributed gateway ports. NAT is not supported yet when there is more than one distributed gateway port on the router.' northd/ovn-northd.log], [0]) + AT_CLEANUP ])
This is not supported and ovn-northd was not logging it properly. The previous attempt tried to log in init_nat_entries() but there the logical router port configuration has not yet been parsed. Move the check and log instead to where we install the NAT flows. Also add a test. Fixes: 15348b7b806f ("ovn-northd: Multiple distributed gateway port support.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- northd/northd.c | 22 ++++++++++++---------- tests/ovn-northd.at | 6 ++++++ 2 files changed, 18 insertions(+), 10 deletions(-)