>>> - skb = skb_clone(skb, GFP_ATOMIC); >>> - if (!skb) >>> - /* Skip the sample action when out of memory. */ >>> - return 0; >>> + if (key) { >>> + err = do_execute_actions(dp, skb, key, actions, rem); >>> + } else if (!add_deferred_actions(skb, orig, actions, rem)) { >>> >> We can refactor this code to avoid duplicate code all actions >> implementation. This way there could be single function dealing with >> both defered action and key fifo arrays. > > O.K. I will make the change in the next version.
After looking more at it, the sample action and recirc action are different enough that I don't see clean ways refactor them. For example, recirc needs to deal with setting recirc_id. recirc calls ovs_dp_process_packet, and sample calls do_execute_action. >> I am not sure if we can put sample or recirc in "may not change flow" >> actions list. Consider following set of actions: >> sample(sample(set-IP)),userpsace(),... >> In this case the userspace action could recieve skb with inconsistent >> flow due to preceding of set action nested in the sample action. > > Good catch. Will fix. What's the objection on recirc action? It always works on clone key.