| Message ID | 1502080323-11190-3-git-send-email-roid@mellanox.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Mon, Aug 07, 2017 at 07:32:03AM +0300, Roi Dayan wrote: > These are normal and unavoidable, because the vifs > disappear from the kernel before they are removed them from the OVS > database. > > Signed-off-by: Roi Dayan <roid@mellanox.com> > Reviewed-by: Paul Blakey <paulb@mellanox.com> > --- > lib/netdev-linux.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 98820ed..f5aa9c9 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -5438,8 +5438,12 @@ linux_get_ifindex(const char *netdev_name) > > error = af_inet_ioctl(SIOCGIFINDEX, &ifr); > if (error) { > - VLOG_WARN_RL(&rl, "ioctl(SIOCGIFINDEX) on %s device failed: %s", > - netdev_name, ovs_strerror(error)); > + /* ENODEV probably means that a vif disappeared asynchronously and > + * hasn't been removed from the database yet, so reduce the log level > + * to INFO for that case. */ > + VLOG(error == ENODEV ? VLL_INFO : VLL_ERR, > + "ioctl(SIOCGIFINDEX) on %s device failed: %s", > + netdev_name, ovs_strerror(error)); > return -error; > } > return ifr.ifr_ifindex; This may be a reasonable change, but why remove the ratelimiting?
On 07/08/2017 17:01, Ben Pfaff wrote: > On Mon, Aug 07, 2017 at 07:32:03AM +0300, Roi Dayan wrote: >> These are normal and unavoidable, because the vifs >> disappear from the kernel before they are removed them from the OVS >> database. >> >> Signed-off-by: Roi Dayan <roid@mellanox.com> >> Reviewed-by: Paul Blakey <paulb@mellanox.com> >> --- >> lib/netdev-linux.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c >> index 98820ed..f5aa9c9 100644 >> --- a/lib/netdev-linux.c >> +++ b/lib/netdev-linux.c >> @@ -5438,8 +5438,12 @@ linux_get_ifindex(const char *netdev_name) >> >> error = af_inet_ioctl(SIOCGIFINDEX, &ifr); >> if (error) { >> - VLOG_WARN_RL(&rl, "ioctl(SIOCGIFINDEX) on %s device failed: %s", >> - netdev_name, ovs_strerror(error)); >> + /* ENODEV probably means that a vif disappeared asynchronously and >> + * hasn't been removed from the database yet, so reduce the log level >> + * to INFO for that case. */ >> + VLOG(error == ENODEV ? VLL_INFO : VLL_ERR, >> + "ioctl(SIOCGIFINDEX) on %s device failed: %s", >> + netdev_name, ovs_strerror(error)); >> return -error; >> } >> return ifr.ifr_ifindex; > > This may be a reasonable change, but why remove the ratelimiting? > because I actually took the same call from get_etheraddr() which and doesn't use a ratelimit. I'll bring it back.
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 98820ed..f5aa9c9 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -5438,8 +5438,12 @@ linux_get_ifindex(const char *netdev_name) error = af_inet_ioctl(SIOCGIFINDEX, &ifr); if (error) { - VLOG_WARN_RL(&rl, "ioctl(SIOCGIFINDEX) on %s device failed: %s", - netdev_name, ovs_strerror(error)); + /* ENODEV probably means that a vif disappeared asynchronously and + * hasn't been removed from the database yet, so reduce the log level + * to INFO for that case. */ + VLOG(error == ENODEV ? VLL_INFO : VLL_ERR, + "ioctl(SIOCGIFINDEX) on %s device failed: %s", + netdev_name, ovs_strerror(error)); return -error; } return ifr.ifr_ifindex;