On Sat, Apr 19, 2014 at 4:10 AM, Jamal Hadi Salim <jhs@xxxxxxxxxxxx> wrote: > On 04/18/14 13:18, Cong Wang wrote: >> IOW, what's wrong with changing if (icmp) { A } to if (icmp) { B } ? >> where A and B could be any complex combination of actions. >> RTNL lock guarantees this is transactional. >> > > RTNL is one dimension. The other is the datapath processing. > You need to make sure that packets still flow correctly during the > change over. Sure, since we grab tcf_tree_lock() before changing actions in tcf_exts_change(), I think this is guaranteed too. > >> I never mean to only add or remove one of them inside, although >> my specific case is just for appending, my patch should allow to >> overwrite all the actions together. >> > > Well - then go nuts and put out a patch. > Replace _all or none_ is a reasonable approach. > > Great! We both agree on this. Looking at the current code, we first initialize a list of actions and then replace them as a whole by splicing the lists with tcf_tree_lock held, so this is already done. IOW, this patch is enough. Or am I missing anything? Thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html