netdev - 64-bit net_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1275514469.2115.70.camel@achroite.uk.solarflarecom.com>
Date:	Wed, 02 Jun 2010 22:34:29 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	David Miller <davem@...emloft.net>
Cc:	junchangwang@...il.com, romieu@...zoreil.com,
	netdev@...r.kernel.org
Subject: 64-bit net_device_stats

On Tue, 2010-05-25 at 16:15 -0700, David Miller wrote:
[...]
> If the problem is that people want 64-bit counters available for core
> statistics on 32-bit systems, we do not fix that problem by hacking
> every single driver to provide them side-band via ethtool.
> 
> First of all, we now have "struct rtnl_link_stats64" in
> linux/if_link.h, it's there to start combating this problem
> generically, for every device, rather than the way you are trying
> handle it only for one specific driver at a time.
> 
> So that's the area where you should start looking to solve these kinds
> of problem.

My understand of the current situation is as follows; correct me if any
of this is wrong:

The standard counters in struct net_device_stats have type unsigned long
which is the native word size and so can be read and updated
automatically.  Net drivers can update counters from the data path
without any interlocking with their ndo_get_stats implementation or the
networking core code which reads them.

The values returned by ndo_get_stats (by reference) are exposed:
- Through procfs (/proc/net/dev) as columns of numbers
- Through sysfs (/sys/class/net/*/stats/*) as single numeric strings
- Through netlink (IFLA_STATS and IFLA_STATS64) as 32-bit or 64-bit
  values in binary structures

Changing the counter types to u64 for 32-bit architectures would remove
atomicity and expose half-updated counters to userland.  Changing the
driver interface significantly so that atomicity is not needed would
require changes to hundreds of drivers.

Assuming the above is all correct, I think we can only solve this with a
gradual change (as for net_device_ops).  The following might work:

1. a. Define struct net_device_stats64 identically to rtnl_link_stats64.
   b. Add net_device_ops::ndo_get_stats64, the implementation of which
      will return a pointer to such a structure.  The referenced
      structure must only be updated atomically, except within the
      call to ndo_get_stats64.
   (For 64-bit architectures, these could be macro aliases to the
   existing structure and operations.)
   c. On 32-bit architectures, insert unsigned long padding after each
      member of struct net_device_stats.
   d. Add an anonymous union in net_device; move stats into the union
      and add struct net_device_stats64 stats64.
   e. Change dev_get_stats() to return a pointer to struct
      net_device_stats64, and to call ndo_get_stats64 if available in
      preference to ndo_get_stats.  Adjust callers accordingly.
   f. Either change dev_get_stats() to clear the padding members, or
      change drivers to ensure that the padding is cleared.

2. Change drivers to define ndo_get_stats64, following the rule
   stated in 1b.

3. Much later, remove net_device_stats.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html