[ovs-dev] [PATCH ovn v1 2/2] northd: Log all dynamic address assignments

[Numan Siddique]

On Mon, Dec 9, 2019 at 8:17 AM Russell Bryant <rbryant at redhat.com> wrote:
>
> On Mon, Dec 9, 2019 at 3:01 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> > On Sun, Dec 8, 2019 at 5:12 AM Russell Bryant <russell at ovn.org> wrote:
> > >
> > > This patch adds INFO level log messages for all dynamic address
> > > assignments (MAC, IPv4, IPv6).  While debugging some issues in
> > > ovn-kubernetes, I found it would be helpful to see ovn-northd's view
> > > of what addresses were assigned where and when from its perspective.
> > >
> >
> > Hi Russsell,
> >
> > While I agree that having this information is really useful for
> > debugging, the INFO logs are enabled by default.
> > Should we consider rate limiting the logs you added?
> >
> > For example, looking at the WARN logs in northd, all of them are rate
> > limited.
> >
>
> We could ... it'd be a little bit extra tracking that would hopefully never
> be needed.  It'd be a bug if the same message was emitted more than once at
> all.
>

Since this code would not hit all the time when ovn_db_run is called, I think
VLOG_INFO should not cause any log flooding.


Acked-by: Numan Siddique <numans at ovn.org>.

Thanks
Numan

> >
> > Thanks,
> > Dumitru
> >
> > > Signed-off-by: Russell Bryant <russell at ovn.org>
> > > ---
> > >  northd/ovn-northd.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > > index f0847d81e..33d3ff2ad 100644
> > > --- a/northd/ovn-northd.c
> > > +++ b/northd/ovn-northd.c
> > > @@ -1714,6 +1714,8 @@ update_dynamic_addresses(struct
> > dynamic_address_update *update)
> > >          break;
> > >      case DYNAMIC:
> > >          ip4 = htonl(ipam_get_unused_ip(update->od));
> > > +        VLOG_INFO("Assigned dynamic IPv4 address '"IP_FMT"' to port
> > '%s'",
> > > +                  IP_ARGS(ip4), update->op->nbsp->name);
> > >      }
> > >
> > >      struct eth_addr mac;
> > > @@ -1728,6 +1730,8 @@ update_dynamic_addresses(struct
> > dynamic_address_update *update)
> > >          break;
> > >      case DYNAMIC:
> > >          eth_addr_from_uint64(ipam_get_unused_mac(ip4), &mac);
> > > +        VLOG_INFO("Assigned dynamic MAC address '"ETH_ADDR_FMT"' to
> > port '%s'",
> > > +                  ETH_ADDR_ARGS(mac), update->op->nbsp->name);
> > >          break;
> > >      }
> > >
> > > @@ -1745,6 +1749,11 @@ update_dynamic_addresses(struct
> > dynamic_address_update *update)
> > >          break;
> > >      case DYNAMIC:
> > >          in6_generate_eui64(mac, &update->od->ipam_info.ipv6_prefix,
> > &ip6);
> > > +        struct ds ip6_ds = DS_EMPTY_INITIALIZER;
> > > +        ipv6_format_addr(&ip6, &ip6_ds);
> > > +        VLOG_INFO("Assigned dynamic IPv6 address '%s' to port '%s'",
> > > +                  ip6_ds.string, update->op->nbsp->name);
> > > +        ds_destroy(&ip6_ds);
> > >          break;
> > >      }
> > >
> > > --
> > > 2.23.0
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev at openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> >
> >
>
> --
> Russell Bryant
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>