| Message ID | 1500951228-18646-2-git-send-email-zhaozhanxu@163.com |
|---|---|
| State | Changes Requested |
| Headers | show |
> This patch adds similar QoS configration with kernel datapath. > > It adds some functions whitch is pointed by `get_queue`, `set_queue` and > `delete_queue` of structure `netdev_class` to support configuration. > > Then the configuration command changed from command A(see below) to > command B, but it only support to configure and rate limitation function > is not ready now. > > Command A: (original command) > > $ ovs-vsctl set port vhost-user0 qos=@newqos -- \ > --id=@newqos create qos type=egress-policer \ > other-config:cir=46000000 other-config:cbs=2048` > > Command B: (new command) > > $ ovs-vsctl set port vhost-user0 qos=@newqos -- \ > --id=@newqos create qos type=egress-policer \ > other-config:cir=46000000 other-config:cbs=2048 \ > queues:123=@q123 queues:234=@q234 -- \ > --id=@q123 create queue \ > other-config:cir=12800000 other-config:cbs=2048 -- \ > --id=@q234 create queue \ > other-config:cir=25600000 other-config:cbs=2048` It's probably clearer in the code below, but does this break backwards compatibility with the previous implementation of the egress policer? I.e. after this patchset can a user still use method A to setup a policer on a per port basis rather than a per queue basis for that port? > > Signed-off-by: zhaozhanxu <zhaozhanxu@163.com> > --- > lib/netdev-dpdk.c | 197 > ++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 193 insertions(+), 4 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ea17b97..089ad64 > 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -259,6 +259,31 @@ struct dpdk_qos_ops { > */ > int (*qos_run)(struct qos_conf *qos_conf, struct rte_mbuf **pkts, > int pkt_cnt); > + > + /* Called to construct a QoS Queue. The implementation should make > + * the appropriate calls to configure QoS Queue according to > 'details'. > + * > + * The contents of 'details' should be documented as valid for > 'ovs_name' > + * in the "other_config" column in the "QoS" table in > vswitchd/vswitch.xml > + * (which is built as ovs-vswitchd.conf.db(8)). > + * > + * This function must return 0 if and only if it constructs > + * qos queue successfully. > + */ > + int (*qos_queue_construct)(const struct smap *details, > + uint32_t queue_id, struct qos_conf > + *conf); > + > + /* Destroys the Qos Queue */ > + void (*qos_queue_destruct)(struct qos_conf *conf, uint32_t > + queue_id); > + > + /* Retrieves details of QoS Queue configuration into 'details'. > + * > + * The contents of 'details' should be documented as valid for > 'ovs_name' > + * in the "other_config" column in the "QoS" table in > vswitchd/vswitch.xml > + * (which is built as ovs-vswitchd.conf.db(8)). > + */ > + int (*qos_queue_get)(struct smap *details, uint32_t queue_id, > + const struct qos_conf *conf); > }; > > /* dpdk_qos_ops for each type of user space QoS implementation */ @@ - > 2986,14 +3011,94 @@ netdev_dpdk_set_qos(struct netdev *netdev, const char > *type, > return error; > } > > +static int > +netdev_dpdk_get_queue(const struct netdev *netdev, uint32_t queue_id, > + struct smap *details) { > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > + struct qos_conf *qos_conf; > + int error = 0; > + > + ovs_mutex_lock(&dev->mutex); > + > + qos_conf = ovsrcu_get_protected(struct qos_conf *, &dev->qos_conf); > + if (!qos_conf || !qos_conf->ops || !qos_conf->ops->qos_queue_get) { > + error = EOPNOTSUPP; > + } else { > + error = qos_conf->ops->qos_queue_get(details, queue_id, > qos_conf); > + } > + > + ovs_mutex_unlock(&dev->mutex); > + > + return error; > +} > + > +static int > +netdev_dpdk_set_queue(struct netdev *netdev, uint32_t queue_id, > + const struct smap *details) { > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > + struct qos_conf *qos_conf; > + int error = 0; > + > + ovs_mutex_lock(&dev->mutex); > + > + qos_conf = ovsrcu_get_protected(struct qos_conf *, &dev->qos_conf); > + if (!qos_conf || !qos_conf->ops || !qos_conf->ops- > >qos_queue_construct) { > + error = EOPNOTSUPP; > + } else { > + error = qos_conf->ops->qos_queue_construct(details, queue_id, > + qos_conf); > + } > + > + if (error) { > + VLOG_ERR("Failed to set QoS queue %d on port %s: %s", > + queue_id, netdev->name, rte_strerror(error)); > + } > + > + ovs_mutex_unlock(&dev->mutex); > + > + return error; > +} > + > +static int > +netdev_dpdk_delete_queue(struct netdev *netdev, uint32_t queue_id) { > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > + struct qos_conf *qos_conf; > + > + ovs_mutex_lock(&dev->mutex); > + > + qos_conf = ovsrcu_get_protected(struct qos_conf *, &dev->qos_conf); > + if (qos_conf && qos_conf->ops && qos_conf->ops->qos_queue_destruct) { > + qos_conf->ops->qos_queue_destruct(qos_conf, queue_id); > + } > + > + ovs_mutex_unlock(&dev->mutex); > + > + return 0; > +} > + > /* egress-policer details */ > > struct egress_policer { > struct qos_conf qos_conf; > struct rte_meter_srtcm_params app_srtcm_params; > struct rte_meter_srtcm egress_meter; > + struct hmap queue; > }; > > +struct egress_queue_policer { Minor nit but the struct name would sound/read better as 'egress_policer_queue', i.e. the QoS type followed by queue. > + struct hmap_node hmap_node; > + uint32_t queue_id; > + struct rte_meter_srtcm_params app_srtcm_params; > + struct rte_meter_srtcm egress_meter; }; Coding style infraction, closing brace and semi colon above should be moved to next line. > + > +static struct egress_queue_policer * > +egress_policer_qos_find_queue(struct egress_policer *policer, > + uint32_t queue_id); > + > static void > egress_policer_details_to_param(const struct smap *details, > struct rte_meter_srtcm_params *params) @@ > -3018,6 +3123,7 @@ egress_policer_qos_construct(const struct smap > *details, > &policer->app_srtcm_params); > if (!err) { > *conf = &policer->qos_conf; > + hmap_init(&policer->queue); > } else { > free(policer); > *conf = NULL; > @@ -3032,6 +3138,13 @@ egress_policer_qos_destruct(struct qos_conf *conf) > { > struct egress_policer *policer = CONTAINER_OF(conf, struct > egress_policer, > qos_conf); > + struct egress_queue_policer *queue_policer, *next_queue_policer; Again, minor nit and same point as above, I'd prefer to see the variables 'queue_policer' renamed to 'policer_queueu', the same with 'next_queue_policer'. If anything it establishes that a queue is associated with the policer struct and reads better, can be applied to all other occurrences below. > + HMAP_FOR_EACH_SAFE (queue_policer, next_queue_policer, hmap_node, > + &policer->queue) { > + hmap_remove(&policer->queue, &queue_policer->hmap_node); > + free(queue_policer); > + } > + hmap_destroy(&policer->queue); > free(policer); > } > > @@ -3072,13 +3185,89 @@ egress_policer_run(struct qos_conf *conf, struct > rte_mbuf **pkts, int pkt_cnt) > return cnt; > } > > +static struct egress_queue_policer * > +egress_policer_qos_find_queue(struct egress_policer *policer, > + uint32_t queue_id) { > + struct egress_queue_policer *queue_policer; > + HMAP_FOR_EACH_WITH_HASH (queue_policer, hmap_node, > + hash_2words(queue_id, 0), &policer->queue) { > + if (queue_policer->queue_id == queue_id) { > + return queue_policer; > + } > + } > + return NULL; > +} > + > +static int > +egress_policer_qos_queue_construct(const struct smap *details, > + uint32_t queue_id, struct qos_conf > +*conf) { > + int err = 0; > + struct egress_policer *policer = CONTAINER_OF(conf, struct > egress_policer, > + qos_conf); > + struct egress_queue_policer *queue_policer; > + queue_policer = egress_policer_qos_find_queue(policer, queue_id); > + if (!queue_policer) { > + queue_policer = xmalloc(sizeof *queue_policer); > + hmap_insert(&policer->queue, &queue_policer->hmap_node, > + hash_2words(queue_id, 0)); > + queue_policer->queue_id = queue_id; > + } > + egress_policer_details_to_param(details, &queue_policer- > >app_srtcm_params); > + err = rte_meter_srtcm_config(&queue_policer->egress_meter, > + &queue_policer->app_srtcm_params); > + if (err) { > + hmap_remove(&policer->queue, &queue_policer->hmap_node); > + free(queue_policer); > + err = -err; > + } > + > + return err; > +} > + > +static void > +egress_policer_qos_queue_destruct(struct qos_conf *conf, uint32_t > +queue_id) { > + struct egress_policer *policer = CONTAINER_OF(conf, struct > egress_policer, > + qos_conf); > + struct egress_queue_policer *queue_policer; > + queue_policer = egress_policer_qos_find_queue(policer, queue_id); > + hmap_remove(&policer->queue, &queue_policer->hmap_node); > + free(queue_policer); > +} > + > +static int > +egress_policer_qos_queue_get(struct smap *details, uint32_t queue_id, > + const struct qos_conf *conf) { > + struct egress_policer *policer = > + CONTAINER_OF(conf, struct egress_policer, qos_conf); > + > + struct egress_queue_policer *queue_policer; > + queue_policer = egress_policer_qos_find_queue(policer, queue_id); > + if (!queue_policer) { > + return -1; > + } > + > + smap_add_format(details, "cir", "%"PRIu64, > + queue_policer->app_srtcm_params.cir); > + smap_add_format(details, "cbs", "%"PRIu64, > + queue_policer->app_srtcm_params.cbs); > + > + return 0; > +} > + > static const struct dpdk_qos_ops egress_policer_ops = { > "egress-policer", /* qos_name */ > egress_policer_qos_construct, > egress_policer_qos_destruct, > egress_policer_qos_get, > egress_policer_qos_is_equal, > - egress_policer_run > + egress_policer_run, > + egress_policer_qos_queue_construct, > + egress_policer_qos_queue_destruct, > + egress_policer_qos_queue_get > }; > > static int > @@ -3260,9 +3449,9 @@ unlock: > NULL, /* get_qos_capabilities */ \ > netdev_dpdk_get_qos, \ > netdev_dpdk_set_qos, \ > - NULL, /* get_queue */ \ > - NULL, /* set_queue */ \ > - NULL, /* delete_queue */ \ > + netdev_dpdk_get_queue, \ > + netdev_dpdk_set_queue, \ > + netdev_dpdk_delete_queue, \ > NULL, /* get_queue_stats */ \ > NULL, /* queue_dump_start */ \ > NULL, /* queue_dump_next */ \ > -- > 2.7.4 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ea17b97..089ad64 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -259,6 +259,31 @@ struct dpdk_qos_ops { */ int (*qos_run)(struct qos_conf *qos_conf, struct rte_mbuf **pkts, int pkt_cnt); + + /* Called to construct a QoS Queue. The implementation should make + * the appropriate calls to configure QoS Queue according to 'details'. + * + * The contents of 'details' should be documented as valid for 'ovs_name' + * in the "other_config" column in the "QoS" table in vswitchd/vswitch.xml + * (which is built as ovs-vswitchd.conf.db(8)). + * + * This function must return 0 if and only if it constructs + * qos queue successfully. + */ + int (*qos_queue_construct)(const struct smap *details, + uint32_t queue_id, struct qos_conf *conf); + + /* Destroys the Qos Queue */ + void (*qos_queue_destruct)(struct qos_conf *conf, uint32_t queue_id); + + /* Retrieves details of QoS Queue configuration into 'details'. + * + * The contents of 'details' should be documented as valid for 'ovs_name' + * in the "other_config" column in the "QoS" table in vswitchd/vswitch.xml + * (which is built as ovs-vswitchd.conf.db(8)). + */ + int (*qos_queue_get)(struct smap *details, uint32_t queue_id, + const struct qos_conf *conf); }; /* dpdk_qos_ops for each type of user space QoS implementation */ @@ -2986,14 +3011,94 @@ netdev_dpdk_set_qos(struct netdev *netdev, const char *type, return error; } +static int +netdev_dpdk_get_queue(const struct netdev *netdev, uint32_t queue_id, + struct smap *details) +{ + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); + struct qos_conf *qos_conf; + int error = 0; + + ovs_mutex_lock(&dev->mutex); + + qos_conf = ovsrcu_get_protected(struct qos_conf *, &dev->qos_conf); + if (!qos_conf || !qos_conf->ops || !qos_conf->ops->qos_queue_get) { + error = EOPNOTSUPP; + } else { + error = qos_conf->ops->qos_queue_get(details, queue_id, qos_conf); + } + + ovs_mutex_unlock(&dev->mutex); + + return error; +} + +static int +netdev_dpdk_set_queue(struct netdev *netdev, uint32_t queue_id, + const struct smap *details) +{ + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); + struct qos_conf *qos_conf; + int error = 0; + + ovs_mutex_lock(&dev->mutex); + + qos_conf = ovsrcu_get_protected(struct qos_conf *, &dev->qos_conf); + if (!qos_conf || !qos_conf->ops || !qos_conf->ops->qos_queue_construct) { + error = EOPNOTSUPP; + } else { + error = qos_conf->ops->qos_queue_construct(details, queue_id, + qos_conf); + } + + if (error) { + VLOG_ERR("Failed to set QoS queue %d on port %s: %s", + queue_id, netdev->name, rte_strerror(error)); + } + + ovs_mutex_unlock(&dev->mutex); + + return error; +} + +static int +netdev_dpdk_delete_queue(struct netdev *netdev, uint32_t queue_id) +{ + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); + struct qos_conf *qos_conf; + + ovs_mutex_lock(&dev->mutex); + + qos_conf = ovsrcu_get_protected(struct qos_conf *, &dev->qos_conf); + if (qos_conf && qos_conf->ops && qos_conf->ops->qos_queue_destruct) { + qos_conf->ops->qos_queue_destruct(qos_conf, queue_id); + } + + ovs_mutex_unlock(&dev->mutex); + + return 0; +} + /* egress-policer details */ struct egress_policer { struct qos_conf qos_conf; struct rte_meter_srtcm_params app_srtcm_params; struct rte_meter_srtcm egress_meter; + struct hmap queue; }; +struct egress_queue_policer { + struct hmap_node hmap_node; + uint32_t queue_id; + struct rte_meter_srtcm_params app_srtcm_params; + struct rte_meter_srtcm egress_meter; +}; + +static struct egress_queue_policer * +egress_policer_qos_find_queue(struct egress_policer *policer, + uint32_t queue_id); + static void egress_policer_details_to_param(const struct smap *details, struct rte_meter_srtcm_params *params) @@ -3018,6 +3123,7 @@ egress_policer_qos_construct(const struct smap *details, &policer->app_srtcm_params); if (!err) { *conf = &policer->qos_conf; + hmap_init(&policer->queue); } else { free(policer); *conf = NULL; @@ -3032,6 +3138,13 @@ egress_policer_qos_destruct(struct qos_conf *conf) { struct egress_policer *policer = CONTAINER_OF(conf, struct egress_policer, qos_conf); + struct egress_queue_policer *queue_policer, *next_queue_policer; + HMAP_FOR_EACH_SAFE (queue_policer, next_queue_policer, hmap_node, + &policer->queue) { + hmap_remove(&policer->queue, &queue_policer->hmap_node); + free(queue_policer); + } + hmap_destroy(&policer->queue); free(policer); } @@ -3072,13 +3185,89 @@ egress_policer_run(struct qos_conf *conf, struct rte_mbuf **pkts, int pkt_cnt) return cnt; } +static struct egress_queue_policer * +egress_policer_qos_find_queue(struct egress_policer *policer, + uint32_t queue_id) +{ + struct egress_queue_policer *queue_policer; + HMAP_FOR_EACH_WITH_HASH (queue_policer, hmap_node, + hash_2words(queue_id, 0), &policer->queue) { + if (queue_policer->queue_id == queue_id) { + return queue_policer; + } + } + return NULL; +} + +static int +egress_policer_qos_queue_construct(const struct smap *details, + uint32_t queue_id, struct qos_conf *conf) +{ + int err = 0; + struct egress_policer *policer = CONTAINER_OF(conf, struct egress_policer, + qos_conf); + struct egress_queue_policer *queue_policer; + queue_policer = egress_policer_qos_find_queue(policer, queue_id); + if (!queue_policer) { + queue_policer = xmalloc(sizeof *queue_policer); + hmap_insert(&policer->queue, &queue_policer->hmap_node, + hash_2words(queue_id, 0)); + queue_policer->queue_id = queue_id; + } + egress_policer_details_to_param(details, &queue_policer->app_srtcm_params); + err = rte_meter_srtcm_config(&queue_policer->egress_meter, + &queue_policer->app_srtcm_params); + if (err) { + hmap_remove(&policer->queue, &queue_policer->hmap_node); + free(queue_policer); + err = -err; + } + + return err; +} + +static void +egress_policer_qos_queue_destruct(struct qos_conf *conf, uint32_t queue_id) +{ + struct egress_policer *policer = CONTAINER_OF(conf, struct egress_policer, + qos_conf); + struct egress_queue_policer *queue_policer; + queue_policer = egress_policer_qos_find_queue(policer, queue_id); + hmap_remove(&policer->queue, &queue_policer->hmap_node); + free(queue_policer); +} + +static int +egress_policer_qos_queue_get(struct smap *details, uint32_t queue_id, + const struct qos_conf *conf) +{ + struct egress_policer *policer = + CONTAINER_OF(conf, struct egress_policer, qos_conf); + + struct egress_queue_policer *queue_policer; + queue_policer = egress_policer_qos_find_queue(policer, queue_id); + if (!queue_policer) { + return -1; + } + + smap_add_format(details, "cir", "%"PRIu64, + queue_policer->app_srtcm_params.cir); + smap_add_format(details, "cbs", "%"PRIu64, + queue_policer->app_srtcm_params.cbs); + + return 0; +} + static const struct dpdk_qos_ops egress_policer_ops = { "egress-policer", /* qos_name */ egress_policer_qos_construct, egress_policer_qos_destruct, egress_policer_qos_get, egress_policer_qos_is_equal, - egress_policer_run + egress_policer_run, + egress_policer_qos_queue_construct, + egress_policer_qos_queue_destruct, + egress_policer_qos_queue_get }; static int @@ -3260,9 +3449,9 @@ unlock: NULL, /* get_qos_capabilities */ \ netdev_dpdk_get_qos, \ netdev_dpdk_set_qos, \ - NULL, /* get_queue */ \ - NULL, /* set_queue */ \ - NULL, /* delete_queue */ \ + netdev_dpdk_get_queue, \ + netdev_dpdk_set_queue, \ + netdev_dpdk_delete_queue, \ NULL, /* get_queue_stats */ \ NULL, /* queue_dump_start */ \ NULL, /* queue_dump_next */ \
This patch adds similar QoS configration with kernel datapath. It adds some functions whitch is pointed by `get_queue`, `set_queue` and `delete_queue` of structure `netdev_class` to support configuration. Then the configuration command changed from command A(see below) to command B, but it only support to configure and rate limitation function is not ready now. Command A: (original command) $ ovs-vsctl set port vhost-user0 qos=@newqos -- \ --id=@newqos create qos type=egress-policer \ other-config:cir=46000000 other-config:cbs=2048` Command B: (new command) $ ovs-vsctl set port vhost-user0 qos=@newqos -- \ --id=@newqos create qos type=egress-policer \ other-config:cir=46000000 other-config:cbs=2048 \ queues:123=@q123 queues:234=@q234 -- \ --id=@q123 create queue \ other-config:cir=12800000 other-config:cbs=2048 -- \ --id=@q234 create queue \ other-config:cir=25600000 other-config:cbs=2048` Signed-off-by: zhaozhanxu <zhaozhanxu@163.com> --- lib/netdev-dpdk.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 193 insertions(+), 4 deletions(-)