On Wed, May 27, 2015 at 10:57 PM, Pravin Shelar <pshe...@nicira.com> wrote: > On Wed, May 27, 2015 at 9:16 PM, Jesse Gross <je...@nicira.com> wrote: >> On Wed, May 27, 2015 at 7:46 PM, Pravin Shelar <pshe...@nicira.com> wrote: >>> On Wed, May 27, 2015 at 2:10 PM, Jesse Gross <je...@nicira.com> wrote: >>>> On Fri, May 22, 2015 at 10:53 AM, Pravin Shelar <pshe...@nicira.com> wrote: >>>>> On Wed, May 20, 2015 at 12:32 PM, Neil McKee <neil.mc...@inmon.com> wrote: >>>>>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c >>>>>> index b491c1c..ee5760d 100644 >>>>>> --- a/net/openvswitch/actions.c >>>>>> +++ b/net/openvswitch/actions.c >>>>>> @@ -608,7 +608,8 @@ static void do_output(struct datapath *dp, struct >>>>>> sk_buff *skb, int out_port) >>>>>> } >>>>>> >>>>>> static int output_userspace(struct datapath *dp, struct sk_buff *skb, >>>>>> - struct sw_flow_key *key, const struct nlattr >>>>>> *attr) >>>>>> + struct sw_flow_key *key, const struct nlattr >>>>>> *attr, >>>>>> + const struct nlattr *actions, int >>>>>> actions_len) >>>>>> { >>>>>> struct ovs_tunnel_info info; >>>>>> struct dp_upcall_info upcall; >>>>>> @@ -619,6 +620,8 @@ static int output_userspace(struct datapath *dp, >>>>>> struct sk_buff *skb, >>>>>> upcall.userdata = NULL; >>>>>> upcall.portid = 0; >>>>>> upcall.egress_tun_info = NULL; >>>>>> + upcall.actions = actions; >>>>>> + upcall.actions_len = actions_len; >>>>>> >>>>> Rather than unconditionally passing actions to the upcall, there >>>>> should be attribute in ovs_userspace_attr to request the actions list. >>>> >>>> Why? It seems simpler to just always pass the actions and I'm not sure >>>> that this is really performance critical (which is the only reason >>>> that comes to mind to not always pass this). >>> >>> This is only required for sFlow sampling so I do not think we should >>> send it on every upcall. >> >> But what is the downside? > > This increases memory allocation in atomic context but if you think > this makes code complicated then I am fine without the attribute.
OK, I see. My guess is that there are only likely to be a significant set of actions for sampling use cases anyways so if this is a real problem then a flag is probably not going to make much of a difference. One possibility is to retry with a smaller size if allocation fails and not include the actions in that case. Userspace is already going to have to handle the case where actions are omitted for existing kernels. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html