netdev - Re: [PATCH net-next-2.6] rps: allocate rx queues in register

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: <AANLkTimQ+Vag=bqscVzsU6G2oCUrWNn6rCF-TggiBC_+@mail.gmail.com>
Date:	Fri, 24 Sep 2010 08:58:32 -0700
From:	Tom Herbert <therbert@...gle.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Ben Hutchings <bhutchings@...arflare.com>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	linux-net-drivers@...arflare.com
Subject: Re: [PATCH net-next-2.6] rps: allocate rx queues in
 register_netdevice only

On Fri, Sep 24, 2010 at 12:07 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> Le vendredi 24 septembre 2010 à 05:26 +0200, Eric Dumazet a écrit :
>> > Also, I dont understand why we need to restrict
>> > netif_set_real_num_rx_queues() to lower the count.
>> > This wastes memory.
>> >
>> > Why dont we allocate dev->_rx once we know the real count, not in
>> > alloc_netdev_mq() but in register_netdevice() ?
>> >
>> >
>>
>> Here is a patch to make this possible
>>
>> I guess a similar thing could be done for tx queues.
>>
>> boot tested, but please double check.
>>
>> Thanks
>>
>> [PATCH net-next-2.6] rps: allocate rx queues in register_netdevice()
>>
>> Instead of having two places were we allocate dev->_rx, introduce
>> netif_alloc_rx_queues() helper and call it only from
>> register_netdevice(), not from alloc_netdev_mq()
>>
>> Goal is to let drivers change dev->num_rx_queues after allocating netdev
>> and before registering it.
>>
>> This also removes a lot of ifdefs in net/core/dev.c
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
>> ---
>>  net/core/dev.c |   76 +++++++++++++++++++----------------------------
>>  1 file changed, 32 insertions(+), 44 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 2c7934f..9110b8d 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -4964,6 +4964,34 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
>>  }
>>  EXPORT_SYMBOL(netif_stacked_transfer_operstate);
>>
>> +static int netif_alloc_rx_queues(struct net_device *dev)
>> +{
>> +#ifdef CONFIG_RPS
>> +     unsigned int i, count = dev->num_rx_queues;
>> +
>
> Tom
>
> I tried to find a caller with dev->num_rx_queues = 0, but failed
>
> If it's valid, we might add a
>
>        if (!dev->num_rx_queues)
>                dev->num_rx_queues = 1;
>
Or maybe just enforce that in register_net_device and make it an error
to call alloc_netdev_mq with zero queue count.  You've uncovered a bug
since get_rps_cpus assumes at least one queue, but alloc_netdev_mq
allows zeros queues to be allocated!

> I wonder if you remember why you added the section :
>        /*
>         * Allocate a single RX queue if driver never called
>         * alloc_netdev_mq
>         */

RPS is configured per queue, so every device will need at least one RX
queue for the structures.
--
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