netdev - Re: [net-next PATCH 2/2] net: sched: cls_u32 changes to knode must appear atomic to readers

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <541D08C6.8060804@gmail.com>
Date:	Fri, 19 Sep 2014 21:55:34 -0700
From:	John Fastabend <john.fastabend@...il.com>
To:	Jamal Hadi Salim <jhs@...atatu.com>
CC:	xiyou.wangcong@...il.com, davem@...emloft.net,
	eric.dumazet@...il.com, netdev@...r.kernel.org
Subject: Re: [net-next PATCH 2/2] net: sched: cls_u32 changes to knode must
 appear atomic to readers

On 09/18/2014 04:38 AM, Jamal Hadi Salim wrote:
> On 09/17/14 15:12, John Fastabend wrote:
>> Changes to the cls_u32 classifier must appear atomic to the
>> readers. Before this patch if a change is requested for both
>> the exts and ifindex, first the ifindex is updated then the
>> exts with tcf_exts_change(). This opens a small window where
>> a reader can have a exts chain with an incorrect ifindex. This
>> violates the the RCU semantics.
>>
>> Here we resolve this by always passing u32_set_parms() a copy
>> of the tc_u_knode to work on and then inserting it into the hash
>> table after the updates have been successfully applied.
>>
>> Tested with the following short script:
>>
>
>>
>> #tc filter add dev p3p2 parent 8001:0 protocol ip prio 99 handle 1: \
>>            u32 divisor 256
>>
>> #tc filter add dev p3p2 parent 8001:0 protocol ip prio 99 \
>>            u32 link 1: hashkey mask ffffff00 at 12    \
>>            match ip src 192.168.8.0/2
>>
>> #tc filter add dev p3p2 parent 8001:0 protocol ip prio 102    \
>>            handle 1::10 u32 classid 1:2 ht 1:           \
>>            match ip src 192.168.8.0/8 match ip tos 0x0a 1e
>>
>> #tc filter change dev p3p2 parent 8001:0 protocol ip prio 102 \
>>          handle 1::10 u32 classid 1:2 ht 1:        \
>>          match ip src 1.1.0.0/8 match ip tos 0x0b 1e
>>
>> CC: Eric Dumazet <edumazet@...gle.com>
>> CC: Jamal Hadi Salim <jhs@...atatu.com>
>> Signed-off-by: John Fastabend <john.r.fastabend@...el.com>
>
>
> Looks good to me.
> Acked-by: Jamal Hadi Salim <jhs@...atatu.com>
>
> cheers,
> jamal

Thanks for looking it over! I made v2 though to address a comment
that my variable/function names could be better and added a comment
around the perhaps tricky cases where it is safe to free the percpu
variables.

Because I did touch the patch and make some changes I dropped your
ACK. I always thought it was a bit of bad form to carry ack's around
after modifying the code without an explicit approval. Please add it
back though if you want.

Thanks,
John

-- 
John Fastabend         Intel Corporation
--
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