Message ID | 20200519155816.24508-3-ihrachys@redhat.com |
---|---|
State | Superseded, archived |
Headers | show |
Series | Support logical switches with multiple localnet ports | expand |
On Tue, May 19, 2020 at 9:28 PM Ihar Hrachyshka <ihrachys@redhat.com> wrote: > Having some localnet ports missing a bridge device on a particular > chassis is a supported configuration (e.g. used to implement "routed > provider networks" for OpenStack) and should not flood logs with > duplicate messages. > > Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> > Acked-by: Dumitru Ceara <dceara@redhat.com> > Acked-by: Numan Siddique <numans@ovn.org> > --- > Thanks Ihar for addressing the comments. I applied both the patches to master. After applying I realized that we need to add this "support for multiple localnet ports" in the NEWS file. Can you please submit a follow up patch ? Thanks Numan controller/ovn-controller.c | 2 ++ > controller/patch.c | 36 ++++++++++++++++++++++++++++++++---- > controller/patch.h | 2 ++ > 3 files changed, 36 insertions(+), 4 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index c0a97a265..11fa2840d 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -1760,6 +1760,7 @@ main(int argc, char *argv[]) > > daemonize_complete(); > > + patch_init(); > pinctrl_init(); > lflow_init(); > > @@ -2271,6 +2272,7 @@ main(int argc, char *argv[]) > lflow_destroy(); > ofctrl_destroy(); > pinctrl_destroy(); > + patch_destroy(); > > ovsdb_idl_loop_destroy(&ovs_idl_loop); > ovsdb_idl_loop_destroy(&ovnsb_idl_loop); > diff --git a/controller/patch.c b/controller/patch.c > index 7ad30d9cc..a2a7bcd79 100644 > --- a/controller/patch.c > +++ b/controller/patch.c > @@ -24,9 +24,25 @@ > #include "openvswitch/hmap.h" > #include "openvswitch/vlog.h" > #include "ovn-controller.h" > +#include "sset.h" > > VLOG_DEFINE_THIS_MODULE(patch); > > +/* Contains list of physical bridges that were missing. */ > +static struct sset missed_bridges; > + > +void > +patch_init(void) > +{ > + sset_init(&missed_bridges); > +} > + > +void > +patch_destroy(void) > +{ > + sset_destroy(&missed_bridges); > +} > + > static char * > patch_port_name(const char *src, const char *dst) > { > @@ -223,20 +239,32 @@ add_bridge_mappings(struct ovsdb_idl_txn > *ovs_idl_txn, > binding->type, binding->logical_port); > continue; > } > + char *msg_key = xasprintf("%s;%s", binding->logical_port, > network); > struct ovsrec_bridge *br_ln = shash_find_data(&bridge_mappings, > network); > if (!br_ln) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > if (!is_localnet) { > + static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > VLOG_ERR_RL(&rl, "bridge not found for %s port '%s' " > "with network name '%s'", > binding->type, binding->logical_port, network); > } else { > - VLOG_INFO_RL(&rl, "bridge not found for localnet port > '%s' " > - "with network name '%s'; skipping", > - binding->logical_port, network); > + /* Since having localnet ports that are not mapped on some > + * chassis is a supported configuration used to implement > + * multisegment switches with fabric L3 routing between > + * segments, log the following message once per run but > don't > + * unnecessarily pollute the log file. */ > + if (!sset_contains(&missed_bridges, msg_key)) { > + VLOG_INFO("bridge not found for localnet port '%s' > with " > + "network name '%s'; skipping", > + binding->logical_port, network); > + sset_add(&missed_bridges, msg_key); > + } > } > + free(msg_key); > continue; > } > + sset_find_and_delete(&missed_bridges, msg_key); > + free(msg_key); > > char *name1 = patch_port_name(br_int->name, > binding->logical_port); > char *name2 = patch_port_name(binding->logical_port, > br_int->name); > diff --git a/controller/patch.h b/controller/patch.h > index 81a43bc2d..e470d502c 100644 > --- a/controller/patch.h > +++ b/controller/patch.h > @@ -43,5 +43,7 @@ void patch_run(struct ovsdb_idl_txn *ovs_idl_txn, > const struct ovsrec_bridge *br_int, > const struct sbrec_chassis *, > const struct hmap *local_datapaths); > +void patch_init(void); > +void patch_destroy(void); > > #endif /* controller/patch.h */ > -- > 2.26.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On 5/20/20 7:11 AM, Numan Siddique wrote: > On Tue, May 19, 2020 at 9:28 PM Ihar Hrachyshka <ihrachys@redhat.com> wrote: > >> Having some localnet ports missing a bridge device on a particular >> chassis is a supported configuration (e.g. used to implement "routed >> provider networks" for OpenStack) and should not flood logs with >> duplicate messages. >> >> Signed-off-by: Ihar Hrachyshka <ihrachys@redhat.com> >> Acked-by: Dumitru Ceara <dceara@redhat.com> >> Acked-by: Numan Siddique <numans@ovn.org> >> --- >> > Thanks Ihar for addressing the comments. > > I applied both the patches to master. > > After applying I realized that we need to add this "support for multiple > localnet ports" > in the NEWS file. > > Can you please submit a follow up patch ? Done: https://patchwork.ozlabs.org/project/openvswitch/patch/20200520151058.712439-1-ihrachys@redhat.com/ > > Thanks > Numan > > controller/ovn-controller.c | 2 ++ >> controller/patch.c | 36 ++++++++++++++++++++++++++++++++---- >> controller/patch.h | 2 ++ >> 3 files changed, 36 insertions(+), 4 deletions(-) >> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index c0a97a265..11fa2840d 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -1760,6 +1760,7 @@ main(int argc, char *argv[]) >> >> daemonize_complete(); >> >> + patch_init(); >> pinctrl_init(); >> lflow_init(); >> >> @@ -2271,6 +2272,7 @@ main(int argc, char *argv[]) >> lflow_destroy(); >> ofctrl_destroy(); >> pinctrl_destroy(); >> + patch_destroy(); >> >> ovsdb_idl_loop_destroy(&ovs_idl_loop); >> ovsdb_idl_loop_destroy(&ovnsb_idl_loop); >> diff --git a/controller/patch.c b/controller/patch.c >> index 7ad30d9cc..a2a7bcd79 100644 >> --- a/controller/patch.c >> +++ b/controller/patch.c >> @@ -24,9 +24,25 @@ >> #include "openvswitch/hmap.h" >> #include "openvswitch/vlog.h" >> #include "ovn-controller.h" >> +#include "sset.h" >> >> VLOG_DEFINE_THIS_MODULE(patch); >> >> +/* Contains list of physical bridges that were missing. */ >> +static struct sset missed_bridges; >> + >> +void >> +patch_init(void) >> +{ >> + sset_init(&missed_bridges); >> +} >> + >> +void >> +patch_destroy(void) >> +{ >> + sset_destroy(&missed_bridges); >> +} >> + >> static char * >> patch_port_name(const char *src, const char *dst) >> { >> @@ -223,20 +239,32 @@ add_bridge_mappings(struct ovsdb_idl_txn >> *ovs_idl_txn, >> binding->type, binding->logical_port); >> continue; >> } >> + char *msg_key = xasprintf("%s;%s", binding->logical_port, >> network); >> struct ovsrec_bridge *br_ln = shash_find_data(&bridge_mappings, >> network); >> if (!br_ln) { >> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); >> if (!is_localnet) { >> + static struct vlog_rate_limit rl = >> VLOG_RATE_LIMIT_INIT(5, 1); >> VLOG_ERR_RL(&rl, "bridge not found for %s port '%s' " >> "with network name '%s'", >> binding->type, binding->logical_port, network); >> } else { >> - VLOG_INFO_RL(&rl, "bridge not found for localnet port >> '%s' " >> - "with network name '%s'; skipping", >> - binding->logical_port, network); >> + /* Since having localnet ports that are not mapped on some >> + * chassis is a supported configuration used to implement >> + * multisegment switches with fabric L3 routing between >> + * segments, log the following message once per run but >> don't >> + * unnecessarily pollute the log file. */ >> + if (!sset_contains(&missed_bridges, msg_key)) { >> + VLOG_INFO("bridge not found for localnet port '%s' >> with " >> + "network name '%s'; skipping", >> + binding->logical_port, network); >> + sset_add(&missed_bridges, msg_key); >> + } >> } >> + free(msg_key); >> continue; >> } >> + sset_find_and_delete(&missed_bridges, msg_key); >> + free(msg_key); >> >> char *name1 = patch_port_name(br_int->name, >> binding->logical_port); >> char *name2 = patch_port_name(binding->logical_port, >> br_int->name); >> diff --git a/controller/patch.h b/controller/patch.h >> index 81a43bc2d..e470d502c 100644 >> --- a/controller/patch.h >> +++ b/controller/patch.h >> @@ -43,5 +43,7 @@ void patch_run(struct ovsdb_idl_txn *ovs_idl_txn, >> const struct ovsrec_bridge *br_int, >> const struct sbrec_chassis *, >> const struct hmap *local_datapaths); >> +void patch_init(void); >> +void patch_destroy(void); >> >> #endif /* controller/patch.h */ >> -- >> 2.26.2 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >>
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index c0a97a265..11fa2840d 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1760,6 +1760,7 @@ main(int argc, char *argv[]) daemonize_complete(); + patch_init(); pinctrl_init(); lflow_init(); @@ -2271,6 +2272,7 @@ main(int argc, char *argv[]) lflow_destroy(); ofctrl_destroy(); pinctrl_destroy(); + patch_destroy(); ovsdb_idl_loop_destroy(&ovs_idl_loop); ovsdb_idl_loop_destroy(&ovnsb_idl_loop); diff --git a/controller/patch.c b/controller/patch.c index 7ad30d9cc..a2a7bcd79 100644 --- a/controller/patch.c +++ b/controller/patch.c @@ -24,9 +24,25 @@ #include "openvswitch/hmap.h" #include "openvswitch/vlog.h" #include "ovn-controller.h" +#include "sset.h" VLOG_DEFINE_THIS_MODULE(patch); +/* Contains list of physical bridges that were missing. */ +static struct sset missed_bridges; + +void +patch_init(void) +{ + sset_init(&missed_bridges); +} + +void +patch_destroy(void) +{ + sset_destroy(&missed_bridges); +} + static char * patch_port_name(const char *src, const char *dst) { @@ -223,20 +239,32 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn, binding->type, binding->logical_port); continue; } + char *msg_key = xasprintf("%s;%s", binding->logical_port, network); struct ovsrec_bridge *br_ln = shash_find_data(&bridge_mappings, network); if (!br_ln) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); if (!is_localnet) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); VLOG_ERR_RL(&rl, "bridge not found for %s port '%s' " "with network name '%s'", binding->type, binding->logical_port, network); } else { - VLOG_INFO_RL(&rl, "bridge not found for localnet port '%s' " - "with network name '%s'; skipping", - binding->logical_port, network); + /* Since having localnet ports that are not mapped on some + * chassis is a supported configuration used to implement + * multisegment switches with fabric L3 routing between + * segments, log the following message once per run but don't + * unnecessarily pollute the log file. */ + if (!sset_contains(&missed_bridges, msg_key)) { + VLOG_INFO("bridge not found for localnet port '%s' with " + "network name '%s'; skipping", + binding->logical_port, network); + sset_add(&missed_bridges, msg_key); + } } + free(msg_key); continue; } + sset_find_and_delete(&missed_bridges, msg_key); + free(msg_key); char *name1 = patch_port_name(br_int->name, binding->logical_port); char *name2 = patch_port_name(binding->logical_port, br_int->name); diff --git a/controller/patch.h b/controller/patch.h index 81a43bc2d..e470d502c 100644 --- a/controller/patch.h +++ b/controller/patch.h @@ -43,5 +43,7 @@ void patch_run(struct ovsdb_idl_txn *ovs_idl_txn, const struct ovsrec_bridge *br_int, const struct sbrec_chassis *, const struct hmap *local_datapaths); +void patch_init(void); +void patch_destroy(void); #endif /* controller/patch.h */