netdev - Re: [PATCH 1/2] rps: core implementation

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: <65634d660911160902t677a4fb4r71602cb0d10f1cce@mail.gmail.com>
Date:	Mon, 16 Nov 2009 09:02:32 -0800
From:	Tom Herbert <therbert@...gle.com>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH 1/2] rps: core implementation

>> +     case __constant_htons(ETH_P_IPV6):
>> +             if (!pskb_may_pull(skb, sizeof(*ip6)))
>> +                     return -1;
>> +
>> +             ip6 = (struct ipv6hdr *) skb->data;
>> +             ip_proto = ip6->nexthdr;
>> +             addr1 = ip6->saddr.s6_addr32[3];
>> +             addr2 = ip6->daddr.s6_addr32[3];
>
> Why only [3] ? Is this future proof?
>
No.  But it's same as inet6_ehashfn :-)

>> +     for_each_cpu_mask_nr(cpu, __get_cpu_var(rps_remote_softirq_cpus)) {
>> +             struct softnet_data *queue = &per_cpu(softnet_data, cpu);
>> +             __smp_call_function_single(cpu, &queue->csd, 0);
>
> How do you get around the standard deadlocks with IPI called from
> irq disabled section?
>

What are the standard deadlocks?  Looks like __send_remote_softirq
will call __smp_call_function with irq's disabled...

> And why are the interrupts are disabled here anyways?
>

Protects rps_remote_softirq_cpus.


> It's a standard pet peeve of me, but it's quite unlikely you'll
> get any useful entropy at this time of kernel startup.
>
> Normally it's always the same.
>
Would it make sense to just use skb_tx_hashrnd for the receive hash
key also (renaming it to be more general)?


>> +     if (err)
>> +             return err;
>> +
>> +     rtnl_lock();
>
> It seems weird to do user parsing while holding that lock.
> Better first set up and allocate and then finally initialize global state.

Yes.  We could build a new map each time and then insert it into the
device structure using an rcu lock (which I hope would be sufficient
locking)

Thanks,
Tom
--
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