On Thu, Aug 30, 2012 at 10:02:51AM +0900, Simon Horman wrote:
> On Wed, Aug 29, 2012 at 10:07:55AM -0700, Ben Pfaff wrote:
> > On Tue, Aug 21, 2012 at 01:55:36PM +0900, Simon Horman wrote:
> > > Signed-off-by: Simon Horman <ho...@verge.net.au>
> >
> > I'm not really happy with this approach. It seems fairly awkward. I
> > would much rather define some "struct ofputil_*" that has a superset of
> > the 1.[012] table stats and define converter functions.
>
> Sure, I'll see what I can do.
> I do agree that the approach I have taken is somewhat awkward.
Hi Ben,
is this closer to what you would like?
From: Simon Horman <ho...@verge.net.au>
[PATCH] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table Statistics
Request Messages
Signed-off-by: Simon Horman <ho...@verge.net.au>
---
v13 (draft)
* Don't make use of a union of different ofpX_table_stats structures.
Rather, use ofp12_table_stats and convert as necessary.
v12
* No change
v11
* No change
v10
* No change
v9
* Set wildcards, match, write_setfields and apply_setfields based
on a bitmap of (1 << OFPXMT_*)
v8
* Manual rebase
* Make use of enum ofp_version
* Add ofp-tests
v7
* Omitted
v6
* No change
v5
* Manual rebase
* Add OFPST_TABLE entry for Open Flow 1.1 and 1.2 to ofputil_msg_types,
this wires-up decoding of table statistics messages.
v4
* Initial post
table test
---
include/openflow/openflow-1.1.h | 4 ++
include/openflow/openflow-1.2.h | 5 +++
ofproto/ofproto-dpif.c | 8 ++--
ofproto/ofproto-provider.h | 40 ++++++++++++++++--
ofproto/ofproto.c | 94 +++++++++++++++++++++++++++++++++++++++--
tests/ofp-print.at | 16 ++++++-
6 files changed, 156 insertions(+), 11 deletions(-)
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 696c3ec..5592520 100644
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -281,6 +281,10 @@ enum ofp11_instruction_type {
OFPIT11_EXPERIMENTER = 0xFFFF /* Experimenter instruction */
};
+#define OFPIT11_ALL OFPIT11_GOTO_TABLE | OFPIT11_WRITE_METADATA | \
+ OFPIT11_WRITE_ACTIONS | OFPIT11_APPLY_ACTIONS | \
+ OFPIT11_CLEAR_ACTIONS
+
#define OFP11_INSTRUCTION_ALIGN 8
/* Generic ofp_instruction structure. */
diff --git a/include/openflow/openflow-1.2.h b/include/openflow/openflow-1.2.h
index 0a73ed1..64bc993 100644
--- a/include/openflow/openflow-1.2.h
+++ b/include/openflow/openflow-1.2.h
@@ -106,8 +106,13 @@ enum oxm12_ofb_match_fields {
OFPXMT12_OFB_IPV6_ND_TLL, /* Target link-layer for ND. */
OFPXMT12_OFB_MPLS_LABEL, /* MPLS label. */
OFPXMT12_OFB_MPLS_TC, /* MPLS TC. */
+
+ /* End Marker */
+ OFPXMT12_OFB_MAX,
};
+#define OFPXMT12_MASK ((1ULL << OFPXMT12_OFB_MAX) - 1)
+
/* OXM implementation makes use of NXM as they are the same format
* with different field definitions
*/
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 78cb186..2dd7eee 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1156,7 +1156,8 @@ get_features(struct ofproto *ofproto_ OVS_UNUSED,
}
static void
-get_tables(struct ofproto *ofproto_, struct ofp10_table_stats *ots)
+get_tables(struct ofproto *ofproto_, struct ofp12_table_stats *ots,
+ enum ofp_version ofp_version OVS_UNUSED)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
struct dpif_dp_stats s;
@@ -1164,9 +1165,8 @@ get_tables(struct ofproto *ofproto_, struct
ofp10_table_stats *ots)
strcpy(ots->name, "classifier");
dpif_get_dp_stats(ofproto->dpif, &s);
- put_32aligned_be64(&ots->lookup_count, htonll(s.n_hit + s.n_missed));
- put_32aligned_be64(&ots->matched_count,
- htonll(s.n_hit + ofproto->n_matches));
+ ots->lookup_count = htonll(s.n_hit + s.n_missed);
+ ots->matched_count = htonll(s.n_hit + ofproto->n_matches);
}
static struct ofport *
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 15dc347..690a933 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -456,7 +456,25 @@ struct ofproto_class {
*
* - 'name' to "table#" where # is the table ID.
*
- * - 'wildcards' to OFPFW10_ALL.
+ * - 'wildcards' to OFPFW10_ALL (OpenFlow 1.0) or
+ * OFPFW11_ALL (OpenFlow 1.1 and 1.2).
+ *
+ * - 'instructions' to OFPIT11_ALL (OpenFlow 1.1 and 1.2).
+ * Not used in OpenFlow 1.0.
+ *
+ * - 'write_actions' to OFPAT11_OUTPUT (OpenFlow 1.1) or
+ * OFPAT12_OUTPUT (OpenFlow 1.2).
+ * Not present in OpenFLow 1.0.
+ *
+ * - 'apply_actions' to OFPAT11_OUTPUT (OpenFlow 1.1) or
+ * OFPAT12_OUTPUT (OpenFlow 1.2).
+ * Not used in OpenFLow 1.0.
+ *
+ * - 'write_setfields' to OFPXMT12_SUPPORTED (OpenFlow 1.2).
+ * Not used in OpenFLow 1.0 or 1.1.
+ *
+ * - 'apply_setfields' to OFPXMT12_SUPPORTED (OpenFlow 1.2).
+ * Not used in OpenFlow 1.0 or 1.1.
*
* - 'max_entries' to 1,000,000.
*
@@ -472,6 +490,21 @@ struct ofproto_class {
* - 'wildcards' to the set of wildcards actually supported by the table
* (if it doesn't support all OpenFlow wildcards).
*
+ * - 'instructions' to set the instructions actually supported by
+ * the table.
+ *
+ * - 'write_actions' to set the write actions actually supported by
+ * the table (if it doesn't support all OpenFlow actions).
+ *
+ * - 'apply_actions' to set the apply actions actually supported by
+ * the table (if it doesn't support all OpenFlow actions).
+ *
+ * - 'write_setfields' to set the write setfields actually supported by
+ * the table.
+ *
+ * - 'apply_setfields' to set the apply setfields actually supported by
+ * the table.
+ *
* - 'max_entries' to the maximum number of flows actually supported by
* the hardware.
*
@@ -481,10 +514,11 @@ struct ofproto_class {
* - 'matched_count' to the number of packets looked up in this flow
* table so far that matched one of the flow entries.
*
- * Keep in mind that all of the members of struct ofp10_table_stats are in
+ * Keep in mind that all of the members of struct ofp12_table_stats are in
* network byte order.
*/
- void (*get_tables)(struct ofproto *ofproto, struct ofp10_table_stats *ots);
+ void (*get_tables)(struct ofproto *ofproto, struct ofp12_table_stats *ots,
+ enum ofp_version ofp_version);
/* ## ---------------- ## */
/* ## ofport Functions ## */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 5c9ab9d..4fe0ca1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2208,21 +2208,49 @@ handle_table_stats_request(struct ofconn *ofconn,
const struct ofp_header *request)
{
struct ofproto *p = ofconn_get_ofproto(ofconn);
- struct ofp10_table_stats *ots;
+ struct ofp12_table_stats *ots;
struct ofpbuf *msg;
size_t i;
+ /* Set up default values.
+ *
+ * ofp12_table_stats is used as a generic structure as
+ * it is able to hold all the fields for ofp10_table_stats
+ * and ofp11_table_stats (and of course itself).
+ */
msg = ofpraw_alloc_stats_reply(request, sizeof *ots * p->n_tables);
ots = ofpbuf_put_zeros(msg, sizeof *ots * p->n_tables);
for (i = 0; i < p->n_tables; i++) {
ots[i].table_id = i;
sprintf(ots[i].name, "table%zu", i);
- ots[i].wildcards = htonl(OFPFW10_ALL);
ots[i].max_entries = htonl(1000000); /* An arbitrary big number. */
ots[i].active_count = htonl(classifier_count(&p->tables[i].cls));
+ ots[i].config = ots[i].apply_actions = htonl(0);
+
+ switch (request->version) {
+ case OFP12_VERSION:
+ ots[i].wildcards = ots[i].match = ots[i].write_setfields =
+ ots[i].apply_setfields = htonll(OFPXMT12_MASK);
+ ots[i].write_actions = ots[i].apply_actions =
htonl(OFPAT12_OUTPUT);
+ ots[i].instructions = htonl(OFPIT11_ALL);
+ break;
+
+ case OFP11_VERSION:
+ ots[i].wildcards = ots[i].match = htonll(OFPFW11_ALL);
+ ots[i].instructions = htonl(OFPIT11_ALL);
+ ots[i].write_actions = ots[i].apply_actions =
htonl(OFPAT11_OUTPUT);
+ break;
+
+ case OFP10_VERSION:
+ ots[i].wildcards = htonll(OFPFW10_ALL);
+ break;
+
+ default:
+ NOT_REACHED();
+ }
}
- p->ofproto_class->get_tables(p, ots);
+ p->ofproto_class->get_tables(p, ots, request->version);
for (i = 0; i < p->n_tables; i++) {
const struct oftable *table = &p->tables[i];
@@ -2236,6 +2264,66 @@ handle_table_stats_request(struct ofconn *ofconn,
}
}
+ /* Convert from ofp12_table_stats as necessary */
+ switch (request->version) {
+ case OFP12_VERSION:
+ break;
+
+ case OFP11_VERSION: {
+ struct ofp11_table_stats *ots11;
+ struct ofpbuf *new_msg;
+
+ new_msg = ofpraw_alloc_stats_reply(request,
+ sizeof *ots11 * p->n_tables);
+ ots11 = ofpbuf_put_zeros(new_msg, sizeof *ots11 * p->n_tables);
+
+ for (i = 0; i < p->n_tables; i++) {
+ ots11[i].table_id = ots[i].table_id;
+ strcpy(ots11[i].name, ots[i].name);
+ ots11[i].wildcards = htonl(ntohll(ots[i].wildcards));
+ ots11[i].match = htonl(ntohll(ots[i].match));
+ ots11[i].instructions = ots[i].instructions;
+ ots11[i].write_actions = ots[i].write_actions;
+ ots11[i].apply_actions = ots[i].apply_actions;
+ ots11[i].config = ots[i].config;
+ ots11[i].max_entries = ots[i].max_entries;
+ ots11[i].active_count = ots[i].active_count;
+ ots11[i].lookup_count = ots[i].lookup_count;
+ ots11[i].matched_count = ots[i].matched_count;
+ }
+
+ ofpbuf_delete(msg);
+ msg = new_msg;
+ break;
+ }
+
+ case OFP10_VERSION: {
+ struct ofp10_table_stats *ots10;
+ struct ofpbuf *new_msg;
+
+ new_msg = ofpraw_alloc_stats_reply(request,
+ sizeof *ots10 * p->n_tables);
+ ots10 = ofpbuf_put_zeros(new_msg, sizeof *ots10 * p->n_tables);
+
+ for (i = 0; i < p->n_tables; i++) {
+ ots10[i].table_id = ots[i].table_id;
+ strcpy(ots10[i].name, ots[i].name);
+ ots10[i].wildcards = htonl(ntohll(ots[i].wildcards));
+ ots10[i].max_entries = ots[i].max_entries;
+ ots10[i].active_count = ots[i].active_count;
+ put_32aligned_be64(&ots10[i].lookup_count, ots[i].lookup_count);
+ put_32aligned_be64(&ots10[i].matched_count, ots[i].matched_count);
+ }
+
+ ofpbuf_delete(msg);
+ msg = new_msg;
+ break;
+ }
+
+ default:
+ NOT_REACHED();
+ }
+
ofconn_send_reply(ofconn, msg);
return 0;
}
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 3c55d91..ca7cf6b 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -794,13 +794,27 @@ OFPST_AGGREGATE reply (OF1.2) (xid=0x2): packet_count=121
byte_count=19279 flow_
])
AT_CLEANUP
-AT_SETUP([OFPST_TABLE request])
+AT_SETUP([OFPST_TABLE request - OF1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
AT_CHECK([ovs-ofctl ofp-print "0110000c0000000100030000"], [0], [dnl
OFPST_TABLE request (xid=0x1):
])
AT_CLEANUP
+AT_SETUP([OFPST_TABLE request - OF1.1])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "02120010000000020003000000000000"], [0], [dnl
+OFPST_TABLE request (OF1.1) (xid=0x2):
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_TABLE request - OF1.2])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "03120010000000020003000000000000"], [0], [dnl
+OFPST_TABLE request (OF1.2) (xid=0x2):
+])
+AT_CLEANUP
+
AT_SETUP([OFPST_TABLE reply - OF1.0])
AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
AT_CHECK([ovs-ofctl ofp-print "\
--
1.7.10.2.484.gcd07cc5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
