On Sep 5, 2012, at 1:03 PM, Jesse Gross wrote: > 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.
Got it, makes sense. Thanks! _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
