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

[John Fastabend <john.r.fastabend@intel.com>]

On 1/6/2011 10:31 AM, Eric Dumazet wrote:
> Le jeudi 06 janvier 2011 à 18:20 +0000, Ben Hutchings a écrit :
>> On Tue, 2011-01-04 at 10:56 -0800, John Fastabend wrote:
>> [...]
>>> 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.