Hello, I finally made my changes to OVS into a patch. I am a beginner in doing 
this kind of low-level programming, and it is my first patch to any open source 
project, so before submitting a patch, it would be nice with some code review.

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 not implemented 
tc_load it is never run on qdiscs without classes. I have also not implemented 
class_{get,set,delete,get_stats,dump_stats} because they are meant for qdiscs 
with classes.

The only problem now is that tc_load does not work unless the qdisc has at 
least one class, which is beyond my skill to fix.

Any advice would be appreciated. The code is tested with 'make', 'make check', 
'make testcheck' and manually.

Patch follows:
----

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 662ccc9..8eb9696 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -374,12 +374,18 @@ tc_destroy(struct tc *tc)
     hmap_destroy(&tc->queues);
 }
 
+static const struct tc_ops tc_ops_codel;
+static const struct tc_ops tc_ops_fqcodel;
+static const struct tc_ops tc_ops_sfq;
 static const struct tc_ops tc_ops_htb;
 static const struct tc_ops tc_ops_hfsc;
 static const struct tc_ops tc_ops_default;
 static const struct tc_ops tc_ops_other;
 
 static const struct tc_ops *const tcs[] = {
+    &tc_ops_codel,              /* Controlled delay */
+    &tc_ops_fqcodel,            /* Fair queue controlled delay */
+    &tc_ops_sfq,                /* Stochastic fair queueing */
     &tc_ops_htb,                /* Hierarchy token bucket (see tc-htb(8)). */
     &tc_ops_hfsc,               /* Hierarchical fair service curve. */
     &tc_ops_default,            /* Default qdisc (see tc-pfifo_fast(8)). */
@@ -2831,6 +2837,492 @@ const struct netdev_class netdev_internal_class =
         NULL,                  /* get_features */
         netdev_internal_get_status);
 
