On Wed, May 11, 2011 at 05:06:50PM -0700, Ethan Jackson wrote:
> > @@ -879,16 +887,15 @@ ofproto_delete_flow(struct ofproto *ofproto, const
> > struct cls_rule *target)
> > ?{
> > ? ? struct rule *rule;
> >
> > - ? ?rule = rule_from_cls_rule(classifier_find_rule_exactly(&ofproto->cls,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? target));
> > + ? ?rule = rule_from_cls_rule(classifier_find_rule_exactly(
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&ofproto->tables[0], target));
> > ? ? ofproto_rule_destroy(rule);
> > ?}
>
> I don't fully understand this function. Why does it assume that
> 'target' is in table 0?
ofproto_add_flow(), ofproto_delete_flow() and ofproto_flush_flows()
are only used by in-band control and fail-open. So far, I haven't
taught those to do anything with multiple tables. They only add rules
to table 0 (and so they only need to remove them from table 0).
When we know more about how people use multiple tables, we can work on
making in-band control and fail-open smarter.
> > + ? ? * ->construct() should initialize the base 'n_tables' member to the
> > number
> > + ? ? * of flow tables supported by the datapath (between 1 and 254,
> > inclusive),
> > + ? ? * initialize the base 'tables' membeer with space for one classifier
> > per
>
> member
Thanks, I fixed this.
> > +.IP \fBtable=\fInumber\fR
> > +If specified, limits the flow manipulation and flow dump commands to
> > +only apply to the table with the given \fInumber\fR.
> > +\fInumber\fR is a number between 0 and 254, inclusive.
>
> Should this be between 1 and 254?
Table 0 is a valid table, and the only one supported by the software
switch. (I think I said something in a meeting the other day about
OpenFlow having a hard limit of 254 tables. I was wrong. The limit
is 255 tables. Value 0xff is reserved.)
I updated the comments:
--8<--------------------------cut here-------------------------->8--
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index dd7d229..f0fc4ec 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -871,7 +871,7 @@ ofproto_port_del(struct ofproto *ofproto, uint16_t ofp_port)
return error;
}
-/* Adds a flow to the OpenFlow flow table in 'p' that matches 'cls_rule' and
+/* Adds a flow to OpenFlow flow table 0 in 'p' that matches 'cls_rule' and
* performs the 'n_actions' actions in 'actions'. The new flow will not
* timeout.
*
@@ -879,7 +879,9 @@ ofproto_port_del(struct ofproto *ofproto, uint16_t ofp_port)
* (0...65535, inclusive) then the flow will be visible to OpenFlow
* controllers; otherwise, it will be hidden.
*
- * The caller retains ownership of 'cls_rule' and 'actions'. */
+ * The caller retains ownership of 'cls_rule' and 'actions'.
+ *
+ * This is a helper function for in-band control and fail-open. */
void
ofproto_add_flow(struct ofproto *p, const struct cls_rule *cls_rule,
const union ofp_action *actions, size_t n_actions)
@@ -888,6 +890,10 @@ ofproto_add_flow(struct ofproto *p, const struct cls_rule
*cls_rule,
rule_create(p, cls_rule, actions, n_actions, 0, 0, 0, false, &rule);
}
+/* Searches for a rule with matching criteria exactly equal to 'target' in
+ * ofproto's table 0 and, if it finds one, deletes it.
+ *
+ * This is a helper function for in-band control and fail-open. */
void
ofproto_delete_flow(struct ofproto *ofproto, const struct cls_rule *target)
{
@@ -920,6 +926,8 @@ ofproto_flush_flows__(struct ofproto *ofproto)
}
}
+/* Deletes all of the flows from all of ofproto's flow tables, then
+ * reintroduces rules required by in-band control and fail open. */
void
ofproto_flush_flows(struct ofproto *ofproto)
{
diff --git a/ofproto/private.h b/ofproto/private.h
index c829415..f2e3204 100644
--- a/ofproto/private.h
+++ b/ofproto/private.h
@@ -243,7 +243,7 @@ struct ofproto_class {
*
* ->construct() should initialize the base 'n_tables' member to the number
* of flow tables supported by the datapath (between 1 and 254, inclusive),
- * initialize the base 'tables' membeer with space for one classifier per
+ * initialize the base 'tables' member with space for one classifier per
* table, and initialize each classifier with classifier_init. Each flow
* table should be initially empty, so ->construct() should delete flows
* from the underlying datapath, if necessary, rather than populating the
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
