[ovs-dev] [PATCH 1/1] dpif-netdev: Conditional EMC insert

[Daniele Di Proietto]

2017-01-26 9:55 GMT-08:00 Loftus, Ciara <ciara.loftus at intel.com>:
>>
>> 2017-01-25 7:52 GMT-08:00 Loftus, Ciara <ciara.loftus at intel.com>:
>> >> 2017-01-22 11:45 GMT-08:00 Jan Scheurich <jan.scheurich at web.de>:
>> >> >
>> >> >> It's not a big deal, since the most important use case we have for
>> >> >> dpif-netdev is with dpdk, but I'd still like the code to behave
>> >> >> similarly on different platforms.  How about defining a function that
>> >> >> uses random_uint32 when compiling without DPDK?
>> >> >>
>> >> >> For testing it's not that simple, because unit tests can be run with
>> >> >> or without DPDK.  It would need to be configurable at runtime.
>> >> >> Perhaps making EM_FLOW_INSERT_PROB configurable at runtime
>> would
>> >> also
>> >> >> help people that want to experiment with different values, even
>> >> >> though, based on the comments, I guess they wouldn't really see
>> much
>> >> >> difference.
>> >> >>
>> >> >> Again, what do you think about simply using counting the packets and
>> >> >> inserting only 1 every EM_FLOW_INSERT_PROB?
>> >> >>
>> >> >> Thanks,
>> >> >>
>> >> >> Daniele
>> >> >
>> >> >
>> >> > As far as I know Ciara did some quick tests with a counter-based
>> >> > implementation and it performed 5% worse for 1K and 4K flows than
>> then
>> >> > current patch. Perhaps we could find the reason for that and fix it, but I
>> >> > also feel uncomfortable with deterministic insertion of every Nth flow.
>> This
>> >> > could lead to very strange lock-step phenomena with typical artificial
>> test
>> >> > work loads, which often generate flows round-robin. I would rather use
>> a
>> >> > random function, as you suggest, or count "cycles" differently when
>> >> > compiling without DPDK.
>> >>
>> >> Ok, using another pseudo random function when compiling without DPDK
>> >> sounds
>> >> good to me.
>> >>
>> >
>> > Any suggestions for the random function?
>>
>> I think we can use random_uint32() from lib/random.h
>>
>> >
>> >> >
>> >> > I agree to making the parameter EM_FLOW_INSERT_PROB configurable
>> for
>> >> unit
>> >> > test or other purposes. Should it be a new option in the OpenvSwitch
>> table
>> >> > in OVSDB or rather a run-time parameter to be changed with ovs-
>> appctl?
>> >>
>> >> I think a new option in Openvswitch other_config would be appropriate.
>> >
>> > I like this idea. I've started making these changes. How about something
>> like the following?..
>> >
>> > +      <column name="other_config" key="emc-insert-prob"
>> > +              type='{"type": "integer", "minInteger": 0, "maxInteger":
>> 4294967295}'>
>> > +        <p>
>> > +          Specifies the probability (1/emc-insert-prob) of a flow being
>> > +          inserted into the Exact Match Cache (EMC). Higher values of
>> > +          emc-insert-prob will result in less insertions, and lower
>> > +          values will result in more insertions. A value of zero will
>> > +          result in no insertions and essentially disable the EMC.
>> > +        </p>
>> > +        <p>
>> > +          Defaults to 100 ie. there is 1/100 chance of EMC insertion.
>>
>> Looks good to me, thanks.
>>
>> I would also add that this only applies to 'netdev' bridges (userspace) and
>> that
>> a value of 1 means that every flow is going to be sent to the EMC.
>
> Thanks Daniele. I've posted a v2. Not sure it's the ideal approach so would appreciate your feedback if you get a chance.
>
> On a separate note, I'm wondering would now be a good time to consider allowing the size of the EMC to be configurable ie. allow EM_FLOW_HASH_SHIFT or a similar value to be modifiable. Whatever scheme is followed for modifying insert probability could probably be easily be re-used for EMC sizing too. Just an idea!

By the way, I forgot to mention that I like this idea.  Hopefully it
shouldn't introduce any overhead.

Thanks,

Daniele

>
> Thanks,
> Ciara
>
>>
>> >
>> > Thanks,
>> > Ciara
>> >
>> >>
>> >> Thanks,
>> >>
>> >> Daniele
>> >>
>> >> >
>> >> > Jan
>> >> >