All of lore.kernel.org
 help / color / mirror / Atom feed
* ndo_get_stats and rtnl_netlink
@ 2012-09-09 15:23 Shlomo Pongartz
  2012-09-10  9:09 ` Eric Dumazet
  0 siblings, 1 reply; 4+ messages in thread
From: Shlomo Pongartz @ 2012-09-09 15:23 UTC (permalink / raw)
  To: netdev

Hi,

Just realized that dev_get_stats which calls into a netdevice 
ndo_get_stats64/ndo_get_stats can be
called with or without RTNL lock protection. If called from 
rtnl_fill_ifinfo e.g as of invocation of
"ip link show <interface>, there IS locking, however if called from 
dev_seq_printf_stats e.g as of
invocation of reading the /sys/class/net/<interface>/statistics/ 
entries, etc more cases -- no locking.

This turned to be problematic when implementing the ethtool 
"set_channels" directive which
changes the number of **rings**, since we stepped on a bug where the 
rings data structure was
changed by the ethtool flow in the same time a statistics call was done 
into the driver, etc.

What would be the way to continue here, per driver lock sounds non 
generic...

Regards
Shlomo Pongratz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: ndo_get_stats and rtnl_netlink
  2012-09-09 15:23 ndo_get_stats and rtnl_netlink Shlomo Pongartz
@ 2012-09-10  9:09 ` Eric Dumazet
  2012-09-10 19:10   ` Or Gerlitz
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2012-09-10  9:09 UTC (permalink / raw)
  To: Shlomo Pongartz; +Cc: netdev

On Sun, 2012-09-09 at 18:23 +0300, Shlomo Pongartz wrote:
> Hi,
> 
> Just realized that dev_get_stats which calls into a netdevice 
> ndo_get_stats64/ndo_get_stats can be
> called with or without RTNL lock protection. If called from 
> rtnl_fill_ifinfo e.g as of invocation of
> "ip link show <interface>, there IS locking, however if called from 
> dev_seq_printf_stats e.g as of
> invocation of reading the /sys/class/net/<interface>/statistics/ 
> entries, etc more cases -- no locking.
> 
> This turned to be problematic when implementing the ethtool 
> "set_channels" directive which
> changes the number of **rings**, since we stepped on a bug where the 
> rings data structure was
> changed by the ethtool flow in the same time a statistics call was done 
> into the driver, etc.
> 
> What would be the way to continue here, per driver lock sounds non 
> generic...

RTNL locking is not needed to fetch stats, and would be overkill.

Each driver has its own way to protect/gather its stats.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: ndo_get_stats and rtnl_netlink
  2012-09-10  9:09 ` Eric Dumazet
@ 2012-09-10 19:10   ` Or Gerlitz
  2012-09-10 19:15     ` Eric Dumazet
  0 siblings, 1 reply; 4+ messages in thread
From: Or Gerlitz @ 2012-09-10 19:10 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Shlomo Pongartz, netdev

On Mon, Sep 10, 2012 at 12:09 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> RTNL locking is not needed to fetch stats, and would be overkill.
> Each driver has its own way to protect/gather its stats.

FWIW, again, when dev_get_stats is called from rtnl_fill_ifinfo e.g as
of invocation
of "ip link show <interface>, there IS RTNL locking, is that a problem?

Or.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: ndo_get_stats and rtnl_netlink
  2012-09-10 19:10   ` Or Gerlitz
@ 2012-09-10 19:15     ` Eric Dumazet
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2012-09-10 19:15 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: Shlomo Pongartz, netdev

On Mon, 2012-09-10 at 22:10 +0300, Or Gerlitz wrote:
> On Mon, Sep 10, 2012 at 12:09 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> > RTNL locking is not needed to fetch stats, and would be overkill.
> > Each driver has its own way to protect/gather its stats.
> 
> FWIW, again, when dev_get_stats is called from rtnl_fill_ifinfo e.g as
> of invocation
> of "ip link show <interface>, there IS RTNL locking, is that a problem?
> 
> Or.

Why should it be a problem ?

A dev_get_stats() is not allowed to sleep, so its not allowed to use
RTNL by itself.

So caller might use RTNL, we dont care at all.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-09-10 19:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-09 15:23 ndo_get_stats and rtnl_netlink Shlomo Pongartz
2012-09-10  9:09 ` Eric Dumazet
2012-09-10 19:10   ` Or Gerlitz
2012-09-10 19:15     ` Eric Dumazet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.