On Thu, Feb 18, 2016 at 03:54:26PM +0530, saloni.jai...@gmail.com wrote:
> From: Saloni Jain <saloni.j...@tcs.com>
>
> On change in a table state, the controller needs to be informed with
> the OFPT_TABLE_STATUS message. The message is sent with reason
> OFPTR_VACANCY_DOWN or OFPTR_VACANCY_UP in case of change in remaining
> space eventually crossing any one of the threshold.
>
> Signed-off-by: Saloni Jain <saloni.j...@tcs.com>
> Co-authored-by: Rishi Bamba <rishi.ba...@tcs.com>
> Signed-off-by: Rishi Bamba <rishi.ba...@tcs.com>
> ---
> difference between v9 <-> v10
> - Fixed a test case failure
This still didn't actually implement the text of the OpenFlow spec, and
in particular this initialization:
When vacancy events are enabled on the table using the
OFPTC_VACANCY_EVENTS flag, either the vacancy up or vacancy down
event is enabled. When enabling events, if the current vacancy is
less than vacancy_up, vacancy up events must be enabled, and vacancy
down events must be disabled. When enabling events, if the current
vacancy is greater or equal to vacancy_up, vacancy down events must
be enabled, and vacancy up events must be disabled.
I implemented this, folding in the following incremental change, and
applied this to master.
--8<--------------------------cut here-------------------------->8--
diff --git a/NEWS b/NEWS
index 57a250e..156fc07 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,7 @@ Post-v2.5.0
- OpenFlow:
* OpenFlow 1.1+ OFPT_QUEUE_GET_CONFIG_REQUEST now supports OFPP_ANY.
* OpenFlow 1.4+ OFPMP_QUEUE_DESC is now supported.
+ * OpenFlow 1.4+ OFPT_TABLE_STATUS is now supported.
* New property-based packet-in message format NXT_PACKET_IN2 with support
for arbitrary user-provided data and for serializing flow table
traversal into a continuation for later resumption.
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 151c5a6..de7cf3c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -10279,8 +10279,7 @@ ofputil_decode_table_status(const struct ofp_header *oh,
raw = ofpraw_pull_assert(&b);
ots = ofpbuf_pull(&b, sizeof *ots);
- if (raw == OFPRAW_OFPT14_TABLE_STATUS)
- {
+ if (raw == OFPRAW_OFPT14_TABLE_STATUS) {
if (ots->reason != OFPTR_VACANCY_DOWN
&& ots->reason != OFPTR_VACANCY_UP) {
return OFPERR_OFPBPC_BAD_VALUE;
@@ -10289,7 +10288,6 @@ ofputil_decode_table_status(const struct ofp_header *oh,
error = ofputil_decode_table_desc(&b, &ts->desc, oh->version);
return error;
-
} else {
return OFPERR_OFPBRC_BAD_VERSION;
}
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index ab1e8ec..2c6e483 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -256,8 +256,11 @@ struct oftable {
#define EVICTION_OPENFLOW (1 << 1) /* Set to 1 if OpenFlow enables eviction. */
unsigned int eviction;
- /* If true, vacancy events are enabled; otherwise they are disabled. */
- bool vacancy_enabled;
+ /* If zero, vacancy events are disabled. If nonzero, this is the type of
+ vacancy event that is enabled: either OFPTR_VACANCY_DOWN or
+ OFPTR_VACANCY_UP. Only one type of vacancy event can be enabled at a
+ time. */
+ enum ofp14_table_reason vacancy_event;
/* Non-zero values for vacancy_up and vacancy_down indicates that vacancy
* is enabled by table-mod, else these values are set to zero when
@@ -265,9 +268,6 @@ struct oftable {
uint8_t vacancy_down; /* Vacancy threshold when space decreases (%). */
uint8_t vacancy_up; /* Vacancy threshold when space increases (%). */
- /* Vacancy Reason sent through TABLE_STATUS */
- enum ofp14_table_reason vacancy_reason; /* One of OFPTR_*. */
-
atomic_ulong n_matched;
atomic_ulong n_missed;
};
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 8fe6464..1335da6 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3594,26 +3594,35 @@ handle_table_features_request(struct ofconn *ofconn,
return 0;
}
+/* Returns the vacancy of 'oftable', a number that ranges from 0 (if the table
+ * is full) to 100 (if the table is empty).
+ *
+ * A table without a limit on flows is considered to be empty. */
+static uint8_t
+oftable_vacancy(const struct oftable *t)
+{
+ return (!t->max_flows ? 100
+ : t->n_flows >= t->max_flows ? 0
+ : (t->max_flows - t->n_flows) * 100.0 / t->max_flows);
+}
+
static void
query_table_desc__(struct ofputil_table_desc *td,
struct ofproto *ofproto, uint8_t table_id)
{
- unsigned int count = ofproto->tables[table_id].n_flows;
- unsigned int max_flows = ofproto->tables[table_id].max_flows;
+ const struct oftable *t = &ofproto->tables[table_id];
td->table_id = table_id;
- td->eviction = (ofproto->tables[table_id].eviction & EVICTION_OPENFLOW
+ td->eviction = (t->eviction & EVICTION_OPENFLOW
? OFPUTIL_TABLE_EVICTION_ON
: OFPUTIL_TABLE_EVICTION_OFF);
td->eviction_flags = OFPROTO_EVICTION_FLAGS;
- td->vacancy = (ofproto->tables[table_id].vacancy_enabled
+ td->vacancy = (t->vacancy_event
? OFPUTIL_TABLE_VACANCY_ON
: OFPUTIL_TABLE_VACANCY_OFF);
- td->table_vacancy.vacancy_down = ofproto->tables[table_id].vacancy_down;
- td->table_vacancy.vacancy_up = ofproto->tables[table_id].vacancy_up;
- td->table_vacancy.vacancy = max_flows
- ? ((double) (max_flows - count) / max_flows) *
100
- : 0;
+ td->table_vacancy.vacancy_down = t->vacancy_down;
+ td->table_vacancy.vacancy_up = t->vacancy_up;
+ td->table_vacancy.vacancy = oftable_vacancy(t);
}
/* This function queries the database for dumping table-desc. */
@@ -3661,23 +3670,29 @@ handle_table_desc_request(struct ofconn *ofconn,
static void
send_table_status(struct ofproto *ofproto, uint8_t table_id)
{
- struct ofputil_table_desc td;
- enum ofp14_table_reason reason;
-
- reason = ofproto->tables[table_id].vacancy_reason;
- query_table_desc__(&td, ofproto, table_id);
-
- if (td.table_vacancy.vacancy < td.table_vacancy.vacancy_down
- && reason != OFPTR_VACANCY_DOWN) {
+ struct oftable *t = &ofproto->tables[table_id];
+ if (!t->vacancy_event) {
+ return;
+ }
- ofproto->tables[table_id].vacancy_reason = OFPTR_VACANCY_DOWN;
- connmgr_send_table_status(ofproto->connmgr, &td, OFPTR_VACANCY_DOWN);
+ uint8_t vacancy = oftable_vacancy(t);
+ enum ofp14_table_reason event;
+ if (vacancy < t->vacancy_down) {
+ event = OFPTR_VACANCY_DOWN;
+ } else if (vacancy > t->vacancy_up) {
+ event = OFPTR_VACANCY_UP;
+ } else {
+ return;
+ }
- } else if (td.table_vacancy.vacancy > td.table_vacancy.vacancy_up
- && reason != OFPTR_VACANCY_UP) {
+ if (event == t->vacancy_event) {
+ struct ofputil_table_desc td;
+ query_table_desc__(&td, ofproto, table_id);
+ connmgr_send_table_status(ofproto->connmgr, &td, event);
- ofproto->tables[table_id].vacancy_reason = OFPTR_VACANCY_UP;
- connmgr_send_table_status(ofproto->connmgr, &td, OFPTR_VACANCY_UP);
+ t->vacancy_event = (event == OFPTR_VACANCY_DOWN
+ ? OFPTR_VACANCY_UP
+ : OFPTR_VACANCY_DOWN);
}
}
@@ -4717,12 +4732,8 @@ add_flow_finish(struct ofproto *ofproto, struct
ofproto_flow_mod *ofm,
req ? req->ofconn : NULL,
req ? req->request->xid : 0, NULL);
- /* Send Vacancy Events for OF1.4+ when there is change in flow table.
- * Provided OFPTC14_VACANCY_EVENTS is set for the table. */
- if (ofproto->tables[new_rule->table_id].vacancy_enabled) {
- send_table_status(ofproto, new_rule->table_id);
- }
-
+ /* Send Vacancy Events for OF1.4+. */
+ send_table_status(ofproto, new_rule->table_id);
}
send_buffered_packet(req, fm->buffer_id, new_rule);
@@ -5119,12 +5130,8 @@ delete_flows_finish__(struct ofproto *ofproto,
req ? req->ofconn : NULL,
req ? req->request->xid : 0, NULL);
- /* Send Vacancy Events for OF1.4+ when there is change in flow
- * table. Provided OFPTC14_VACANCY_EVENTS is set for the table.
- */
- if (ofproto->tables[rule->table_id].vacancy_enabled) {
- send_table_status(ofproto, rule->table_id);
- }
+ /* Send Vacancy Event for OF1.4+. */
+ send_table_status(ofproto, rule->table_id);
ofproto_rule_remove__(ofproto, rule);
learned_cookies_dec(ofproto, rule_get_actions(rule),
@@ -6785,11 +6792,16 @@ table_mod__(struct oftable *oftable,
if (tm->vacancy != OFPUTIL_TABLE_VACANCY_DEFAULT) {
ovs_mutex_lock(&ofproto_mutex);
- oftable->vacancy_enabled = (tm->vacancy == OFPUTIL_TABLE_VACANCY_ON
- ? OFPTC14_VACANCY_EVENTS
- : 0);
oftable->vacancy_down = tm->table_vacancy.vacancy_down;
oftable->vacancy_up = tm->table_vacancy.vacancy_up;
+ if (tm->vacancy == OFPUTIL_TABLE_VACANCY_OFF) {
+ oftable->vacancy_event = 0;
+ } else if (!oftable->vacancy_event) {
+ uint8_t vacancy = oftable_vacancy(oftable);
+ oftable->vacancy_event = (vacancy < oftable->vacancy_up
+ ? OFPTR_VACANCY_UP
+ : OFPTR_VACANCY_DOWN);
+ }
ovs_mutex_unlock(&ofproto_mutex);
}
}
diff --git a/tests/ofproto.at b/tests/ofproto.at
index d5a9c29..b2b32b5 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2918,12 +2918,74 @@ OFPT_PORT_STATUS (OF1.4): MOD: ${INDEX}(test):
addr:aa:55:aa:55:00:0x
# OFPT_TABLE_STATUS, OFPTR_VACANCY_UP
if test X"$1" = X"OFPTR_VACANCY_UP"; then shift;
ovs-vsctl -- --id=@t1 create Flow_Table flow-limit=10 -- set bridge
br0 flow_tables:1=@t1
+
+ # Turn on vacancy events, then add flows until we're full.
+ # With initial vacancy of 100% and vacancy_up of 80%, so that
+ # vacancy >= vacancy_up, this enables VACANY_DOWN events, so
+ # we get a single such message when vacancy dips below 20%.
ovs-ofctl -O OpenFlow14 mod-table br0 1 vacancy:20,80
ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=1,actions=2
ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=2,actions=2
ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=3,actions=2
ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=4,actions=2
ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=5,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=6,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=7,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=8,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=9,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=10,actions=2
+ echo >>expout "OFPT_TABLE_STATUS (OF1.4): reason=VACANCY_DOWN
+table_desc:-
+ table 1:
+ eviction=off eviction_flags=OTHER|IMPORTANCE|LIFETIME
+ vacancy=on vacancy_down=20% vacancy_up=80% vacancy=10%"
+ # Then delete flows until we're empty. Sending the
+ # VACANCY_DOWN message enabled VACANCY_UP events, so we get a
+ # single such message when vacancy rises above 80%.
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=1
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=2
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=3
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=4
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=5
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=6
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=7
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=8
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=9
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=10
+ echo >>expout "OFPT_TABLE_STATUS (OF1.4): reason=VACANCY_UP
+table_desc:-
+ table 1:
+ eviction=off eviction_flags=OTHER|IMPORTANCE|LIFETIME
+ vacancy=on vacancy_down=20% vacancy_up=80% vacancy=90%"
+
+ # Now approach vacancy from the other direction. First
+ # disable vacancy events. With initial vacancy of 70%, so
+ # that vacancy < vacancy_up, this enables VACANCY_UP events.
+ # That means that filling up the table generates no message,
+ # but deleting all the flows generates VACANCY_UP at the point
+ # vacancy rises above 80%.
+ ovs-ofctl -O OpenFlow14 mod-table br0 1 novacancy
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=1,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=2,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=3,actions=2
+ ovs-ofctl -O OpenFlow14 mod-table br0 1 vacancy:20,80
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=4,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=5,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=6,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=7,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=8,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=9,actions=2
+ ovs-ofctl -O OpenFlow14 add-flow br0 table=1,in_port=10,actions=2
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=1
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=2
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=3
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=4
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=5
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=6
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=7
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=8
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=9
+ ovs-ofctl -O OpenFlow14 del-flows br0 table=1,in_port=10
echo >>expout "OFPT_TABLE_STATUS (OF1.4): reason=VACANCY_UP
table_desc:-
table 1:
@@ -2956,7 +3018,7 @@ ovs-appctl -t ovs-ofctl ofctl/send
051800180000000200000003000000000000000000000
check_async 3 OFPPR_ADD OFPPR_MODIFY OFPPR_DELETE
# Use OF 1.4 OFPT_SET_ASYNC to enable a patchwork of asynchronous messages.
-ovs-appctl -t ovs-ofctl ofctl/send
051c0040000000020000000800000005000100080000000200020008000000020003000800000005000400080000001c00050008000000050008000800000010
+ovs-appctl -t ovs-ofctl ofctl/send
051c0040000000020000000800000005000100080000000200020008000000020003000800000005000400080000001c00050008000000050008000800000018
check_async 4 OFPR_INVALID_TTL OFPPR_DELETE OFPRR_DELETE OFPRR_GROUP_DELETE
OFPTR_VACANCY_UP
# Set controller ID 123.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
