Re: [net-next-2.6 PATCH v5 1/2] net: implement mechanism for HW based QOS

On 1/6/2011 10:31 AM, Eric Dumazet wrote:

Le jeudi 06 janvier 2011 à 18:20 +0000, Ben Hutchings a écrit :

quoted

On Tue, 2011-01-04 at 10:56 -0800, John Fastabend wrote:
[...]

quoted

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0f6b1c9..ae51323 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h

@@ -646,6 +646,14 @@ struct xps_dev_maps {
     (nr_cpu_ids * sizeof(struct xps_map *)))
 #endif /* CONFIG_XPS */
 
+#define TC_MAX_QUEUE	16
+#define TC_BITMASK	15
+/* HW offloaded queuing disciplines txq count and offset maps */
+struct netdev_tc_txq {
+	u16 count;
+	u16 offset;
+};
+
 /*
  * This structure defines the management hooks for network devices.
  * The following hooks can be defined; unless noted otherwise, they are

@@ -1146,6 +1154,9 @@ struct net_device {
 	/* Data Center Bridging netlink ops */
 	const struct dcbnl_rtnl_ops *dcbnl_ops;
 #endif
+	u8 num_tc;
+	struct netdev_tc_txq tc_to_txq[TC_MAX_QUEUE];
+	u8 prio_tc_map[TC_BITMASK+1];

[...]

I'm still concerned by the addition of all this state to every
net_device.  From previous discussion, Eric wanted this, citing 'false
sharing' while Stephen thought it should be accessed indirectly.

Eric, when you refer to 'false sharing' do you mean that the TC state
might end up sharing a cache line with some other data?  That seems
quite unlikely as the allocation size will be 128 bytes, and it could be
padded to fill a cache line if that's still a concern.

At the time I made a comment, the allocated data was less than 64 bytes

Problem is adding so many indirections here and here reduce latencies on
workloads handling a few packets per second.

sizeof(struct net_device)=0x600

We currently have 512 unused bytes (because of kmalloc() power of two)

(Most virtual devices have small private part added to net_device.
The real devices are probably crossing the 0x800 limit (or even 0x1000))

It should still be 64 bytes.

struct netdev_tc_txq {
        u16 count;
        u16 offset;
};

struct netdev_tc_txq (4 octets) and TC_MAX_QUEUE = 16, so 4*16 = 64

+	struct netdev_tc_txq tc_to_txq[TC_MAX_QUEUE];   => 64 octets
+	u8 prio_tc_map[TC_BITMASK+1];			=> 16 octets

All together 80 bytes this is certainly in the 512 unused bytes.