On Wed, Jan 11, 2012 at 01:43:08PM -0800, Ethan Jackson wrote:
> > +choose_port(struct dpif *dpif, struct netdev *netdev)
> >  {
> >     struct dp_netdev *dp = get_dp_netdev(dpif);
> >     int port_no;
> >
> > +    if (dpif->dpif_class == &dpif_dummy_class) {
> > +        /* If the port name contains a number, try to assign that port 
> > number.
> > +         * This can make writing unit tests easier because port numbers are
> > +         * predictable. */
> > +        const char *p;
> > +
> > +        for (p = netdev_get_name(netdev); *p != '\0'; p++) {
> > +            if (isdigit((unsigned char) *p)) {
> > +                port_no = strtol(p, NULL, 10);
> > +                if (port_no > 0 && port_no < MAX_PORTS
> > +                    && !dp->ports[port_no]) {
> > +                    return port_no;
> > +                }
> 
> I would be inclined to break if p is a digit, but doesn't fit into the
> range of valid port numbers.  This indicates that the user attempted
> to manually allocate a port number but failed.  I'm worried about a
> user naming their netdev port-123456 and getting 456 as their port
> number because we keep attempting until successful.  I think it would
> be cleaner to fall back to the standard port selection logic in this
> case.

I made that change.  That was my intention anyway, I just failed to
implement it properly.

Besides that, I applied the simplifications to tests that had been
introduced since I sent out this commit for review, and then pushed
it.

Thanks,

Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to