On Mon, Feb 08, 2016 at 03:20:11PM +0530, Babu Shanmugam wrote:
The qos settings are managed using the 'options' fields in the
"Port_Binding" table.
Signed-off-by: Babu Shanmugam <bscha...@redhat.com>
This seems more complicated than necessary. In the
SBREC_PORT_BINDING_FOR_EACH loop in binding_run(), I think that we only
need to be able to find the ovsrec_interface associated with the
binding_rec. Then we can check and possibly update the
ovsrec_interface's ingress_policing_rate and ingress_policing_burst.
Can you explain why there's this more complicated data structure?
If not for this iface_qos hash map, we need to run a loop OVSREC
Interface_table entries for every iteration of SBREC Port_Binding entry to
check against its policing_values and update it if needed.
If we have an api to fetch the Interface_table entry with the interface id,
this hash map wouldn't be needed. I was not able find such an api, but is
there any such api?
Below is a summary on the use of this hash map;
* At the start of binding_run(), policing values of all the interfaces
are hashed inside this map with the interface_id as the key.
* On each iteration of SBREC Port_Binding, this hash map is looked up
and policing values of the SBREC port_binding and OVSREC
Interface_table are compared and one of the following actions are taken
o If both the values are equal, then the hash map entry
corresponding to this interface id is removed
o If there is a difference, the difference is noted in the hash
map entry
* After looping SBREC_Port_Binding, the OVSREC Inteface_table is
looped for all the entries, and if the interface_id is present in
the hash map, its policing values are updated.
Here's what I had in mind. It's untested.
--8<--------------------------cut here-------------------------->8--
From: Ben Pfaff <b...@ovn.org>
Date: Mon, 8 Feb 2016 16:14:20 -0800
Subject: [PATCH] ovn-controller: Copy southbound database policing settings
into OVS config.
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
ovn/controller/binding.c | 46 ++++++++++++++++++++++++++++-------------
ovn/controller/ovn-controller.c | 4 ++++
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index ce9cccf..f295473 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2015 Nicira, Inc.
+/* Copyright (c) 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -47,7 +47,7 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
}
static void
-get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports)
+get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
{
int i;
@@ -68,7 +68,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, struct sset *lports)
if (!iface_id) {
continue;
}
- sset_add(lports, iface_id);
+ shash_add(lports, iface_id, iface_rec);
}
}
}
@@ -132,6 +132,17 @@ add_local_datapath(struct hmap *local_datapaths,
}
}
+static void
+update_qos(const struct ovsrec_interface *iface_rec,
+ const struct sbrec_port_binding *pb)
+{
+ int rate = smap_get_int(&pb->options, "policing_rate", 0);
+ int burst = smap_get_int(&pb->options, "policing_burst", 0);
+
+ ovsrec_interface_set_ingress_policing_rate(iface_rec, MAX(0, rate));
+ ovsrec_interface_set_ingress_policing_burst(iface_rec, MAX(0, burst));
+}
+
void
binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
const char *chassis_id, struct simap *ct_zones,
@@ -139,8 +150,6 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
{
const struct sbrec_chassis *chassis_rec;
const struct sbrec_port_binding *binding_rec;
- struct sset lports, all_lports;
- const char *name;
if (!ctx->ovnsb_idl_txn) {
return;
@@ -151,15 +160,19 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
return;
}
- sset_init(&lports);
- sset_init(&all_lports);
+ struct shash lports = SHASH_INITIALIZER(&lports);
if (br_int) {
get_local_iface_ids(br_int, &lports);
} else {
/* We have no integration bridge, therefore no local logical ports.
* We'll remove our chassis from all port binding records below. */
}
- sset_clone(&all_lports, &lports);
+
+ struct sset all_lports = SSET_INITIALIZER(&all_lports);
+ struct shash_node *node;
+ SHASH_FOR_EACH (node, &lports) {
+ sset_add(&all_lports, node->name);
+ }
ovsdb_idl_txn_add_comment(
ctx->ovnsb_idl_txn,"ovn-controller: updating port bindings for '%s'",
@@ -169,14 +182,19 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
* chassis and update the binding accordingly. This includes both
* directly connected logical ports and children of those ports. */
SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) {
- if (sset_find_and_delete(&lports, binding_rec->logical_port) ||
- (binding_rec->parent_port && binding_rec->parent_port[0] &&
- sset_contains(&all_lports, binding_rec->parent_port))) {
+ const struct ovsrec_interface *iface_rec
+ = shash_find_and_delete(&lports, binding_rec->logical_port);
+ if (iface_rec
+ || (binding_rec->parent_port && binding_rec->parent_port[0] &&
+ sset_contains(&all_lports, binding_rec->parent_port))) {
if (binding_rec->parent_port && binding_rec->parent_port[0]) {
/* Add child logical port to the set of all local ports. */
sset_add(&all_lports, binding_rec->logical_port);
}
add_local_datapath(local_datapaths, binding_rec);
+ if (iface_rec) {
+ update_qos(iface_rec, binding_rec);
+ }
if (binding_rec->chassis == chassis_rec) {
continue;
}
@@ -199,13 +217,13 @@ binding_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
}
}
- SSET_FOR_EACH (name, &lports) {
- VLOG_DBG("No port binding record for lport %s", name);
+ SHASH_FOR_EACH (node, &lports) {
+ VLOG_DBG("No port binding record for lport %s", node->name);
}
update_ct_zones(&all_lports, ct_zones, ct_zone_bitmap);
- sset_destroy(&lports);
+ shash_destroy(&lports);
sset_destroy(&all_lports);
}
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 3638342..ac824ad 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -240,6 +240,10 @@ main(int argc, char *argv[])
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_interface_col_name);
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_interface_col_type);
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_interface_col_options);
+ ovsdb_idl_add_column(ovs_idl_loop.idl,
+ &ovsrec_interface_col_ingress_policing_rate);
+ ovsdb_idl_add_column(ovs_idl_loop.idl,
+ &ovsrec_interface_col_ingress_policing_burst);
ovsdb_idl_add_table(ovs_idl_loop.idl, &ovsrec_table_port);
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_port_col_name);
ovsdb_idl_add_column(ovs_idl_loop.idl, &ovsrec_port_col_interfaces);
