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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
