[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