On Tue, Nov 20, 2012 at 11:22 AM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Tue, Nov 20, 2012 at 11:17 AM, Jesse Gross <je...@nicira.com> wrote:
>>
>> On Tue, Nov 20, 2012 at 10:15 AM, Pravin B Shelar <pshe...@nicira.com>
>> wrote:
>> > Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
>> > ---
>> > lib/odp-util.c | 14 ++++++++++++++
>> > 1 file changed, 14 insertions(+)
>> >
>> > diff --git a/lib/odp-util.c b/lib/odp-util.c
>> > index 9b0876c..3884b4d 100644
>> > --- a/lib/odp-util.c
>> > +++ b/lib/odp-util.c
>> > @@ -1874,6 +1874,20 @@ odp_flow_key_to_flow(const struct nlattr *key,
>> > size_t key_len,
>>
>> Don't we need the corresponding code in odp_flow_key_from_flow()?
>> Otherwise, this will now return ODP_FIT_PERFECT but we won't actually
>> be able to regenerate the kernel's original flow. Is there any reason
>> not to add support for not just these two but as an action as well?
>>
> ok, I will fix odp_flow_key_from_flow().
>
>>
>> > + if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV4_TUNNEL)) {
>> > + struct ovs_key_ipv4_tunnel *ipv4_tun_key;
>> > +
>> > + ipv4_tun_key = nl_attr_get(attrs[OVS_KEY_ATTR_IPV4_TUNNEL]);
>> > +
>> > + flow->tunnel.tun_id = ipv4_tun_key->tun_id;
>> > + flow->tunnel.flags = ipv4_tun_key->tun_flags;
>>
>> This implicitly assumes that the flags are the same between userspace
>> and kernel. This is the case now but I'm not sure that we want to
>> make that assumption in the future. It can also cause us to pull in
>> additional information from the kernel that we don't actually
>> understand.
>
>
> We have used that assumption at other places, I will post separate patch to
> fix that.
Pravin and I talked offline and it looks like there aren't any other
places with this assumption. He's going to follow up with a patch to
convert tunnel information in all the places that userspace interacts
with the kernel.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
