Good point. I will take a look and send out V2.
On Wed, Jul 3, 2013 at 4:23 PM, Jesse Gross <[email protected]> wrote: > On Wed, Jul 3, 2013 at 9:11 AM, Andy Zhou <[email protected]> wrote: > > Reported-by: Justin Pettit <[email protected]> > > Signed-off-by: Andy Zhou <[email protected]> > > --- > > 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. >
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
