Thank you!
I have looked at the changes and I'm very thankful for your comments. Thank you
for pointing out nl_msg_put_u32 and nl_attr_get_u32, I somehow missed it. I
also forgot to check for trailing whitespace, sorry about that.
I hope that I again will have the opportunity to work on Open vSwitch, and
thanks again for all the help.
On 03/23/2015 09:56 PM, Ben Pfaff wrote:
> On Wed, Mar 18, 2015 at 05:13:01PM +0100, Jonathan Vestin wrote:
>> This patch adds support for SFQ, CoDel and FQ_CoDel classless qdiscs to Open
>> vSwitch. It also removes the requirement for a QoS to have at least one
>> Queue (as this makes no sense when using classless qdiscs). I have also not
>> implemented class_{get,set,delete,get_stats,dump_stats} because they are
>> meant for qdiscs with classes.
>>
>> Signed-off-by: Jonathan Vestin <jonav...@kau.se>
> Thanks for the contribution! I applied this to master. I folded in
> the following incremental changes. Most of them are purely stylistic
> (including a lot of removal of trailing white space) but you might
> want to notice the nl_msg_put_u32() and nl_attr_get_u32() functions
> for next time.
>
> Thanks again!
>
> diff --git a/AUTHORS b/AUTHORS
> index db4520f..8fba915 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -85,6 +85,7 @@ Jesse Gross je...@nicira.com
> Jing Ai ji...@google.com
> Joe Perches j...@perches.com
> Joe Stringer joestrin...@nicira.com
> +Jonathan Vestin jonav...@kau.se
> Jun Nakajima jun.nakaj...@intel.com
> Justin Pettit jpet...@nicira.com
> Keith Amidon ke...@nicira.com
> @@ -268,7 +269,6 @@ Joan Cirer j...@ev0.net
> John Darrington j...@darrington.wattle.id.au
> John Galgay j...@galgay.net
> John Hurley john.hur...@netronome.com
> -Jonathan Vestin jonav...@kau.se
> K 華 k940...@hotmail.com
> Kevin Mancuso kevin.manc...@rackspace.com
> Kiran Shanbhog ki...@vmware.com
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 65f2555..8253dfb 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2856,7 +2856,7 @@ codel_get__(const struct netdev *netdev_)
> }
>
> static void
> -codel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit,
> +codel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit,
> uint32_t interval)
> {
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> @@ -2872,7 +2872,7 @@ codel_install__(struct netdev *netdev_, uint32_t
> target, uint32_t limit,
> }
>
> static int
> -codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
> +codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
> uint32_t interval)
> {
> size_t opt_offset;
> @@ -2897,18 +2897,17 @@ codel_setup_qdisc__(struct netdev *netdev, uint32_t
> target, uint32_t limit,
>
> nl_msg_put_string(&request, TCA_KIND, "codel");
> opt_offset = nl_msg_start_nested(&request, TCA_OPTIONS);
> - nl_msg_put_unspec(&request, TCA_CODEL_TARGET, &otarget, sizeof otarget);
> - nl_msg_put_unspec(&request, TCA_CODEL_LIMIT, &olimit, sizeof olimit);
> - nl_msg_put_unspec(&request, TCA_CODEL_INTERVAL, &ointerval,
> - sizeof ointerval);
> + nl_msg_put_u32(&request, TCA_CODEL_TARGET, otarget);
> + nl_msg_put_u32(&request, TCA_CODEL_LIMIT, olimit);
> + nl_msg_put_u32(&request, TCA_CODEL_INTERVAL, ointerval);
> nl_msg_end_nested(&request, opt_offset);
>
> error = tc_transact(&request, NULL);
> if (error) {
> VLOG_WARN_RL(&rl, "failed to replace %s qdisc, "
> - "target %u, limit %u, interval %u error %d(%s)",
> + "target %u, limit %u, interval %u error %d(%s)",
> netdev_get_name(netdev),
> - otarget, olimit, ointerval,
> + otarget, olimit, ointerval,
> error, ovs_strerror(error));
> }
> return error;
> @@ -2930,9 +2929,15 @@ codel_parse_qdisc_details__(struct netdev *netdev
> OVS_UNUSED,
> codel->limit = limit_s ? strtoull(limit_s, NULL, 10) : 0;
> codel->interval = interval_s ? strtoull(interval_s, NULL, 10) : 0;
>
> - if (!codel->target) codel->target = 5000;
> - if (!codel->limit) codel->limit = 10240;
> - if (!codel->interval) codel->interval = 100000;
> + if (!codel->target) {
> + codel->target = 5000;
> + }
> + if (!codel->limit) {
> + codel->limit = 10240;
> + }
> + if (!codel->interval) {
> + codel->interval = 100000;
> + }
> }
>
> static int
> @@ -2942,7 +2947,7 @@ codel_tc_install(struct netdev *netdev, const struct
> smap *details)
> struct codel codel;
>
> codel_parse_qdisc_details__(netdev, details, &codel);
> - error = codel_setup_qdisc__(netdev, codel.target, codel.limit,
> + error = codel_setup_qdisc__(netdev, codel.target, codel.limit,
> codel.interval);
> if (!error) {
> codel_install__(netdev, codel.target, codel.limit, codel.interval);
> @@ -2967,9 +2972,9 @@ codel_parse_tca_options__(struct nlattr *nl_options,
> struct codel *codel)
> return EPROTO;
> }
>
> - codel->target = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_TARGET]));
> - codel->limit = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_LIMIT]));
> - codel->interval = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_INTERVAL]));
> + codel->target = nl_attr_get_u32(attrs[TCA_CODEL_TARGET]);
> + codel->limit = nl_attr_get_u32(attrs[TCA_CODEL_LIMIT]);
> + codel->interval = nl_attr_get_u32(attrs[TCA_CODEL_INTERVAL]);
> return 0;
> }
>
> @@ -3042,7 +3047,7 @@ static const struct tc_ops tc_ops_codel = {
> NULL,
> NULL
> };
> -
> +
> /* FQ-CoDel traffic control class. */
>
> #define FQCODEL_N_QUEUES 0x0000
> @@ -3064,7 +3069,7 @@ fqcodel_get__(const struct netdev *netdev_)
> }
>
> static void
> -fqcodel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit,
> +fqcodel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit,
> uint32_t interval, uint32_t flows, uint32_t quantum)
> {
> struct netdev_linux *netdev = netdev_linux_cast(netdev_);
> @@ -3082,7 +3087,7 @@ fqcodel_install__(struct netdev *netdev_, uint32_t
> target, uint32_t limit,
> }
>
> static int
> -fqcodel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t
> limit,
> +fqcodel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
> uint32_t interval, uint32_t flows, uint32_t quantum)
> {
> size_t opt_offset;
> @@ -3105,26 +3110,24 @@ fqcodel_setup_qdisc__(struct netdev *netdev, uint32_t
> target, uint32_t limit,
> olimit = limit ? limit : 10240;
> ointerval = interval ? interval : 100000;
> oflows = flows ? flows : 1024;
> - oquantum = quantum ? quantum : 1514; /* fq_codel default quantum is 1514
> + oquantum = quantum ? quantum : 1514; /* fq_codel default quantum is 1514
> not mtu */
>
> nl_msg_put_string(&request, TCA_KIND, "fq_codel");
> opt_offset = nl_msg_start_nested(&request, TCA_OPTIONS);
> - nl_msg_put_unspec(&request, TCA_FQ_CODEL_TARGET, &otarget, sizeof
> otarget);
> - nl_msg_put_unspec(&request, TCA_FQ_CODEL_LIMIT, &olimit, sizeof olimit);
> - nl_msg_put_unspec(&request, TCA_FQ_CODEL_INTERVAL, &ointerval,
> - sizeof ointerval);
> - nl_msg_put_unspec(&request, TCA_FQ_CODEL_FLOWS, &oflows, sizeof oflows);
> - nl_msg_put_unspec(&request, TCA_FQ_CODEL_QUANTUM, &oquantum,
> - sizeof oquantum);
> + nl_msg_put_u32(&request, TCA_FQ_CODEL_TARGET, otarget);
> + nl_msg_put_u32(&request, TCA_FQ_CODEL_LIMIT, olimit);
> + nl_msg_put_u32(&request, TCA_FQ_CODEL_INTERVAL, ointerval);
> + nl_msg_put_u32(&request, TCA_FQ_CODEL_FLOWS, oflows);
> + nl_msg_put_u32(&request, TCA_FQ_CODEL_QUANTUM, oquantum);
> nl_msg_end_nested(&request, opt_offset);
>
> error = tc_transact(&request, NULL);
> if (error) {
> VLOG_WARN_RL(&rl, "failed to replace %s qdisc, "
> - "target %u, limit %u, interval %u, flows %u, quantum %u error
> %d(%s)",
> + "target %u, limit %u, interval %u, flows %u, quantum %u error
> %d(%s)",
> netdev_get_name(netdev),
> - otarget, olimit, ointerval, oflows, oquantum,
> + otarget, olimit, ointerval, oflows, oquantum,
> error, ovs_strerror(error));
> }
> return error;
> @@ -3150,11 +3153,21 @@ fqcodel_parse_qdisc_details__(struct netdev *netdev
> OVS_UNUSED,
> fqcodel->interval = interval_s ? strtoull(interval_s, NULL, 10) : 0;
> fqcodel->flows = flows_s ? strtoull(flows_s, NULL, 10) : 0;
> fqcodel->quantum = quantum_s ? strtoull(quantum_s, NULL, 10) : 0;
> - if (!fqcodel->target) fqcodel->target = 5000;
> - if (!fqcodel->limit) fqcodel->limit = 10240;
> - if (!fqcodel->interval) fqcodel->interval = 1000000;
> - if (!fqcodel->flows) fqcodel->flows = 1024;
> - if (!fqcodel->quantum) fqcodel->quantum = 1514;
> + if (!fqcodel->target) {
> + fqcodel->target = 5000;
> + }
> + if (!fqcodel->limit) {
> + fqcodel->limit = 10240;
> + }
> + if (!fqcodel->interval) {
> + fqcodel->interval = 1000000;
> + }
> + if (!fqcodel->flows) {
> + fqcodel->flows = 1024;
> + }
> + if (!fqcodel->quantum) {
> + fqcodel->quantum = 1514;
> + }
> }
>
> static int
> @@ -3164,11 +3177,11 @@ fqcodel_tc_install(struct netdev *netdev, const
> struct smap *details)
> struct fqcodel fqcodel;
>
> fqcodel_parse_qdisc_details__(netdev, details, &fqcodel);
> - error = fqcodel_setup_qdisc__(netdev, fqcodel.target, fqcodel.limit,
> - fqcodel.interval, fqcodel.flows,
> + error = fqcodel_setup_qdisc__(netdev, fqcodel.target, fqcodel.limit,
> + fqcodel.interval, fqcodel.flows,
> fqcodel.quantum);
> if (!error) {
> - fqcodel_install__(netdev, fqcodel.target, fqcodel.limit,
> + fqcodel_install__(netdev, fqcodel.target, fqcodel.limit,
> fqcodel.interval, fqcodel.flows, fqcodel.quantum);
> }
> return error;
> @@ -3193,11 +3206,11 @@ fqcodel_parse_tca_options__(struct nlattr
> *nl_options, struct fqcodel *fqcodel)
> return EPROTO;
> }
>
> - fqcodel->target = *((uint32_t *)nl_attr_get(attrs[TCA_FQ_CODEL_TARGET]));
> - fqcodel->limit = *((uint32_t *)nl_attr_get(attrs[TCA_FQ_CODEL_LIMIT]));
> - fqcodel->interval =*((uint32_t
> *)nl_attr_get(attrs[TCA_FQ_CODEL_INTERVAL]));
> - fqcodel->flows = *((uint32_t *)nl_attr_get(attrs[TCA_FQ_CODEL_FLOWS]));
> - fqcodel->quantum = *((uint32_t
> *)nl_attr_get(attrs[TCA_FQ_CODEL_QUANTUM]));
> + fqcodel->target = nl_attr_get_u32(attrs[TCA_FQ_CODEL_TARGET]);
> + fqcodel->limit = nl_attr_get_u32(attrs[TCA_FQ_CODEL_LIMIT]);
> + fqcodel->interval =nl_attr_get_u32(attrs[TCA_FQ_CODEL_INTERVAL]);
> + fqcodel->flows = nl_attr_get_u32(attrs[TCA_FQ_CODEL_FLOWS]);
> + fqcodel->quantum = nl_attr_get_u32(attrs[TCA_FQ_CODEL_QUANTUM]);
> return 0;
> }
>
> @@ -3219,7 +3232,7 @@ fqcodel_tc_load(struct netdev *netdev, struct ofpbuf
> *nlmsg)
> return error;
> }
>
> - fqcodel_install__(netdev, fqcodel.target, fqcodel.limit,
> fqcodel.interval,
> + fqcodel_install__(netdev, fqcodel.target, fqcodel.limit,
> fqcodel.interval,
> fqcodel.flows, fqcodel.quantum);
> return 0;
> }
> @@ -3250,7 +3263,7 @@ fqcodel_qdisc_set(struct netdev *netdev, const struct
> smap *details)
> struct fqcodel fqcodel;
>
> fqcodel_parse_qdisc_details__(netdev, details, &fqcodel);
> - fqcodel_install__(netdev, fqcodel.target, fqcodel.limit,
> fqcodel.interval,
> + fqcodel_install__(netdev, fqcodel.target, fqcodel.limit,
> fqcodel.interval,
> fqcodel.flows, fqcodel.quantum);
> fqcodel_get__(netdev)->target = fqcodel.target;
> fqcodel_get__(netdev)->limit = fqcodel.limit;
> @@ -3275,7 +3288,7 @@ static const struct tc_ops tc_ops_fqcodel = {
> NULL,
> NULL
> };
> -
> +
> /* SFQ traffic control class. */
>
> #define SFQ_N_QUEUES 0x0000
> @@ -3329,17 +3342,16 @@ sfq_setup_qdisc__(struct netdev *netdev, uint32_t
> quantum, uint32_t perturb)
>
> memset(&opt, 0, sizeof opt);
> if (!quantum) {
> - if (!mtu_error)
> + if (!mtu_error) {
> opt.quantum = mtu; /* if we cannot find mtu, use default */
> - }
> - else {
> + }
> + } else {
> opt.quantum = quantum;
> }
>
> if (!perturb) {
> opt.perturb_period = 10;
> - }
> - else {
> + } else {
> opt.perturb_period = perturb;
> }
>
> @@ -3349,10 +3361,10 @@ sfq_setup_qdisc__(struct netdev *netdev, uint32_t
> quantum, uint32_t perturb)
> error = tc_transact(&request, NULL);
> if (error) {
> VLOG_WARN_RL(&rl, "failed to replace %s qdisc, "
> - "quantum %u, perturb %u error %d(%s)",
> - netdev_get_name(netdev),
> - opt.quantum, opt.perturb_period,
> - error, ovs_strerror(error));
> + "quantum %u, perturb %u error %d(%s)",
> + netdev_get_name(netdev),
> + opt.quantum, opt.perturb_period,
> + error, ovs_strerror(error));
> }
> return error;
> }
> @@ -3376,11 +3388,11 @@ sfq_parse_qdisc_details__(struct netdev *netdev,
>
> if (!sfq->quantum) {
> mtu_error = netdev_linux_get_mtu__(netdev_linux_cast(netdev), &mtu);
> - if(!mtu_error) {
> + if (!mtu_error) {
> sfq->quantum = mtu;
> } else {
> - VLOG_WARN_RL(&rl, "when using SFQ, you must specify quantum on a
> "
> - "device without mtu");
> + VLOG_WARN_RL(&rl, "when using SFQ, you must specify quantum on a
> "
> + "device without mtu");
> return;
> }
> }
> @@ -3462,7 +3474,7 @@ static const struct tc_ops tc_ops_sfq = {
> NULL,
> NULL
> };
> -
> +
> /* HTB traffic control class. */
>
> #define HTB_N_QUEUES 0xf000
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 466e7c0..07f3bea 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -3152,19 +3152,28 @@
> <dl>
> <dt><code>linux-sfq</code></dt>
> <dd>
> - Linux "Stochastic Fairness Queueing" classifier. See tc-sfq(8)
> (also at <code>http://linux.die.net/man/8/tc-sfq</code> ) for information on
> how this classifier works.
> + Linux ``Stochastic Fairness Queueing'' classifier. See
> + <code>tc-sfq</code>(8) (also at
> + <code>http://linux.die.net/man/8/tc-sfq</code>) for information on
> + how this classifier works.
> </dd>
> </dl>
> <dl>
> <dt><code>linux-codel</code></dt>
> <dd>
> - Linux "Controlled Delay" classifier. See tc-codel(8) (also at
> <code>http://man7.org/linux/man-pages/man8/tc-codel.8.html</code> ) for
> information on how this classifier works.
> + Linux ``Controlled Delay'' classifier. See <code>tc-codel</code>(8)
> + (also at
> + <code>http://man7.org/linux/man-pages/man8/tc-codel.8.html</code>)
> + for information on how this classifier works.
> </dd>
> </dl>
> <dl>
> <dt><code>linux-fq_codel</code></dt>
> <dd>
> - Linux "Fair Queuing with Controlled Delay" classifier. See
> tc-fq_codel(8) (also at
> <code>http://man7.org/linux/man-pages/man8/tc-fq_codel.8.html</code> ) for
> information on how this classifier works.
> + Linux ``Fair Queuing with Controlled Delay'' classifier. See
> + <code>tc-fq_codel</code>(8) (also at
> +
> <code>http://man7.org/linux/man-pages/man8/tc-fq_codel.8.html</code>)
> + for information on how this classifier works.
> </dd>
> </dl>
> </column>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
