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

[Daniele Di Proietto]

2017-01-26 9:55 GMT-08:00 Loftus, Ciara :
>>
>> 2017-01-25 7:52 GMT-08:00 Loftus, Ciara :
>> >> 2017-01-22 11:45 GMT-08:00 Jan Scheurich :
>> >> >
>> >> >> 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?..
>> >
>> > +  > > +  type='{"type": "integer", "minInteger": 0, "maxInteger":
>> 4294967295}'>
>> > +
>> > +  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.
>> > +
>> > +
>> > +  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
>> >> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Loftus, Ciara]

> 
> 2017-01-25 7:52 GMT-08:00 Loftus, Ciara :
> >> 2017-01-22 11:45 GMT-08:00 Jan Scheurich :
> >> >
> >> >> 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?..
> >
> > +   > +  type='{"type": "integer", "minInteger": 0, "maxInteger":
> 4294967295}'>
> > +
> > +  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.
> > +
> > +
> > +  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!

Thanks,
Ciara

> 
> >
> > Thanks,
> > Ciara
> >
> >>
> >> Thanks,
> >>
> >> Daniele
> >>
> >> >
> >> > Jan
> >> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Daniele Di Proietto]

2017-01-22 11:45 GMT-08:00 Jan Scheurich :
>
>> 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.

>
> 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.

Thanks,

Daniele

>
> Jan
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Jan Scheurich]

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.


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?


Jan

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev