On Fri, Feb 28, 2020 at 5:53 PM Numan Siddique <[email protected]> wrote:
>
> On Thu, Feb 27, 2020 at 6:46 PM Frode Nordahl
> <[email protected]> wrote:
> >
> > There is a pattern among CMSes to create a `localnet` port
> > binding without any chassis affiliation.
> >
> > It is then up to the user to configure chassis with external
> > connectivity by adding a mapping under the
> > `external_ids:ovn-bridge-mappings` key in the Open_vSwitch
> > table of the local OVS instance.  This may be some chassis
> > or all chassis depending on end user requirements.
> >
>
> Hi Frode,
>
> Thanks for the patch. I've few comments.

Thank you for swift review/response, Numan

> ovn-controller creates a patch port between br-int and provider bridge only
> if it is required. Let's say you create a distributed router port and
> set the gateway chassis (or ha chassis group)
> for this port, then if no ovn-bridge-mappings is configured on that
> gateway chassis, ovn-controller logs
> it as error.
>
> I think that should be fine.
>
> ovn-controller also creates a bridge mapping if you create a logical
> port on the provider logical switch (i.e
> with localnet port) and this port is claimed by a chassis. I think
> even in this scenario  it should log as error.
>
> Why do you think it is not an error ? Probably ovn-controller should
> not even try to create a patch port if
> it is not really required. Probably we should fix the error there.
> What do you think ?

I'm definitely for solving the problem rather than the symptoms, so it
may well be I have jumped the gun on this one and made an incorrect
assumption or two.

I'll go back and track down when/where something changed.

Cheers!

--
Frode Nordahl


> Thanks
> Numan
>
>
> > At present `ovn-controller` will repeatedly log an error on
> > chassis without mapping for a localnet port while in reality
> > it is not an error.
> >
> > Signed-off-by: Frode Nordahl <[email protected]>
> > ---
> >  controller/patch.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/controller/patch.c b/controller/patch.c
> > index 349faae17..2eb62b193 100644
> > --- a/controller/patch.c
> > +++ b/controller/patch.c
> > @@ -198,8 +198,13 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
> >              continue;
> >          }
> >
> > +        enum vlog_level level = VLL_ERR;
> >          const char *patch_port_id;
> >          if (!strcmp(binding->type, "localnet")) {
> > +            /* Not having external connectivity present on all chassis is
> > +             * a feature our user may choose to use, let's not log it as an
> > +             * error. */
> > +            level = VLL_DBG;
> >              patch_port_id = "ovn-localnet-port";
> >          } else if (!strcmp(binding->type, "l2gateway")) {
> >              if (!binding->chassis
> > @@ -224,7 +229,7 @@ add_bridge_mappings(struct ovsdb_idl_txn *ovs_idl_txn,
> >          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);
> > -            VLOG_ERR_RL(&rl, "bridge not found for %s port '%s' "
> > +            VLOG_RL(&rl, level, "bridge not found for %s port '%s' "
> >                      "with network name '%s'",
> >                      binding->type, binding->logical_port, network);
> >              continue;
> > --
> > 2.25.0
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to