On Nov 6, 2012, at 8:45 PM, Jesse Gross <[email protected]> wrote:

> On Tue, Nov 6, 2012 at 6:39 PM, Kyle Mestery (kmestery) <[email protected]> 
> wrote:
> 
> On Nov 6, 2012, at 6:50 PM, Jesse Gross <[email protected]> wrote:
> 
> > Tunnel ports now always include full outer IP information, even if
> > userspace can't understand it.  Since our flows our exact match this
> > information must also be provided when setting up flows.  Since flows
> > with only OVS_KEY_ATTR_TUN_ID keys don't contain all of this information
> > they can never be hit and we should just reject them at setup time.
> >
> > Signed-off-by: Jesse Gross <[email protected]>
> > ---
> > datapath/flow.c |   13 ++++++-------
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/datapath/flow.c b/datapath/flow.c
> > index 2acdd05..f33760a 100644
> > --- a/datapath/flow.c
> > +++ b/datapath/flow.c
> > @@ -1041,14 +1041,11 @@ int ovs_flow_from_nlattrs(struct sw_flow_key 
> > *swkey, int *key_lenp,
> >               if (tun_id != tun_key->tun_id)
> >                       return -EINVAL;
> >
> > -             memcpy(&swkey->phy.tun.tun_key, tun_key, 
> > sizeof(swkey->phy.tun.tun_key));
> > -             attrs &= ~(1ULL << OVS_KEY_ATTR_TUN_ID);
> > -             attrs &= ~(1ULL << OVS_KEY_ATTR_IPV4_TUNNEL);
> > -     } else if (attrs & (1ULL << OVS_KEY_ATTR_TUN_ID)) {
> > -             swkey->phy.tun.tun_key.tun_id = 
> > nla_get_be64(a[OVS_KEY_ATTR_TUN_ID]);
> > -             swkey->phy.tun.tun_key.tun_flags |= OVS_FLOW_TNL_F_KEY;
> > +             memcpy(&swkey->phy.tun.tun_key, tun_key,
> > +                     sizeof(swkey->phy.tun.tun_key));
> >
> >               attrs &= ~(1ULL << OVS_KEY_ATTR_TUN_ID);
> > +             attrs &= ~(1ULL << OVS_KEY_ATTR_IPV4_TUNNEL);
> >       } else if (attrs & (1ULL << OVS_KEY_ATTR_IPV4_TUNNEL)) {
> >               struct ovs_key_ipv4_tunnel *tun_key;
> >               tun_key = nla_data(a[OVS_KEY_ATTR_IPV4_TUNNEL]);
> > @@ -1056,7 +1053,9 @@ int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, 
> > int *key_lenp,
> >               if (!tun_key->ipv4_dst)
> >                       return -EINVAL;
> >
> > -             memcpy(&swkey->phy.tun.tun_key, tun_key, 
> > sizeof(swkey->phy.tun.tun_key));
> > +             memcpy(&swkey->phy.tun.tun_key, tun_key,
> > +                     sizeof(swkey->phy.tun.tun_key));
> > +
> 
> Why is this change needed?
> 
> Which part?  The whole thing or just the line wrap immediately above?
> 
> The idea of the patch is just to reject flows that can't possibly be correct 
> to catch problems early.  The line wrap is just cosmetic to fit on 80 
> characters - there shouldn't be any actual changes.

Yes, just the wrap, sorry about that. Looking back on it, the diff itself 
confused me, and I
didn't count out the 80 characters.

Acked-by: Kyle Mestery <[email protected]>

_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to