[ovs-dev] [PATCH] netdev_dpdk.c: Add QoS functionality.
Daniele Di Proietto
diproiettod at vmware.com
Tue Oct 13 16:14:27 UTC 2015
On 12/10/2015 18:02, "Stokes, Ian" <ian.stokes at intel.com> wrote:
>Thanks for providing feedback, answers inline.
>> -----Original Message-----
>> From: Daniele Di Proietto [mailto:diproiettod at vmware.com]
>> Sent: Friday, October 09, 2015 6:53 PM
>> To: Stokes, Ian
>> Cc: dev at openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH] netdev_dpdk.c: Add QoS functionality.
>> Thanks for the patch, the implementation looks simpler than I expected.
>> General questions:
>> * It appears that we're using the srCTM, but
>> * We're only sending the green packets and dropping the rest.
>> * The `input_color` is always green.
>> * `other-config:ebs` is ignored (isn't it?). I would rather not put in
>> database a value that is ignored.
>Although we do not use the ebs value it is a requirement for the
>rte_meter initialization function. DPDK expects an ebs value
>greater than 0 or it fails to initialize the rte_meter.
>We can remove the other_config:ebs from the command line but
>will have to add a define in netdev_dpdk so as to supply some ebs
>Would this be acceptable?
If there aren't plans to use different input/output colors, that
would be fine for me.
Since that parameter is useless, I would even avoid a define and always
>> * The `us_policer_table` seems redundant
>> * We just need two `us_policer_action`s, or perhaps just a boolean.
>> Can we just call this a token bucket? Are there any good reasons not
>> (Maybe you're planning to expand this, I'm not sure)
>Is this in relation to the name 'us-policer'or the name for the cbs and
>ebs paramaters? ?
The name is fine. My point is:
This patch implements a token bucket, using (internally) srTCM. Is it
to expose to the user the `ebs` parameter, or references to RFC2697, since
not using the colors at all?
>I think we should change the us-policer name anyhow to signify that it
>is egress. Would 'egress_policer' be acceptable? Failing that we ca use
>token_bucket can be used. Once it's clear to the end user.
>> * Out of curiosity, are there plans to support different output queues?
>There are plans to introduce user space queues at a later date to allow
>buffering of traffic. This would be included as maybe a rate limiting type
>implementation where traffic is not dropped but instead buffered. This
>will be a separate submission in the future however.
Sorry, I wasn't clear enough. By `different output queues` I meant having
different instances of a meter on the same port (that can be configured
the `Queue` table, and can be used via the `enqueue` OpenFlow action).
>> * I did't experience any performance degradation when QoS is not
>> but one can imagine that taking a spinlock might have some performance
>> when multiple threads insist on the same NIC. Have you considered
>> for the `qos_conf` pointer? (I'm not saying that we absolutely should
>I haven't tried an RCU for this. I can look at this but if it's not a
>blocking issue perhaps it could be implemented at a later stage?
Absolutely, we can implement that later, if we find a bottleneck.
More information about the dev