netdev - Re: [PATCH RFC] act_cpu: packet distributing

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]

Date:	Wed, 14 Jul 2010 12:17:57 +0800
From:	Changli Gao <xiaosuo@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Jamal Hadi Salim <hadi@...erus.ca>,
	"David S. Miller" <davem@...emloft.net>,
	Patrick McHardy <kaber@...sh.net>,
	Tom Herbert <therbert@...gle.com>, netdev@...r.kernel.org
Subject: Re: [PATCH RFC] act_cpu: packet distributing

On Wed, Jul 14, 2010 at 11:41 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> Le mercredi 14 juillet 2010 à 11:17 +0800, Changli Gao a écrit :
>> I want to know if I can assign the sk to skb as nf_tproxy_core does to avoid
>> the duplicate search later. Thanks.
>>
>
> I suggest we first correct bugs before adding new features.
>
> For me, tproxy is a high suspect at this moment, I would not use it on
> production machine.
>
>> act_cpu: packet distributing
>>
>> This TC action can be used to redirect packets to the CPU:
>>  * specified by the cpuid option
>>  * specified by the class minor ID obtained previously
>>  * on which the corresponding application runs
>>
>> It supports the similar functions of RPS and RFS, but is more flexible.
>>
>
> Thins kind of claims is disgusting.
>
> No documentation,  I have no idea how you setup the thing.

for example:

redirect packets to the CPU on which the corresponding application runs

tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff:0 protocol ip basic action cpu socket

redirect packets to special CPU.

tc filter add dev eth0 parent ffff:0 protocol ip basic action cpu cpuid 1

sport hash packet distributing among CPU 0-1.

tc filter add dev eth0 parent ffff:0 protocol ip handle 1 flow hash
keys proto-src divisor 2 action cpu map 1

>> +static inline void sock_save_cpu(struct sock *sk)
>> +{
>> +#if defined(CONFIG_NET_ACT_CPU) || defined(CONFIG_NET_ACT_CPU_MODULE)
>> +     int cpu = get_cpu();
>> +     if (sk->sk_cpu != cpu)
>> +             sk->sk_cpu = cpu;
>> +     put_cpu();
>
> sk->sk_cpu = raw_smp_processor_id();

Thanks.

>
> Why doing the search again, in case skb->sk already set by another
> module before you, like tproxy ?
>

Although it is unlikely skb->sk is non null, as tc is before
netfilter, I will handle this case. Thanks.

>> +static int tcf_cpu(struct sk_buff *skb, struct tc_action *a,
>> +                struct tcf_result *res)
>> +{
>> +     struct tcf_cpu *p = a->priv;
>> +     u32 type;
>> +     u32 value;
>> +     int cpu, action;
>> +     struct sk_buff *nskb;
>> +     unsigned int qtail;
>> +
>> +     spin_lock(&p->tcf_lock);
>
> Ok, the big lock...
>
> We have a lockless TCP/UDP input path, and this modules adds a lock
> again.
>
>> +     p->tcf_tm.lastuse = jiffies;
>> +     p->tcf_bstats.bytes += qdisc_pkt_len(skb);
>> +     p->tcf_bstats.packets++;
>> +     type = p->type;
>> +     value = p->value;
>> +     action = p->tcf_action;
>> +     spin_unlock(&p->tcf_lock);
>> +
>
> Please change all this crap  (legacy crap, copied from other tc
> modules), by modern one, using RCU and no locking in hot path.
>

Thanks, I'll try. It is a write critical section, and for me it is
difficult to convert this lock to RCU. Could you show me some
examples?
 Thanks.

-- 
Regards,
Changli Gao(xiaosuo@...il.com)
--
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