[ovs-dev] [PATCH 2/2] datapath: Hash only the part of struct sw_flow_key populated by flow_extract().

Andrew Evans aevans at nicira.com
Sat Apr 30 21:24:24 UTC 2011


On Fri, 2011-04-29 at 20:12 -0700, Jesse Gross wrote:
> On Fri, Apr 29, 2011 at 5:01 PM, Andrew Evans <aevans at nicira.com> wrote:
> >                case TRANSITION(ODP_KEY_ATTR_ETHERNET, ODP_KEY_ATTR_8021Q):
> >                        q_key = nla_data(nla);
> >                        /* Only standard 0x8100 VLANs currently supported. */
> >                        if (q_key->q_tpid != htons(ETH_P_8021Q))
> > -                               return -EINVAL;
> > +                               goto invalid;
> >                        if (q_key->q_tci & htons(VLAN_TAG_PRESENT))
> > -                               return -EINVAL;
> > +                               goto invalid;
> >                        swkey->dl_tci = q_key->q_tci | htons(VLAN_TAG_PRESENT);
> > +                       /* key_len already past dl_tci */
> 
> Should we just move dl_tci down below the MAC addresses?  I believe
> the alignment should still be correct.

I agree. Changed.

> >                case TRANSITION(ODP_KEY_ATTR_ETHERTYPE, ODP_KEY_ATTR_IPV4):
> >                        if (swkey->dl_type != htons(ETH_P_IP))
> > -                               return -EINVAL;
> > +                               goto invalid;
> >                        ipv4_key = nla_data(nla);
> >                        swkey->nw_proto = ipv4_key->ipv4_proto;
> >                        swkey->nw_tos = ipv4_key->ipv4_tos;
> >                        swkey->ipv4.src = ipv4_key->ipv4_src;
> >                        swkey->ipv4.dst = ipv4_key->ipv4_dst;
> > +                       key_len = SW_FLOW_KEY_OFFSET(ipv4.dst);
> 
> This can be just SW_FLOW_KEY_OFFSET(ipv4), right?

Won't that set key_len past the part of swkey that contains valid data
at this point? We haven't populated the TCP/UDP ports or ARP Ethernet
addresses yet, I think.

> >                case TRANSITION(ODP_KEY_ATTR_ETHERTYPE, ODP_KEY_ATTR_IPV6):
> >                        if (swkey->dl_type != htons(ETH_P_IPV6))
> > -                               return -EINVAL;
> > +                               goto invalid;
> >                        ipv6_key = nla_data(nla);
> >                        swkey->nw_proto = ipv6_key->ipv6_proto;
> >                        swkey->nw_tos = ipv6_key->ipv6_tos;
> > @@ -719,152 +726,155 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_len,
> >                                        sizeof(swkey->ipv6.src));
> >                        memcpy(&swkey->ipv6.dst, ipv6_key->ipv6_dst,
> >                                        sizeof(swkey->ipv6.dst));
> > +                       key_len = SW_FLOW_KEY_OFFSET(ipv6.dst);

Same as above?





More information about the dev mailing list