+#define CODEL_N_QUEUES 0x0000
+
+struct codel {
+    struct tc tc;
+    uint32_t target;
+    uint32_t limit;
+    uint32_t interval;
+};
+
+static struct codel *
+codel_get__(const struct netdev *netdev_)
+{
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+    return CONTAINER_OF(netdev->tc, struct codel, tc);
+}
+
+static void
+codel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit, 
uint32_t interval)
+{
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+    struct codel *codel;
+
+    codel = xmalloc(sizeof *codel);
+    tc_init(&codel->tc, &tc_ops_codel);
+    codel->target = target;
+    codel->limit = limit;
+    codel->interval = interval;
+
+    netdev->tc = &codel->tc;
+}
+
+static int
+codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit, 
uint32_t interval)
+{
+    size_t opt_offset;
+    struct ofpbuf request;
+    struct tcmsg *tcmsg;
+    uint32_t otarget, olimit, ointerval;
+    int error;
+
+    tc_del_qdisc(netdev);
+
+    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
+                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    if (!tcmsg) {
+        return ENODEV;
+    }
+    tcmsg->tcm_handle = tc_make_handle(1, 0);
+    tcmsg->tcm_parent = TC_H_ROOT;
+
+    otarget = target ? target : 5000;
+    olimit = limit ? limit : 10240;
+    ointerval = interval ? interval : 100000;
+
+    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_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)",
+        netdev_get_name(netdev),
+        otarget, olimit, ointerval,
+        error, ovs_strerror(error));
+    return error;
+}
+
+static void
+codel_parse_qdisc_details__(struct netdev *netdev OVS_UNUSED,
+                          const struct smap *details, struct codel *codel)
+{
+    const char *target_s;
+    const char *limit_s;
+    const char *interval_s;
+
+    target_s = smap_get(details, "target");
+    limit_s = smap_get(details, "limit");
+    interval_s = smap_get(details, "interval");
+    codel->target = target_s ? strtoull(target_s, NULL, 10) : 0;
+    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;
+}
+
+static int
+codel_tc_install(struct netdev *netdev, const struct smap *details)
+{
+    int error;
+    struct codel codel;
+
+    codel_parse_qdisc_details__(netdev, details, &codel);
+    error = codel_setup_qdisc__(netdev, codel.target, codel.limit, 
codel.interval);
+    if (!error) {
+        codel_install__(netdev, codel.target, codel.limit, codel.interval);
+    }
+    return error;
+}
+
+static void
+codel_tc_destroy(struct tc *tc)
+{
+    struct codel *codel = CONTAINER_OF(tc, struct codel, tc);
+    tc_destroy(tc);
+    free(codel);
+}
+
+static int
+codel_qdisc_get(const struct netdev *netdev, struct smap *details)
+{
+    const struct codel *codel = codel_get__(netdev);
+    smap_add_format(details, "target", "%u", codel->target);
+    smap_add_format(details, "limit", "%u", codel->limit);
+    smap_add_format(details, "interval", "%u", codel->interval);
+    return 0;
+}
+
+static int
+codel_qdisc_set(struct netdev *netdev, const struct smap *details)
+{
+    struct codel codel;
+
+    codel_parse_qdisc_details__(netdev, details, &codel);
+    codel_install__(netdev, codel.target, codel.limit, codel.interval);
+    codel_get__(netdev)->target = codel.target;
+    codel_get__(netdev)->limit = codel.limit;
+    codel_get__(netdev)->interval = codel.interval;
+    return 0;
+}
+
+static const struct tc_ops tc_ops_codel = {
+    "codel",                      /* linux_name */
+    "linux-codel",                /* ovs_name */
+    CODEL_N_QUEUES,               /* n_queues */
+    codel_tc_install,
+    NULL,
+    codel_tc_destroy,
+    codel_qdisc_get,
+    codel_qdisc_set,
+    NULL,
+    NULL,
+    NULL,
+    NULL,
+    NULL
+};
+
+/* FQ-CoDel traffic control class. */
+
+#define FQCODEL_N_QUEUES 0x0000
+
+struct fqcodel {
+    struct tc tc;
+    uint32_t target;
+    uint32_t limit;
+    uint32_t interval;
+    uint32_t flows;
+    uint32_t quantum;
+};
+
+static struct fqcodel *
+fqcodel_get__(const struct netdev *netdev_)
+{
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+    return CONTAINER_OF(netdev->tc, struct fqcodel, tc);
+}
+
+static void
+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_);
+    struct fqcodel *fqcodel;
+
+    fqcodel = xmalloc(sizeof *fqcodel);
+    tc_init(&fqcodel->tc, &tc_ops_fqcodel);
+    fqcodel->target = target;
+    fqcodel->limit = limit;
+    fqcodel->interval = interval;
+    fqcodel->flows = flows;
+    fqcodel->quantum = quantum;
+
+    netdev->tc = &fqcodel->tc;
+}
+
+static int
+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;
+    struct ofpbuf request;
+    struct tcmsg *tcmsg;
+    uint32_t otarget, olimit, ointerval, oflows,  oquantum;
+    int error;
+
+    tc_del_qdisc(netdev);
+
+    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
+                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    if (!tcmsg) {
+        return ENODEV;
+    }
+    tcmsg->tcm_handle = tc_make_handle(1, 0);
+    tcmsg->tcm_parent = TC_H_ROOT;
+
+    otarget = target ? target : 5000;
+    olimit = limit ? limit : 10240;
+    ointerval = interval ? interval : 100000;
+    oflows = flows ? flows : 1024;
+    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_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)",
+        netdev_get_name(netdev),
+        otarget, olimit, ointerval, oflows, oquantum,
+        error, ovs_strerror(error));
+    return error;
+}
+
+static void
+fqcodel_parse_qdisc_details__(struct netdev *netdev OVS_UNUSED,
+                          const struct smap *details, struct fqcodel *fqcodel)
+{
+    const char *target_s;
+    const char *limit_s;
+    const char *interval_s;
+    const char *flows_s;
+    const char *quantum_s;
+
+    target_s = smap_get(details, "target");
+    limit_s = smap_get(details, "limit");
+    interval_s = smap_get(details, "interval");
+    flows_s = smap_get(details, "flows");
+    quantum_s = smap_get(details, "quantum");
+    fqcodel->target = target_s ? strtoull(target_s, NULL, 10) : 0;
+    fqcodel->limit = limit_s ? strtoull(limit_s, NULL, 10) : 0;
+    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;
+}
+
+static int
+fqcodel_tc_install(struct netdev *netdev, const struct smap *details)
+{
+    int error;
+    struct fqcodel fqcodel;
+
+    fqcodel_parse_qdisc_details__(netdev, details, &fqcodel);
+    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.interval, fqcodel.flows, fqcodel.quantum);
+    }
+    return error;
+}
+
+static void
+fqcodel_tc_destroy(struct tc *tc)
+{
+    struct fqcodel *fqcodel = CONTAINER_OF(tc, struct fqcodel, tc);
+    tc_destroy(tc);
+    free(fqcodel);
+}
+
+static int
+fqcodel_qdisc_get(const struct netdev *netdev, struct smap *details)
+{
+    const struct fqcodel *fqcodel = fqcodel_get__(netdev);
+    smap_add_format(details, "target", "%u", fqcodel->target);
+    smap_add_format(details, "limit", "%u", fqcodel->limit);
+    smap_add_format(details, "interval", "%u", fqcodel->interval);
+    smap_add_format(details, "flows", "%u", fqcodel->flows);
+    smap_add_format(details, "quantum", "%u", fqcodel->quantum);
+    return 0;
+}
+
+static int
+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.flows, fqcodel.quantum);
+    fqcodel_get__(netdev)->target = fqcodel.target;
+    fqcodel_get__(netdev)->limit = fqcodel.limit;
+    fqcodel_get__(netdev)->interval = fqcodel.interval;
+    fqcodel_get__(netdev)->flows = fqcodel.flows;
+    fqcodel_get__(netdev)->quantum = fqcodel.quantum;
+    return 0;
+}
+
+static const struct tc_ops tc_ops_fqcodel = {
+    "fq_codel",                      /* linux_name */
+    "linux-fq_codel",                /* ovs_name */
+    FQCODEL_N_QUEUES,                /* n_queues */
+    fqcodel_tc_install,
+    NULL,
+    fqcodel_tc_destroy,
+    fqcodel_qdisc_get,
+    fqcodel_qdisc_set,
+    NULL,
+    NULL,
+    NULL,
+    NULL,
+    NULL
+};
+
+/* SFQ traffic control class. */
+
+#define SFQ_N_QUEUES 0x0000
+
+struct sfq {
+    struct tc tc;
+    uint32_t quantum;
+    uint32_t perturb;
+};
+
+static struct sfq *
+sfq_get__(const struct netdev *netdev_)
+{
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+    return CONTAINER_OF(netdev->tc, struct sfq, tc);
+}
+
+static void
+sfq_install__(struct netdev *netdev_, uint32_t quantum, uint32_t perturb)
+{
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+    struct sfq *sfq;
+
+    sfq = xmalloc(sizeof *sfq);
+    tc_init(&sfq->tc, &tc_ops_sfq);
+    sfq->perturb = perturb;
+    sfq->quantum = quantum;
+
+    netdev->tc = &sfq->tc;
+}
+
+static int
+sfq_setup_qdisc__(struct netdev *netdev, uint32_t quantum, uint32_t perturb)
+{
+    struct tc_sfq_qopt opt;
+    struct ofpbuf request;
+    struct tcmsg *tcmsg;
+    int mtu;
+    int mtu_error, error;
+    mtu_error = netdev_linux_get_mtu__(netdev_linux_cast(netdev), &mtu);
+
+    tc_del_qdisc(netdev);
+
+    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
+                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    if (!tcmsg) {
+        return ENODEV;
+    }
+    tcmsg->tcm_handle = tc_make_handle(1, 0);
+    tcmsg->tcm_parent = TC_H_ROOT;
+
+    memset(&opt, 0, sizeof opt);
+    if (!quantum) {
+        if (!mtu_error)
+            opt.quantum = mtu; /* if we cannot find mtu, use default */
+    }
+    else
+        opt.quantum = quantum;
+
+    if (!perturb)
+        opt.perturb_period = 10;
+    else
+        opt.perturb_period = perturb;
+
+    nl_msg_put_string(&request, TCA_KIND, "sfq");
+    nl_msg_put_unspec(&request, TCA_OPTIONS, &opt, sizeof opt);
+
+    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));
+    return error;
+}
+
+static void
+sfq_parse_qdisc_details__(struct netdev *netdev,
+                          const struct smap *details, struct sfq *sfq)
+{
+    const char *perturb_s;
+    const char *quantum_s;
+    int mtu;
+    int mtu_error;
+
+    perturb_s = smap_get(details, "perturb");
+    quantum_s = smap_get(details, "quantum");
+    sfq->perturb = perturb_s ? strtoull(perturb_s, NULL, 10) : 0;
+    sfq->quantum = quantum_s ? strtoull(quantum_s, NULL, 10) : 0;
+    if (!sfq->perturb) sfq->perturb = 10;
+    if (!sfq->quantum) {
+        mtu_error = netdev_linux_get_mtu__(netdev_linux_cast(netdev), &mtu);
+        if(!mtu_error) {
+            sfq->quantum = mtu;
+        } else {
+            VLOG_WARN_RL(&rl, "when using SFQ, you must specify quantum on a 
device without mtu");
+            return;
+        }
+    }
+}
+
+static int
+sfq_tc_install(struct netdev *netdev, const struct smap *details)
+{
+    int error;
+    struct sfq sfq;
+
+    sfq_parse_qdisc_details__(netdev, details, &sfq);
+    error = sfq_setup_qdisc__(netdev, sfq.quantum, sfq.perturb);
+    if (!error) {
+        sfq_install__(netdev, sfq.quantum, sfq.perturb);
+    }
+    return error;
+}
+
+static void
+sfq_tc_destroy(struct tc *tc)
+{
+    struct sfq *sfq = CONTAINER_OF(tc, struct sfq, tc);
+    tc_destroy(tc);
+    free(sfq);
+}
+
+static int
+sfq_qdisc_get(const struct netdev *netdev, struct smap *details)
+{
+    const struct sfq *sfq = sfq_get__(netdev);
+    smap_add_format(details, "quantum", "%u", sfq->quantum);
+    smap_add_format(details, "perturb", "%u", sfq->perturb);
+    return 0;
+}
+
+static int
+sfq_qdisc_set(struct netdev *netdev, const struct smap *details)
+{
+    struct sfq sfq;
+
+    sfq_parse_qdisc_details__(netdev, details, &sfq);
+    sfq_install__(netdev, sfq.quantum, sfq.perturb);
+    sfq_get__(netdev)->quantum = sfq.quantum;
+    sfq_get__(netdev)->perturb = sfq.perturb;
+    return 0;
+}
+
+static const struct tc_ops tc_ops_sfq = {
+    "sfq",                      /* linux_name */
+    "linux-sfq",                /* ovs_name */
+    SFQ_N_QUEUES,               /* n_queues */
+    sfq_tc_install,
+    NULL,
+    sfq_tc_destroy,
+    sfq_qdisc_get,
+    sfq_qdisc_set,
+    NULL,
+    NULL,
+    NULL,
+    NULL,
+    NULL
+};
+
 /* HTB traffic control class. */
 
 #define HTB_N_QUEUES 0xf000
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index dd622dc..9338c2e 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -4382,7 +4382,7 @@ iface_configure_qos(struct iface *iface, const struct 
ovsrec_qos *qos)
 
     ofpbuf_init(&queues_buf, 0);
 
-    if (!qos || qos->type[0] == '\0' || qos->n_queues < 1) {
+    if (!qos || qos->type[0] == '\0') {
         netdev_set_qos(iface->netdev, NULL, NULL);
     } else {
         const struct ovsdb_datum *queues;

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to