netdev - Re: [PATCH 02/33] netdev: introduce dev_get

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]

Date:	Thu, 20 Nov 2008 10:17:02 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	David Miller <davem@...emloft.net>
CC:	shemminger@...tta.com, netdev@...r.kernel.org
Subject: Re: [PATCH 02/33] netdev: introduce dev_get_stats()

David Miller a écrit :
> From: Stephen Hemminger <shemminger@...tta.com>
> Date: Mon, 17 Nov 2008 15:42:09 -0800
> 
>> In order for the network device ops get_stats call to be immutable, the handling
>> of the default internal network device stats block has to be changed. Add a new
>> helper function which replaces the old use of internal_get_stats.
>>
>> Note: change return code to make it clear that the caller should not
>> go changing the returned statistics.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@...tta.com>
> 
> Applied.

I have one comment about netdev stats

on 32bit arches, SMP :

struct net_device = 0x480
offsetof(struct net_device, features)=0x44
offsetof(struct net_device, stats)=0x50
offsetof(struct net_device, stats.rx_packets)=0x50

So we trash features field, thats a problem...

I wonder if we could zap stats from netdev structure.
Some drivers already use external stats handling (like loopback)

get_dev_stats() would accept a second parameter : a pointer to a struct net_device_stats
provided by the reader, that the driver could use as a working zone, or not.

static struct net_device_stats *bond_get_stats(struct net_device *bond_dev,
	struct net_device_stats *temp)
{
        struct bonding *bond = netdev_priv(bond_dev);
        struct net_device_stats wrk;
        struct slave *slave;
        int i;

	/* use the caller provided zone */
        memset(temp, 0, sizeof(struct net_device_stats));

        read_lock_bh(&bond->lock);

        bond_for_each_slave(bond, slave, i) {
                const struct net_device_stats *sstats = dev_get_stats(slave->dev, &wrk);

                temp->rx_packets += sstats->rx_packets;
                temp->rx_bytes += sstats->rx_bytes;
                temp->rx_errors += sstats->rx_errors;
		...
	}


        read_unlock_bh(&bond->lock);

        return temp;
}

....

static struct net_device_stats *loopback_get_stats(struct net_device *dev,
			struct net_device_stats *temp)
{
        const struct pcpu_lstats *pcpu_lstats;
        unsigned long bytes = 0;
        unsigned long packets = 0;
        int i;

	memset(temp, 0, sizeof(*temp);
        pcpu_lstats = dev->ml_priv;
        for_each_possible_cpu(i) {
                const struct pcpu_lstats *lb_stats;

                lb_stats = per_cpu_ptr(pcpu_lstats, i);
                bytes   += lb_stats->bytes;
                packets += lb_stats->packets;
        }
        temp->rx_packets = packets;
        temp->tx_packets = packets;
        temp->rx_bytes = bytes;
        temp->tx_bytes = bytes;
        return temp;
}

...

--
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