[ovs-dev] [classifier-opt 09/28] ofproto: Move 'max_ports' from ofproto-dpif.c to ofproto.c.
Ben Pfaff
blp at nicira.com
Fri Jul 20 23:25:06 UTC 2012
This allows port numbers in actions and elsewhere in OpenFlow messages to
be checked at a higher level.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
ofproto/ofproto-dpif.c | 16 +++++++---------
ofproto/ofproto-provider.h | 30 +++++++++++++++++++-----------
ofproto/ofproto.c | 26 ++++++++++++++++++++++++++
3 files changed, 52 insertions(+), 20 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b95179a..f1d2ae0 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -588,7 +588,6 @@ struct ofproto_dpif {
struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'. */
struct ofproto up;
struct dpif *dpif;
- int max_ports;
/* Special OpenFlow rules. */
struct rule_dpif *miss_rule; /* Sends flow table misses to controller. */
@@ -734,6 +733,7 @@ construct(struct ofproto *ofproto_)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
const char *name = ofproto->up.name;
+ int max_ports;
int error;
int i;
@@ -743,7 +743,9 @@ construct(struct ofproto *ofproto_)
return error;
}
- ofproto->max_ports = dpif_get_max_ports(ofproto->dpif);
+ max_ports = dpif_get_max_ports(ofproto->dpif);
+ ofproto_init_max_ports(ofproto_, MIN(max_ports, OFPP_MAX));
+
ofproto->n_matches = 0;
dpif_flow_flush(ofproto->dpif);
@@ -4593,7 +4595,7 @@ rule_construct(struct rule *rule_)
enum ofperr error;
error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
- &rule->up.cr.flow, ofproto->max_ports);
+ &rule->up.cr.flow, ofproto->up.max_ports);
if (error) {
return error;
}
@@ -4703,7 +4705,7 @@ rule_modify_actions(struct rule *rule_)
enum ofperr error;
error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
- &rule->up.cr.flow, ofproto->max_ports);
+ &rule->up.cr.flow, ofproto->up.max_ports);
if (error) {
ofoperation_complete(rule->up.pending, error);
return;
@@ -6359,11 +6361,7 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet,
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
enum ofperr error;
- if (flow->in_port >= ofproto->max_ports && flow->in_port < OFPP_MAX) {
- return OFPERR_NXBRC_BAD_IN_PORT;
- }
-
- error = ofpacts_check(ofpacts, ofpacts_len, flow, ofproto->max_ports);
+ error = ofpacts_check(ofpacts, ofpacts_len, flow, ofproto->up.max_ports);
if (!error) {
struct odputil_keybuf keybuf;
struct dpif_flow_stats stats;
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 6eef106..2f62473 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -61,6 +61,7 @@ struct ofproto {
/* Datapath. */
struct hmap ports; /* Contains "struct ofport"s. */
struct shash port_by_name;
+ uint16_t max_ports; /* Max possible OpenFlow port num, plus one. */
/* Flow tables. */
struct oftable *tables;
@@ -93,6 +94,7 @@ struct ofproto {
};
void ofproto_init_tables(struct ofproto *, int n_tables);
+void ofproto_init_max_ports(struct ofproto *, uint16_t max_ports);
struct ofproto *ofproto_lookup(const char *name);
struct ofport *ofproto_get_port(const struct ofproto *, uint16_t ofp_port);
@@ -365,6 +367,11 @@ struct ofproto_class {
* ->construct() should delete flows from the underlying datapath, if
* necessary, rather than populating the tables.
*
+ * If the ofproto knows the maximum port number that the datapath can have,
+ * then it can call ofproto_init_max_ports(). If it does so, then the
+ * client will ensure that the actions it allows to be used through
+ * OpenFlow do not refer to ports above that maximum number.
+ *
* Only one ofproto instance needs to be supported for any given datapath.
* If a datapath is already open as part of one "ofproto", then another
* attempt to "construct" the same datapath as part of another ofproto is
@@ -946,17 +953,18 @@ struct ofproto_class {
*
* flow->in_port comes from the OpenFlow OFPT_PACKET_OUT message. The
* implementation should reject invalid flow->in_port values by returning
- * OFPERR_NXBRC_BAD_IN_PORT. For consistency, the implementation should
- * consider valid for flow->in_port any value that could possibly be seen
- * in a packet that it passes to connmgr_send_packet_in(). Ideally, even
- * an implementation that never generates packet-ins (e.g. due to hardware
- * limitations) should still allow flow->in_port values for every possible
- * physical port and OFPP_LOCAL. The only virtual ports (those above
- * OFPP_MAX) that the caller will ever pass in as flow->in_port, other than
- * OFPP_LOCAL, are OFPP_NONE and OFPP_CONTROLLER. The implementation
- * should allow both of these, treating each of them as packets generated
- * by the controller as opposed to packets originating from some switch
- * port.
+ * OFPERR_NXBRC_BAD_IN_PORT. (If the implementation called
+ * ofproto_init_max_ports(), then the client will reject these ports
+ * itself.) For consistency, the implementation should consider valid for
+ * flow->in_port any value that could possibly be seen in a packet that it
+ * passes to connmgr_send_packet_in(). Ideally, even an implementation
+ * that never generates packet-ins (e.g. due to hardware limitations)
+ * should still allow flow->in_port values for every possible physical port
+ * and OFPP_LOCAL. The only virtual ports (those above OFPP_MAX) that the
+ * caller will ever pass in as flow->in_port, other than OFPP_LOCAL, are
+ * OFPP_NONE and OFPP_CONTROLLER. The implementation should allow both of
+ * these, treating each of them as packets generated by the controller as
+ * opposed to packets originating from some switch port.
*
* (Ordinarily the only effect of flow->in_port is on output actions that
* involve the input port, such as actions that output to OFPP_IN_PORT,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 467dfda..cbcf0d2 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -389,6 +389,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
ofproto->frag_handling = OFPC_FRAG_NORMAL;
hmap_init(&ofproto->ports);
shash_init(&ofproto->port_by_name);
+ ofproto->max_ports = OFPP_MAX;
ofproto->tables = NULL;
ofproto->n_tables = 0;
ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name);
@@ -421,6 +422,9 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
return 0;
}
+/* Must be called (only) by an ofproto implementation in its constructor
+ * function. See the large comment on 'construct' in struct ofproto_class for
+ * details. */
void
ofproto_init_tables(struct ofproto *ofproto, int n_tables)
{
@@ -436,6 +440,24 @@ ofproto_init_tables(struct ofproto *ofproto, int n_tables)
}
}
+/* To be optionally called (only) by an ofproto implementation in its
+ * constructor function. See the large comment on 'construct' in struct
+ * ofproto_class for details.
+ *
+ * Sets the maximum number of ports to 'max_ports'. The ofproto generic layer
+ * will then ensure that actions passed into the ofproto implementation will
+ * not refer to OpenFlow ports numbered 'max_ports' or higher. If this
+ * function is not called, there will be no such restriction.
+ *
+ * Reserved ports numbered OFPP_MAX and higher are special and not subject to
+ * the 'max_ports' restriction. */
+void
+ofproto_init_max_ports(struct ofproto *ofproto, uint16_t max_ports)
+{
+ assert(max_ports <= OFPP_MAX);
+ ofproto->max_ports = max_ports;
+}
+
uint64_t
ofproto_get_datapath_id(const struct ofproto *ofproto)
{
@@ -2100,6 +2122,10 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_packet_out *opo)
if (error) {
goto exit_free_ofpacts;
}
+ if (po.in_port >= p->max_ports && po.in_port < OFPP_MAX) {
+ error = OFPERR_NXBRC_BAD_IN_PORT;
+ goto exit_free_ofpacts;
+ }
/* Get payload. */
if (po.buffer_id != UINT32_MAX) {
--
1.7.2.5
More information about the dev
mailing list