On Fri, Jun 17, 2016 at 2:35 PM, Cong Wang <xiyou.wangc...@gmail.com> wrote: > On Fri, Jun 17, 2016 at 2:24 PM, Eric Dumazet <eduma...@google.com> wrote: >> Well, I added a READ_ONCE() to read tcf_action once. >> >> Adding rcu here would mean adding a pointer and extra cache line, to >> deref the values. >> >> IMHO the race here has no effect . You either read the old or new value. > > Sure, the point is we may read a new ->tcf_action and an old ->tcfm_eaction, > this is what I am worrying. > > If that is not a good example, what about new ->tcf_action and ->tcfm_eaction, > with an old ->tcfm_ifindex? > >> >> If the packet is processed before or after the 'change' it would have >> the same 'race' >> > > Why? As long as the change is like a transaction, we are safe. > >> All these fields are integers, they never are 'partially written'. >> >> The only case m->tcfm_eaction could be read twice is in the error >> path. Who cares ? > > This is not what I worry about. I guess you miss read eaction with action.
No I did not. I am referring to the fact that we currently might read m->tcfm_eaction multiple times. Please explain what would be wrong reading a wrong pair of values ? One packet might come to a wrong device in the unlikely case an admin change all the fields during an update ? Is it going to crash or reveal highly sensitive security data ? If yes, then please send a patch. I considered all this when writing my patch and maybe I was wrong.
