[ovs-dev] [tunnel flags: 1/1] fix out key action flag setting

Jesse Gross jesse at nicira.com
Wed Mar 2 02:31:42 UTC 2011


On Tue, Mar 1, 2011 at 5:09 PM, Valient Gough <valient at gmail.com> wrote:
> On Tue, Mar 1, 2011 at 4:36 PM, Jesse Gross <jesse at nicira.com> wrote:
>> On Tue, Mar 1, 2011 at 3:39 PM, Valient Gough <valient at gmail.com> wrote:
>>>
>>> While adding support for a tunnel with optional tunnel key, I've found
>>> problems handling the key configuration.  In existing tunnel code, such as
>>> GRE, the tunnel determines a key should be added to the packet if either
>>> TNL_F_OUT_KEY_ACTION is set, or else out_key is provided.  The logic below
>>> in tunnel.c will set the ACTION flag if NO key is provided, which means a
>>> tunnel with key support will always attempt to insert a key.  Patch below.
>>
>> The two methods of operation are supposed to be mutually exclusive.
>> Either the port has a specific key associated with it that is
>> implicitly used or one is provided by a flow action.  That's why the
>> flag is set if no key is provided.  In general key 0 is treated as
>> equivalent to key not present, so for GRE we don't include it at all
>> if the key is permanently zero.  If it is a flow based key then we
>> always include it in the GRE header for consistency.
>>
>> Out of curiosity, what protocol are you adding support for?
>>
>
>
> I'm confused about the purpose of this flag.  What does it mean to set
> TNL_F_OUT_KEY_ACTION but not have a key?  How could I configure GRE
> without setting a key and without setting TNL_F_OUT_KEY_ACTION?

Setting TNL_F_OUT_KEY_ACTION means that the key will be provided on a
per-flow basis using a datapath action instead of being statically
associated with the port.

If you don't want any key present in the GRE header, you can pass a key of 0.

>
> In vport-gre, gre_build_header, the logic is:
>
>        if (mutable->out_key || mutable->flags & TNL_F_OUT_KEY_ACTION)
>                greh->flags |= GRE_KEY;
>
>        if (mutable->out_key)
>                *options = be64_get_low32(mutable->out_key);
>
> The packet parsing code just looks for GRE_KEY to be set.  So that
> seems confusing to indicate in the packet that there is a key, but not
> write one.

The following function gre_update_header() gets called to perform
per-packet modifications to the header and writes the key:

        if (mutable->flags & TNL_F_OUT_KEY_ACTION) {
                *options = be64_get_low32(OVS_CB(skb)->tun_id);
                options--;
        }




More information about the dev mailing list