[ovs-dev] [PATCH 2/3] datapath: Mega flow implementation

[Jesse Gross]

On Mon, Jun 10, 2013 at 9:10 AM, Andy Zhou <azhou at nicira.com> wrote:
> On Fri, Jun 7, 2013 at 4:43 PM, Jesse Gross <jesse at nicira.com> wrote:
>> On Wed, Jun 5, 2013 at 10:46 PM, Andy Zhou <azhou at nicira.com> wrote:
>> > +static void flow_key_mask(struct sw_flow_key *dst,
>> > +                         const struct sw_flow_key *src,
>> > +                         const struct sw_flow_mask *mfm)
>> > +{
>> > +       u8 *m = (u8 *)&mfm->key + mfm->desc.start;
>> > +       u8 *s = (u8 *)src + mfm->desc.start;
>> > +       u8 *d = (u8 *)dst + mfm->desc.start;
>>
>> You could improve the performance here by using longs instead of chars.
>
> What about CPU architectures requires longs to be aligned? What is the
> conventional #ifdef style here?

There is a 64 bit value in struct sw_flow_key, so the compiler will
enforce alignment for us.

>> > +static bool __flow_cmp_key_mask(struct sw_flow *flow, u32 hash, u8
>> > *key,
>> > +                               int key_start, int key_len,
>> > +                               struct sw_flow_mask *mfm)
>> > +{
>> > +       return (flow->hash == hash && (flow->mfm == mfm) &&
>> > +               !memcmp((u8 *)&flow->key + key_start, key, (key_len -
>> > key_start)));
>> > +}
>>
>> Since the flows are non-overlapping, do we need to compare the masks?
>
> Yes, we are comparing masked key, which is no longer unique without
> comparing the mask.

Well, more to the point, I think we want to make sure that they are
unique. The ordering of the masks has no inherent meaning so in order
to avoid surprising behavior I think we need to enforce that there is
no possibility of overlap. This should happen naturally when we start
looking up exact matches on flow setup.