Hi Lorenzo,
I think a better approach to this would be to modify
ovn_extend_table_id_init() to take two additional parameters: one
parameter would tell the size of the extend table bitmap. The other
would be an offset to add to IDs produced by the bitmap.
This has a few advantages over the current approach:
* There is no wasted space in the extend table bitmap for meters.
* It makes the ID space a property of the extend_table, rather than a
parameter of the assignment function.
* It makes it less likely for a developer to make a mistake when
assigning an ID (e.g. passing is_meter == "true" when allocating an ID
on the group table).
* It requires making fewer changes throughout the code.
On 2/24/22 05:38, Lorenzo Bianconi wrote:
This is a preliminary patch to move metering management in IP engine.
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
controller/lflow.c | 2 +-
lib/actions.c | 10 +++++-----
lib/extend-table.c | 6 ++++--
lib/extend-table.h | 2 +-
tests/ovn-controller.at | 6 +++---
tests/ovn.at | 10 +++++-----
6 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index e169edef1..e87b1c66e 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -1094,7 +1094,7 @@ lflow_parse_ctrl_meter(const struct sbrec_logical_flow
*lflow,
if (lflow->controller_meter) {
*meter_id = ovn_extend_table_assign_id(meter_table,
lflow->controller_meter,
- lflow->header_.uuid);
+ lflow->header_.uuid, true);
if (*meter_id == EXT_TABLE_ID_INVALID) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "Unable to assign id for meter: %s",
diff --git a/lib/actions.c b/lib/actions.c
index 5d3caaf2b..a49d07b04 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1307,7 +1307,7 @@ encode_CT_LB(const struct ovnact_ct_lb *cl,
}
table_id = ovn_extend_table_assign_id(ep->group_table, ds_cstr(&ds),
- ep->lflow_uuid);
+ ep->lflow_uuid, false);
ds_destroy(&ds);
if (table_id == EXT_TABLE_ID_INVALID) {
return;
@@ -1447,7 +1447,7 @@ encode_SELECT(const struct ovnact_select *select,
}
table_id = ovn_extend_table_assign_id(ep->group_table, ds_cstr(&ds),
- ep->lflow_uuid);
+ ep->lflow_uuid, false);
ds_destroy(&ds);
if (table_id == EXT_TABLE_ID_INVALID) {
return;
@@ -3413,7 +3413,7 @@ encode_LOG(const struct ovnact_log *log,
if (log->meter) {
meter_id = ovn_extend_table_assign_id(ep->meter_table, log->meter,
- ep->lflow_uuid);
+ ep->lflow_uuid, true);
if (meter_id == EXT_TABLE_ID_INVALID) {
VLOG_WARN("Unable to assign id for log meter: %s", log->meter);
return;
@@ -3508,7 +3508,7 @@ encode_SET_METER(const struct ovnact_set_meter *cl,
}
table_id = ovn_extend_table_assign_id(ep->meter_table, name,
- ep->lflow_uuid);
+ ep->lflow_uuid, true);
free(name);
if (table_id == EXT_TABLE_ID_INVALID) {
return;
@@ -3849,7 +3849,7 @@ encode_FWD_GROUP(const struct ovnact_fwd_group *fwd_group,
uint32_t table_id = 0;
struct ofpact_group *og;
table_id = ovn_extend_table_assign_id(ep->group_table, ds_cstr(&ds),
- ep->lflow_uuid);
+ ep->lflow_uuid, false);
ds_destroy(&ds);
if (table_id == EXT_TABLE_ID_INVALID) {
return;
diff --git a/lib/extend-table.c b/lib/extend-table.c
index c708e24b9..219624cc3 100644
--- a/lib/extend-table.c
+++ b/lib/extend-table.c
@@ -286,7 +286,7 @@ ovn_extend_table_sync(struct ovn_extend_table *table)
* If it already exists, return the old ID. */
uint32_t
ovn_extend_table_assign_id(struct ovn_extend_table *table, const char *name,
- struct uuid lflow_uuid)
+ struct uuid lflow_uuid, bool is_meter)
{
uint32_t table_id = 0, hash;
struct ovn_extend_table_info *table_info;
@@ -315,8 +315,10 @@ ovn_extend_table_assign_id(struct ovn_extend_table *table,
const char *name,
bool new_table_id = false;
if (!table_id) {
+ size_t start = is_meter ? MAX_EXT_TABLE_ID / 2 : 1;
/* Reserve a new group_id. */
- table_id = bitmap_scan(table->table_ids, 0, 1, MAX_EXT_TABLE_ID + 1);
+ table_id = bitmap_scan(table->table_ids, 0, start,
+ MAX_EXT_TABLE_ID + 1);
new_table_id = true;
}
diff --git a/lib/extend-table.h b/lib/extend-table.h
index 4d80cfd80..05d4271c0 100644
--- a/lib/extend-table.h
+++ b/lib/extend-table.h
@@ -92,7 +92,7 @@ void ovn_extend_table_sync(struct ovn_extend_table *);
uint32_t ovn_extend_table_assign_id(struct ovn_extend_table *,
const char *name,
- struct uuid lflow_uuid);
+ struct uuid lflow_uuid, bool is_meter);
/* Iterates 'DESIRED' through all of the 'ovn_extend_table_info's in
* 'TABLE'->desired that are not in 'TABLE'->existing. (The loop body
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 6e4c24f0f..3d1fbd26f 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -725,7 +725,7 @@ check ovn-nbctl meter-add event-elb drop 100 pktps 10
check ovn-nbctl --wait=hv copp-add copp0 event-elb event-elb
check ovn-nbctl --wait=hv ls-copp-add copp0 ls1
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.0f | grep -q meter_id=1])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.0f | grep -q meter_id=32767])
check ovn-nbctl copp-del copp0
AT_CHECK([ovn-nbctl copp-list copp0], [0], [dnl
@@ -738,13 +738,13 @@ check ovn-nbctl --wait=hv copp-add copp1 reject acl-meter
check ovn-nbctl ls-copp-add copp1 ls1
check ovn-nbctl --wait=hv acl-add ls1 from-lport 1002 'inport == "lsp1" && ip
&& udp' reject
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.16 | grep -q meter_id=1])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.16 | grep -q meter_id=32767])
# arp metering
check ovn-nbctl meter-add arp-meter drop 200 pktps 0
check ovn-nbctl --wait=hv copp-add copp2 arp-resolve arp-meter
check ovn-nbctl --wait=hv lr-copp-add copp2 lr1
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.00 | grep -q meter_id=2])
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep
userdata=00.00.00.00 | grep -q meter_id=32768])
OVN_CLEANUP([hv1])
AT_CLEANUP
diff --git a/tests/ovn.at b/tests/ovn.at
index fa0e0a4a2..d2be9f946 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1584,13 +1584,13 @@ reg1[0] = dns_lookup("foo");
set_meter(0);
Rate 0 for set_meter is not in valid.
set_meter(1);
- encodes as meter:1
+ encodes as meter:32767
set_meter(100, 1000);
- encodes as meter:2
+ encodes as meter:32768
set_meter(100, 1000, );
Syntax error at `,' expecting `)'.
set_meter(4294967295, 4294967295);
- encodes as meter:3
+ encodes as meter:32769
# log
log(verdict=allow, severity=warning);
@@ -9159,8 +9159,8 @@ AT_SKIP_IF([test $d_secs -gt 9])
# Print some information that may help debugging.
AT_CHECK([as hv ovs-appctl -t ovn-controller meter-table-list], [0], [dnl
-http-rl1: 1
-http-rl2: 2
+http-rl1: 32767
+http-rl2: 32768
])
as hv ovs-ofctl -O OpenFlow13 meter-stats br-int
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
