On Sat, Sep 08, 2012 at 12:15:53PM +0900, Simon Horman wrote:
> On Fri, Sep 07, 2012 at 10:33:00AM -0700, Ben Pfaff wrote:
> > On Fri, Aug 31, 2012 at 05:42:48PM +0900, Simon Horman wrote:
> > > 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.
> > >
> > > is this closer to what you would like?
> >
> > Much closer. I think that if you move the conversion code from
> > ofproto.c to single-purpose functions in lib/ofp-util.c then it's what
> > I had in mind.
>
> Sure, I'll make that change and see how it looks.
>
> > (I did mean to actually invent a new struct for that,
> > but it seems like the OF1.2 version is good enough for now.)
>
> (me too)
Hi Ben,
The patch below attempts to implement your suggestion.
I have created ofputil function to both convert to Open Flow version
specific table stats structures, and create Open Flow version specific
table stats reply messages. This much seems clean. Going beyond that
seemed to get a bit messy.
From: Simon Horman <ho...@verge.net.au>
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 2)
* Don't make use of a union of different ofpX_table_stats structures.
Rather, use ofp12_table_stats and convert using new ofputil functions
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 ++
lib/ofp-util.c | 102 +++++++++++++++++++++++++++++++++++++++
lib/ofp-util.h | 7 +++
ofproto/ofproto-dpif.c | 8 +--
ofproto/ofproto-provider.h | 40 +++++++++++++--
ofproto/ofproto.c | 64 ++++++++++++++++++++++--
tests/ofp-print.at | 16 +++++-
8 files changed, 235 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/lib/ofp-util.c b/lib/ofp-util.c
index 1976b94..7a3f9c6 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3763,3 +3763,105 @@ ofputil_parse_key_value(char **stringp, char **keyp,
char **valuep)
*valuep = value;
return true;
}
+
+
+/* Converts a generic table stats structure into an Open Flow 1.0 table
+ * stats structure.
+ *
+ * The code makes use of struct ofp12_table_stats, which is the Open Flow 1.2
+ * structure, as the generic structure as it has all the required fields
+ * and it seems unnecessary to create a separate generic structure at this
+ * time.
+ */
+static void
+ofputil_table_stats_to_of10_table_stats(struct ofp12_table_stats *in,
+ int n_tables,
+ struct ofp10_table_stats *out)
+{
+ int i;
+
+ for (i = 0; i < n_tables; i++) {
+ out[i].table_id = in[i].table_id;
+ strcpy(out[i].name, in[i].name);
+ out[i].wildcards = htonl(ntohll(in[i].wildcards));
+ out[i].max_entries = in[i].max_entries;
+ out[i].active_count = in[i].active_count;
+ put_32aligned_be64(&out[i].lookup_count, in[i].lookup_count);
+ put_32aligned_be64(&out[i].matched_count, in[i].matched_count);
+ }
+}
+
+/* Converts a generic table stats structure into an Open Flow 1.0 table
+ * stats response message.
+ *
+ * The code makes use of struct ofp12_table_stats, which is the Open Flow 1.2
+ * structure, as the generic structure as it has all the required fields
+ * and it seems unnecessary to create a separate generic structure at this
+ * time.
+ */
+struct ofpbuf *
+ofputil_table_stats_to_of10_msg(struct ofp12_table_stats *in,
+ const struct ofp_header *request, int n_tables)
+{
+ struct ofpbuf *msg;
+ struct ofp10_table_stats *ots10;
+
+ msg = ofpraw_alloc_stats_reply(request, sizeof *ots10 * n_tables);
+ ots10 = ofpbuf_put_zeros(msg, sizeof *ots10 * n_tables);
+ ofputil_table_stats_to_of10_table_stats(in, n_tables, ots10);
+
+ return msg;
+}
+
+/* Converts a generic table stats structure into an Open Flow 1.1 table
+ * stats structure.
+ *
+ * The code makes use of struct ofp12_table_stats, which is the Open Flow 1.2
+ * structure, as the generic structure as it has all the required fields
+ * and it seems unnecessary to create a separate generic structure at this
+ * time.
+ */
+static void
+ofputil_table_stats_to_of11_table_stats(struct ofp12_table_stats *in,
+ int n_tables,
+ struct ofp11_table_stats *out)
+{
+ int i;
+
+ for (i = 0; i < n_tables; i++) {
+ out[i].table_id = in[i].table_id;
+ strcpy(out[i].name, in[i].name);
+ out[i].wildcards = htonl(ntohll(in[i].wildcards));
+ out[i].match = htonl(ntohll(in[i].match));
+ out[i].instructions = in[i].instructions;
+ out[i].write_actions = in[i].write_actions;
+ out[i].apply_actions = in[i].apply_actions;
+ out[i].config = in[i].config;
+ out[i].max_entries = in[i].max_entries;
+ out[i].active_count = in[i].active_count;
+ out[i].lookup_count = in[i].lookup_count;
+ out[i].matched_count = in[i].matched_count;
+ }
+}
+
+/* Converts a generic table stats structure into an Open Flow 1.1 table
+ * stats response message.
+ *
+ * The code makes use of struct ofp12_table_stats, which is the Open Flow 1.2
+ * structure, as the generic structure as it has all the required fields
+ * and it seems unnecessary to create a separate generic structure at this
+ * time.
+ */
+struct ofpbuf *
+ofputil_table_stats_to_of11_msg(struct ofp12_table_stats *ots,
+ const struct ofp_header *request, int n_tables)
+{
+ struct ofpbuf *msg;
+ struct ofp11_table_stats *ots11;
+
+ msg = ofpraw_alloc_stats_reply(request, sizeof *ots11 * n_tables);
+ ots11 = ofpbuf_put_zeros(msg, sizeof *ots11 * n_tables);
+ ofputil_table_stats_to_of11_table_stats(ots, n_tables, ots11);
+
+ return msg;
+}
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index e3a93c9..6fa0c8e 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -614,4 +614,11 @@ union ofp_action *ofputil_actions_clone(const union
ofp_action *, size_t n);
/* Handy utility for parsing flows and actions. */
bool ofputil_parse_key_value(char **stringp, char **keyp, char **valuep);
+struct ofpbuf *ofputil_table_stats_to_of10_msg(struct ofp12_table_stats *ots,
+ const struct ofp_header
*request,
+ int n_tables);
+struct ofpbuf *ofputil_table_stats_to_of11_msg(struct ofp12_table_stats *ots,
+ const struct ofp_header
*request,
+ int n_tables);
+
#endif /* ofp-util.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 1289025..9ea6c01 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1158,7 +1158,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;
@@ -1166,9 +1167,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 fb7db23..d9a90b2 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -464,7 +464,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.
*
@@ -480,6 +498,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.
*
@@ -489,10 +522,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 9b235e9..b1376f9 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2242,21 +2242,52 @@ 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;
+ struct ofpbuf *(conv)(struct ofp12_table_stats *ots,
+ const struct ofp_header *request,
+ int n_tables);
+ /* 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];
@@ -2270,6 +2301,33 @@ 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 ofpbuf *new_msg;
+
+ new_msg = ofputil_table_stats_to_of11_msg(ots, request, p->n_tables);
+ ofpbuf_delete(msg);
+ msg = new_msg;
+ break;
+ }
+
+ case OFP10_VERSION: {
+ struct ofpbuf *new_msg;
+
+ new_msg = ofputil_table_stats_to_of10_msg(ots, request, p->n_tables);
+ 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 9844592..a8ace84 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.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
