[ovs-dev] [PATCH] datapath: fix bugs in exporting tunnel mask attributes in netlink

Andy Zhou azhou at nicira.com
Thu Jul 4 00:39:59 UTC 2013


Good point. I will take a look and send out V2.


On Wed, Jul 3, 2013 at 4:23 PM, Jesse Gross <jesse at nicira.com> wrote:

> On Wed, Jul 3, 2013 at 9:11 AM, Andy Zhou <azhou at nicira.com> wrote:
> > Reported-by: Justin Pettit <jpettit at nicira.com>
> > Signed-off-by: Andy Zhou <azhou at nicira.com>
> > ---
> >  datapath/flow.c |    7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/datapath/flow.c b/datapath/flow.c
> > index 2ac36b6..c598641 100644
> > --- a/datapath/flow.c
> > +++ b/datapath/flow.c
> > @@ -1251,15 +1251,14 @@ int ipv4_tun_to_nlattr(struct sk_buff *skb,
> >                 return -EMSGSIZE;
> >         if (nla_put_be32(skb, OVS_TUNNEL_KEY_ATTR_IPV4_DST,
> output->ipv4_dst))
> >                 return -EMSGSIZE;
> > -       if (tun_key->ipv4_tos &&
> > -           nla_put_u8(skb, OVS_TUNNEL_KEY_ATTR_TOS, output->ipv4_tos))
> > +       if (nla_put_u8(skb, OVS_TUNNEL_KEY_ATTR_TOS, output->ipv4_tos))
> >                 return -EMSGSIZE;
> >         if (nla_put_u8(skb, OVS_TUNNEL_KEY_ATTR_TTL, output->ipv4_ttl))
> >                 return -EMSGSIZE;
> > -       if ((tun_key->tun_flags & TUNNEL_DONT_FRAGMENT) &&
> > +       if ((output->tun_flags & TUNNEL_DONT_FRAGMENT) &&
> >                 nla_put_flag(skb, OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT))
> >                 return -EMSGSIZE;
> > -       if ((tun_key->tun_flags & TUNNEL_CSUM) &&
> > +       if ((output->tun_flags & TUNNEL_CSUM) &&
> >                 nla_put_flag(skb, OVS_TUNNEL_KEY_ATTR_CSUM))
> >                 return -EMSGSIZE;
>
> I think this patch is correct but needs to go farther.
>
> For the masks, at least, we need to always output the attribute. For
> example, if the TUNNEL_KEY flag isn't set in the key but it is in the
> mask that means that there must not be a tunnel key. However, with the
> current logic we wouldn't output the mask, which means that we don't
> care.
>
> It's safe to output a key attribute that is zero, even if it isn't
> strictly necessary, so we can just error on the side of doing that to
> simplify the code. Really the only place where we should look at the
> key is when figuring out the prerequisites for later protocols. Can
> you look through the other serialization code? I think we have the
> same problem other places as well.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130703/62c03f87/attachment-0003.html>


More information about the dev mailing list