On Wed, Sep 5, 2012 at 10:17 AM, Kyle Mestery (kmestery) <kmest...@cisco.com> wrote: > On Sep 4, 2012, at 8:40 PM, Jesse Gross wrote: >> On Tue, Sep 4, 2012 at 12:24 PM, Kyle Mestery (kmestery) >> <kmest...@cisco.com> wrote: >>> On Sep 3, 2012, at 3:43 PM, Jesse Gross wrote: >>>> On Wed, Aug 29, 2012 at 7:00 AM, Kyle Mestery <kmest...@cisco.com> wrote: >>>>> diff --git a/datapath/actions.c b/datapath/actions.c >>>>> index 208f260..83382b9 100644 >>>>> --- a/datapath/actions.c >>>>> +++ b/datapath/actions.c >>>>> @@ -343,7 +343,11 @@ static int execute_set_action(struct sk_buff *skb, >>>>> break; >>>>> >>>>> case OVS_KEY_ATTR_TUN_ID: >>>>> - OVS_CB(skb)->tun_id = nla_get_be64(nested_attr); >>>>> + OVS_CB(skb)->tun_key->tun_id = nla_get_be64(nested_attr); >>>> >>>> I think this doesn't quite work out so simply as a compatibility layer >>>> because tun_key is either NULL or pointing directly into the actions >>>> of the flow, which are supposed to be immutable. Long term this won't >>>> be a problem but in the meantime we'll have a figure out a way to fake >>>> up an appropriate tun_key. >>>> >>> So, I think what we should do here is allocate OVS_CB(skb)->tun_key in >>> this case. We could modify struct ovs_key_ipv4_tunnel to have a flag value >>> indicating if it was allocated or not, to allow for correct handling at the >>> time >>> of termination. Thoughts? >> >> I think we probably could keep one on the stack when we execute >> actions. That would avoid the need to track allocation and freeing >> later. >> > Actually, after thinking about this, I'm unclear as to how to proceed here. > Don't we > need to ensure the tun_id is passed with the skbuff when outputting the > packet? > If so, how does just keeping it on the stack help here, since all the output > code is > now changed to grab it from the OVS_SB(skbuff) pointer?
All I meant was rather than allocating a new struct ovs_key_ipv4_tunnel dynamically we can have one on the stack in do_execute_actions(). If we have a set tun_id action then we can write into that one and point the OVS_CB(skb)->tun_key at it. It should be pretty much the same as what you mentioned before but being on the stack is faster and doesn't require freeing it later. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
