Thanks.

I folded in this change to update the comment:

@@ -172,14 +172,14 @@ static bool rconn_logging_connection_attempts__(const 
struct rconn *);
  * The new rconn is initially unconnected.  Use rconn_connect() or
  * rconn_connect_unreliably() to connect it.
  *
  * Connections made by the rconn will automatically negotiate an OpenFlow
  * protocol version acceptable to both peers on the connection.  The version
- * negotiated will be one of those in the 'allowed_versions' bitmap:
- * version 'x' is allowed if allowed_versions & (1 << x) is nonzero.  If
- * 'allowed_versions' is zero, then OFPUTIL_DEFAULT_VERSIONS are allowed.
- **/
+ * negotiated will be one of those in the 'allowed_versions' bitmap: version
+ * 'x' is allowed if allowed_versions & (1 << x) is nonzero.  (The underlying
+ * vconn will treat an 'allowed_versions' of 0 as OFPUTIL_DEFAULT_VERSIONS.)
+ */
 struct rconn *
 rconn_create(int probe_interval, int max_backoff, uint8_t dscp,
              uint32_t allowed_versions)
 {
     struct rconn *rc = xzalloc(sizeof *rc);


On Tue, Dec 04, 2012 at 06:19:32PM -0800, Justin Pettit wrote:
> Looks good.  Should the last sentence in the comment describing
> rconn_create be removed, since it's talking about what happens if
> 'allowed_versions' is zero?
> 
> --Justin
> 
> 
> On Dec 4, 2012, at 5:51 PM, Ben Pfaff <b...@nicira.com> wrote:
> 
> > rconn_create() was substituting OFPUTIL_DEFAULT_VERSIONS if an
> > allowed_versions of 0 was passed in.  At the same time,
> > connmgr_set_controllers() compared the adjusted value of allowed_versions
> > against the original value, saw that they were different, and concluded
> > that it should kill off and recreate the rconn with the "corrected"
> > allowed_versions.
> > 
> > This commit fixes the problem by no longer adjusting allowed_versions.
> > There is no need, because it is only used in contexts where the original
> > version is OK.
> > 
> > This problem was introduced by commit 90ef0206ea8f5a39 (connmgr:
> > Reinitialise controllers if protocols changes).
> > 
> > Bug #14126.
> > CC: Simon Horman <ho...@verge.net.au>
> > Reported-by: Natasha Gude <nata...@nicira.com>
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > lib/rconn.c |    4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/lib/rconn.c b/lib/rconn.c
> > index 45a0acd..ec661cd 100644
> > --- a/lib/rconn.c
> > +++ b/lib/rconn.c
> > @@ -218,9 +218,7 @@ rconn_create(int probe_interval, int max_backoff, 
> > uint8_t dscp,
> >     rconn_set_dscp(rc, dscp);
> > 
> >     rc->n_monitors = 0;
> > -    rc->allowed_versions = allowed_versions
> > -        ? allowed_versions
> > -        : OFPUTIL_DEFAULT_VERSIONS;
> > +    rc->allowed_versions = allowed_versions;
> > 
> >     return rc;
> > }
> > -- 
> > 1.7.10.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to