On Mon, Sep 10, 2012 at 10:47:52AM -0700, Ben Pfaff wrote:
> On Mon, Sep 10, 2012 at 09:59:28AM +0900, Simon Horman wrote:
> > 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.
>
> Thanks. It's moving in the right direction. I'm working on massaging
> it a little more toward further abstraction. (If you have time for
> anything else in the series, please don't let that hold you up.)
Here's the version I'm proposing. Please review.
--8<--------------------------cut here-------------------------->8--
From: Simon Horman <ho...@verge.net.au>
Date: Tue, 11 Sep 2012 21:47:27 -0700
Subject: [PATCH] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table
Statistics Request Messages
Co-authored-by: Ben Pfaff <b...@nicira.com>
Signed-off-by: Simon Horman <ho...@verge.net.au>
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
include/openflow/openflow-1.1.h | 24 +++++++
include/openflow/openflow-1.2.h | 5 ++
lib/ofp-util.c | 137 +++++++++++++++++++++++++++++++++++++++
lib/ofp-util.h | 9 +++
ofproto/ofproto-dpif.c | 7 +-
ofproto/ofproto-provider.h | 33 ++++++++--
ofproto/ofproto.c | 26 ++++++--
tests/ofp-print.at | 16 ++++-
8 files changed, 244 insertions(+), 13 deletions(-)
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 696c3ec..9785db4 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. */
@@ -588,6 +592,26 @@ OFP_ASSERT(sizeof(struct ofp11_flow_stats) == 48);
/* Body for ofp_stats_request of type OFPST_AGGREGATE. */
/* Identical to ofp11_flow_stats_request */
+/* Flow match fields. */
+enum ofp11_flow_match_fields {
+ OFPFMF11_IN_PORT = 1 << 0, /* Switch input port. */
+ OFPFMF11_DL_VLAN = 1 << 1, /* VLAN id. */
+ OFPFMF11_DL_VLAN_PCP = 1 << 2, /* VLAN priority. */
+ OFPFMF11_DL_TYPE = 1 << 3, /* Ethernet frame type. */
+ OFPFMF11_NW_TOS = 1 << 4, /* IP ToS (DSCP field, 6 bits). */
+ OFPFMF11_NW_PROTO = 1 << 5, /* IP protocol. */
+ OFPFMF11_TP_SRC = 1 << 6, /* TCP/UDP/SCTP source port. */
+ OFPFMF11_TP_DST = 1 << 7, /* TCP/UDP/SCTP destination port. */
+ OFPFMF11_MPLS_LABEL = 1 << 8, /* MPLS label. */
+ OFPFMF11_MPLS_TC = 1 << 9, /* MPLS TC. */
+ OFPFMF11_TYPE = 1 << 10, /* Match type. */
+ OFPFMF11_DL_SRC = 1 << 11, /* Ethernet source address. */
+ OFPFMF11_DL_DST = 1 << 12, /* Ethernet destination address. */
+ OFPFMF11_NW_SRC = 1 << 13, /* IP source address. */
+ OFPFMF11_NW_DST = 1 << 14, /* IP destination address. */
+ OFPFMF11_METADATA = 1 << 15, /* Metadata passed between tables. */
+};
+
/* Body of reply to OFPST_TABLE request. */
struct ofp11_table_stats {
uint8_t table_id; /* Identifier of table. Lower numbered tables
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..300ef4c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2933,6 +2933,143 @@ ofputil_encode_port_mod(const struct ofputil_port_mod
*pm,
return b;
}
+/* Table stats. */
+
+static void
+ofputil_put_ofp10_table_stats(const struct ofp12_table_stats *in,
+ struct ofpbuf *buf)
+{
+ struct wc_map {
+ enum ofp_flow_wildcards wc10;
+ enum oxm12_ofb_match_fields mf12;
+ };
+
+ static const struct wc_map wc_map[] = {
+ { OFPFW10_IN_PORT, OFPXMT12_OFB_IN_PORT },
+ { OFPFW10_DL_VLAN, OFPXMT12_OFB_VLAN_VID },
+ { OFPFW10_DL_SRC, OFPXMT12_OFB_ETH_SRC },
+ { OFPFW10_DL_DST, OFPXMT12_OFB_ETH_DST},
+ { OFPFW10_DL_TYPE, OFPXMT12_OFB_ETH_TYPE },
+ { OFPFW10_NW_PROTO, OFPXMT12_OFB_IP_PROTO },
+ { OFPFW10_TP_SRC, OFPXMT12_OFB_TCP_SRC },
+ { OFPFW10_TP_DST, OFPXMT12_OFB_TCP_DST },
+ { OFPFW10_NW_SRC_MASK, OFPXMT12_OFB_IPV4_SRC },
+ { OFPFW10_NW_DST_MASK, OFPXMT12_OFB_IPV4_DST },
+ { OFPFW10_DL_VLAN_PCP, OFPXMT12_OFB_VLAN_PCP },
+ { OFPFW10_NW_TOS, OFPXMT12_OFB_IP_DSCP },
+ };
+
+ struct ofp10_table_stats *out;
+ const struct wc_map *p;
+
+ out = ofpbuf_put_uninit(buf, sizeof *out);
+ out->table_id = in->table_id;
+ strcpy(out->name, in->name);
+ out->wildcards = 0;
+ for (p = wc_map; p < &wc_map[ARRAY_SIZE(wc_map)]; p++) {
+ if (in->wildcards & htonll(1ULL << p->mf12)) {
+ out->wildcards |= htonl(p->wc10);
+ }
+ }
+ out->max_entries = in->max_entries;
+ out->active_count = in->active_count;
+ put_32aligned_be64(&out->lookup_count, in->lookup_count);
+ put_32aligned_be64(&out->matched_count, in->matched_count);
+}
+
+static ovs_be32
+oxm12_to_ofp11_flow_match_fields(ovs_be64 oxm12)
+{
+ struct map {
+ enum ofp11_flow_match_fields fmf11;
+ enum oxm12_ofb_match_fields mf12;
+ };
+
+ static const struct map map[] = {
+ { OFPFMF11_IN_PORT, OFPXMT12_OFB_IN_PORT },
+ { OFPFMF11_DL_VLAN, OFPXMT12_OFB_VLAN_VID },
+ { OFPFMF11_DL_VLAN_PCP, OFPXMT12_OFB_VLAN_PCP },
+ { OFPFMF11_DL_TYPE, OFPXMT12_OFB_ETH_TYPE },
+ { OFPFMF11_NW_TOS, OFPXMT12_OFB_IP_DSCP },
+ { OFPFMF11_NW_PROTO, OFPXMT12_OFB_IP_PROTO },
+ { OFPFMF11_TP_SRC, OFPXMT12_OFB_TCP_SRC },
+ { OFPFMF11_TP_DST, OFPXMT12_OFB_TCP_DST },
+ { OFPFMF11_MPLS_LABEL, OFPXMT12_OFB_MPLS_LABEL },
+ { OFPFMF11_MPLS_TC, OFPXMT12_OFB_MPLS_TC },
+ /* I don't know what OFPFMF11_TYPE means. */
+ { OFPFMF11_DL_SRC, OFPXMT12_OFB_ETH_SRC },
+ { OFPFMF11_DL_DST, OFPXMT12_OFB_ETH_DST },
+ { OFPFMF11_NW_SRC, OFPXMT12_OFB_IPV4_SRC },
+ { OFPFMF11_NW_DST, OFPXMT12_OFB_IPV4_DST },
+ { OFPFMF11_METADATA, OFPXMT12_OFB_METADATA },
+ };
+
+ const struct map *p;
+ uint32_t fmf11;
+
+ fmf11 = 0;
+ for (p = map; p < &map[ARRAY_SIZE(map)]; p++) {
+ if (oxm12 & htonll(1ULL << p->mf12)) {
+ fmf11 |= p->fmf11;
+ }
+ }
+ return htonl(fmf11);
+}
+
+static void
+ofputil_put_ofp11_table_stats(const struct ofp12_table_stats *in,
+ struct ofpbuf *buf)
+{
+ struct ofp11_table_stats *out;
+
+ out = ofpbuf_put_uninit(buf, sizeof *out);
+ out->table_id = in->table_id;
+ strcpy(out->name, in->name);
+ out->wildcards = oxm12_to_ofp11_flow_match_fields(in->wildcards);
+ out->match = oxm12_to_ofp11_flow_match_fields(in->match);
+ out->instructions = in->instructions;
+ out->write_actions = in->write_actions;
+ out->apply_actions = in->apply_actions;
+ out->config = in->config;
+ out->max_entries = in->max_entries;
+ out->active_count = in->active_count;
+ out->lookup_count = in->lookup_count;
+ out->matched_count = in->matched_count;
+}
+
+struct ofpbuf *
+ofputil_encode_table_stats_reply(const struct ofp12_table_stats stats[], int n,
+ const struct ofp_header *request)
+{
+ struct ofpbuf *reply;
+ int i;
+
+ reply = ofpraw_alloc_stats_reply(request, n * sizeof *stats);
+
+ switch ((enum ofp_version) request->version) {
+ case OFP10_VERSION:
+ for (i = 0; i < n; i++) {
+ ofputil_put_ofp10_table_stats(&stats[i], reply);
+ }
+ break;
+
+ case OFP11_VERSION:
+ for (i = 0; i < n; i++) {
+ ofputil_put_ofp11_table_stats(&stats[i], reply);
+ }
+ break;
+
+ case OFP12_VERSION:
+ ofpbuf_put(reply, stats, n * sizeof *stats);
+ break;
+
+ default:
+ NOT_REACHED();
+ }
+
+ return reply;
+}
+
/* ofputil_flow_monitor_request */
/* Converts an NXST_FLOW_MONITOR request in 'msg' into an abstract
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index e3a93c9..a860d87 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -450,6 +450,15 @@ enum ofperr ofputil_decode_port_mod(const struct
ofp_header *,
struct ofpbuf *ofputil_encode_port_mod(const struct ofputil_port_mod *,
enum ofputil_protocol);
+/* Abstract table stats.
+ *
+ * For now we use ofp12_table_stats as a superset of the other protocol
+ * versions' table stats. */
+
+struct ofpbuf *ofputil_encode_table_stats_reply(
+ const struct ofp12_table_stats[], int n,
+ const struct ofp_header *request);
+
/* Abstract nx_flow_monitor_request. */
struct ofputil_flow_monitor_request {
uint32_t id;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 1289025..9f7acd1 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1158,7 +1158,7 @@ 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)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
struct dpif_dp_stats s;
@@ -1166,9 +1166,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..0ade586 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -464,7 +464,17 @@ struct ofproto_class {
*
* - 'name' to "table#" where # is the table ID.
*
- * - 'wildcards' to OFPFW10_ALL.
+ * - 'match' and 'wildcards' to OFPXMT12_MASK.
+ *
+ * - 'write_actions' and 'apply_actions' to OFPAT12_OUTPUT.
+ *
+ * - 'write_setfields' and 'apply_setfields' to OFPXMT12_MASK.
+ *
+ * - 'metadata_match' and 'metadata_write' to UINT64_MAX.
+ *
+ * - 'instructions' to OFPIT11_ALL.
+ *
+ * - 'config' to OFPTC11_TABLE_MISS_MASK.
*
* - 'max_entries' to 1,000,000.
*
@@ -480,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.
*
@@ -489,10 +514,10 @@ 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
- * network byte order.
+ * 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);
/* ## ---------------- ## */
/* ## ofport Functions ## */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9b235e9..47cf22b 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2242,16 +2242,30 @@ 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;
- msg = ofpraw_alloc_stats_reply(request, sizeof *ots * p->n_tables);
- ots = ofpbuf_put_zeros(msg, sizeof *ots * p->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).
+ */
+ ots = xcalloc(p->n_tables, sizeof *ots);
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].match = htonll(OFPXMT12_MASK);
+ ots[i].wildcards = htonll(OFPXMT12_MASK);
+ ots[i].write_actions = htonl(OFPAT12_OUTPUT);
+ ots[i].apply_actions = htonl(OFPAT12_OUTPUT);
+ ots[i].write_setfields = htonll(OFPXMT12_MASK);
+ ots[i].apply_setfields = htonll(OFPXMT12_MASK);
+ ots[i].metadata_match = htonll(UINT64_MAX);
+ ots[i].metadata_write = htonll(UINT64_MAX);
+ ots[i].instructions = htonl(OFPIT11_ALL);
+ ots[i].config = htonl(OFPTC11_TABLE_MISS_MASK);
ots[i].max_entries = htonl(1000000); /* An arbitrary big number. */
ots[i].active_count = htonl(classifier_count(&p->tables[i].cls));
}
@@ -2270,7 +2284,11 @@ handle_table_stats_request(struct ofconn *ofconn,
}
}
+ msg = ofputil_encode_table_stats_reply(ots, p->n_tables, request);
ofconn_send_reply(ofconn, msg);
+
+ free(ots);
+
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
