[ovs-dev] [PATCH 04/27] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table Statistics Request Messages
Simon Horman
horms at verge.net.au
Fri Sep 7 00:06:23 UTC 2012
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 <horms at 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?
Hi Ben,
could you take a moment to look over this?
I'd like to know if this approach is appropriate.
> From: Simon Horman <horms at verge.net.au>
>
> [PATCH] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table Statistics Request Messages
>
> Signed-off-by: Simon Horman <horms at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
More information about the dev
mailing list