netdev - Re: [PATCH 2/3] NET: [CORE] Stack changes to add multiqueue hardware support API

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 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:	Fri, 29 Jun 2007 06:54:37 -0400
From:	Jeff Garzik <jeff@...zik.org>
To:	David Miller <davem@...emloft.net>
CC:	peter.p.waskiewicz.jr@...el.com, netdev@...r.kernel.org,
	auke-jan.h.kok@...el.com, hadi@...erus.ca, kaber@...sh.net
Subject: Re: [PATCH 2/3] NET: [CORE] Stack changes to add multiqueue hardware
 support API

David Miller wrote:
> From: PJ Waskiewicz <peter.p.waskiewicz.jr@...el.com>
> Date: Thu, 28 Jun 2007 09:21:13 -0700
> 
>> -struct net_device *alloc_netdev(int sizeof_priv, const char *name,
>> -		void (*setup)(struct net_device *))
>> +struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
>> +		void (*setup)(struct net_device *), int queue_count)
>>  {
>>  	void *p;
>>  	struct net_device *dev;
>> @@ -3557,7 +3564,9 @@ struct net_device *alloc_netdev(int sizeof_priv, const char *name,
>>  	BUG_ON(strlen(name) >= sizeof(dev->name));
>>  
>>  	/* ensure 32-byte alignment of both the device and private area */
>> -	alloc_size = (sizeof(*dev) + NETDEV_ALIGN_CONST) & ~NETDEV_ALIGN_CONST;
>> +	alloc_size = (sizeof(*dev) + NETDEV_ALIGN_CONST +
>> +		     (sizeof(struct net_device_subqueue) * queue_count)) &
>> +		     ~NETDEV_ALIGN_CONST;
>>  	alloc_size += sizeof_priv + NETDEV_ALIGN_CONST;
>>  
>>  	p = kzalloc(alloc_size, GFP_KERNEL);
>> @@ -3573,12 +3582,14 @@ struct net_device *alloc_netdev(int sizeof_priv, const char *name,
>>  	if (sizeof_priv)
>>  		dev->priv = netdev_priv(dev);
>>  
>> +	dev->egress_subqueue_count = queue_count;
>> +
>>  	dev->get_stats = internal_stats;
>>  	setup(dev);
>>  	strcpy(dev->name, name);
>>  	return dev;
>>  }
> 
> This isn't going to work.
> 
> The pointer returned from netdev_priv() doesn't take into account the
> variable sized queues at the end of struct netdev, so we can stomp
> over the queues with the private area.
> 
> This probably works by luck because of NETDEV_ALIGN.
> 
> The simplest fix is to just make netdev_priv() use dev->priv,
> except when it's being initialized during allocation, and
> that's what I'm going to do when I apply your patch.

Ugh.  That will reverse the gains we had with the current setup, won't it?

Also, what happens when we want to add ingress_queue[0] ?

	Jeff





-
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