Re: [ovs-dev] [PATCH ovn v1 2/2] northd: Log all dynamic address assignments
On Mon, Dec 9, 2019 at 11:18 AM Numan Siddique wrote:
> On Mon, Dec 9, 2019 at 8:17 AM Russell Bryant wrote:
> >
> > On Mon, Dec 9, 2019 at 3:01 AM Dumitru Ceara wrote:
> >
> > > On Sun, Dec 8, 2019 at 5:12 AM Russell Bryant 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 .
>
Thanks, I've pushed this to master.
>
> Thanks
> Numan
>
> > >
> > > Thanks,
> > > Dumitru
> > >
> > > > Signed-off-by: Russell Bryant
> > > > ---
> > > > 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), );
> > > > +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, >od->ipam_info.ipv6_prefix,
> > > );
> > > > +struct ds ip6_ds = DS_EMPTY_INITIALIZER;
> > > > +ipv6_format_addr(, _ds);
> > > > +VLOG_INFO("Assigned dynamic IPv6 address '%s' to port '%s'",
> > > > + ip6_ds.string, update->op->nbsp->name);
> > > > +ds_destroy(_ds);
> > > > break;
> > > > }
> > > >
> > > > --
> > > > 2.23.0
> > > >
> > > > ___
> > > > dev mailing list
> > > > d...@openvswitch.org
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > >
> > >
> > >
> >
> > --
> > Russell Bryant
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>
--
Russell Bryant
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v1 2/2] northd: Log all dynamic address assignments
On Mon, Dec 9, 2019 at 8:17 AM Russell Bryant wrote:
>
> On Mon, Dec 9, 2019 at 3:01 AM Dumitru Ceara wrote:
>
> > On Sun, Dec 8, 2019 at 5:12 AM Russell Bryant 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 .
Thanks
Numan
> >
> > Thanks,
> > Dumitru
> >
> > > Signed-off-by: Russell Bryant
> > > ---
> > > 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), );
> > > +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, >od->ipam_info.ipv6_prefix,
> > );
> > > +struct ds ip6_ds = DS_EMPTY_INITIALIZER;
> > > +ipv6_format_addr(, _ds);
> > > +VLOG_INFO("Assigned dynamic IPv6 address '%s' to port '%s'",
> > > + ip6_ds.string, update->op->nbsp->name);
> > > +ds_destroy(_ds);
> > > break;
> > > }
> > >
> > > --
> > > 2.23.0
> > >
> > > ___
> > > dev mailing list
> > > d...@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> >
> >
>
> --
> Russell Bryant
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v1 2/2] northd: Log all dynamic address assignments
On Mon, Dec 9, 2019 at 3:01 AM Dumitru Ceara wrote:
> On Sun, Dec 8, 2019 at 5:12 AM Russell Bryant 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.
>
> Thanks,
> Dumitru
>
> > Signed-off-by: Russell Bryant
> > ---
> > 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), );
> > +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, >od->ipam_info.ipv6_prefix,
> );
> > +struct ds ip6_ds = DS_EMPTY_INITIALIZER;
> > +ipv6_format_addr(, _ds);
> > +VLOG_INFO("Assigned dynamic IPv6 address '%s' to port '%s'",
> > + ip6_ds.string, update->op->nbsp->name);
> > +ds_destroy(_ds);
> > break;
> > }
> >
> > --
> > 2.23.0
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>
--
Russell Bryant
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v1 2/2] northd: Log all dynamic address assignments
On Sun, Dec 8, 2019 at 5:12 AM Russell Bryant 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.
Thanks,
Dumitru
> Signed-off-by: Russell Bryant
> ---
> 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), );
> +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, >od->ipam_info.ipv6_prefix, );
> +struct ds ip6_ds = DS_EMPTY_INITIALIZER;
> +ipv6_format_addr(, _ds);
> +VLOG_INFO("Assigned dynamic IPv6 address '%s' to port '%s'",
> + ip6_ds.string, update->op->nbsp->name);
> +ds_destroy(_ds);
> break;
> }
>
> --
> 2.23.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v1 2/2] northd: Log all dynamic address assignments
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.
Signed-off-by: Russell Bryant
---
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), );
+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, >od->ipam_info.ipv6_prefix, );
+struct ds ip6_ds = DS_EMPTY_INITIALIZER;
+ipv6_format_addr(, _ds);
+VLOG_INFO("Assigned dynamic IPv6 address '%s' to port '%s'",
+ ip6_ds.string, update->op->nbsp->name);
+ds_destroy(_ds);
break;
}
--
2.23.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
