On Oct 18, 2012, at 9:30 PM, Ben Pfaff <b...@nicira.com> wrote: > The new comment on the ->port_add member function of ofproto_class > implies that the caller might pass in a NULL 'ofp_portp'. I don't think > that the implementation should have to bother with that possibility, > because ofproto_port_add(), by design, always passes a non-null value, > so I'd revise the comment.
Updated. > struct if_cfg, in bridge.c, now ends with a spurious blank line. Whoops. > It looks like the signed 64-bit value from the database gets truncated > down to an unsigned 16-bit value in iface_do_create(). Actually in > iface_create() too, in the initialization of 'ofp_port'. I'd check that > it's in the valid range [1,OFPP_MAX) and use OFPP_NONE if not. As you suggested later, I defined constraints in the database definition. Is that sufficient? > In iface_create(), the variable name 'result' is fairly vague, maybe > 'ok' instead? 'ok' > In vswitch.xml, I'm not sure that it makes sense to say that the port > number can be greater than 0xff00. I think that OpenFlow reserves > those. I guess I'd be inclined to say something more like: > > <p>Requested OpenFlow port number for this interface. The port > number must be between 1 and 65279, inclusive. Some datapaths > cannot satisfy all requests for particular port numbers. When > this column is empty or the request cannot be fulfilled, the > system will choose a free port. The <ref column="ofport"/> > column reports + the assigned OpenFlow port number.</p> Sounds good. > One could limit the allowed values at the database level, in the schema > itself. I didn't originally do this, since the limit varies by datapath implementation. However, as you noted, there are real limits defined by OpenFlow, so I went ahead and put them in there. > I think that this implementation generally considers the ofport > request_value only if it is set in the same transaction that creates the > port. That is, changing ofport_request, or setting it when it was > initially unset, will not change the OpenFlow port number. Perhaps this > should be noted in vswitch.xml too. Done. --Justin _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
