[ovs-dev] [PATCH 01/12] ovs-ofputil: Make str_to_port_no() aware of invalid ports

[Simon Horman]

On Wed, Sep 19, 2012 at 10:45:25AM -0700, Ben Pfaff wrote:
> On Tue, Sep 18, 2012 at 08:58:14AM +0900, Simon Horman wrote:
> > On Mon, Sep 17, 2012 at 03:53:28PM -0700, Ben Pfaff wrote:
> > > On Mon, Sep 17, 2012 at 09:26:48AM +0900, Simon Horman wrote:
> > > > Open Flow 1.1 and 1.2 make use of 32 bit ports, however Open vSwtich maps
> > > > them to 16 bits. Make ovs-ofputl aware of this.
> > > > 
> > > > Also, only accept ports that fit into 16 bits for Open Flow 1.0.
> > > > 
> > > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > > 
> > > This makes the acceptable port numbers a function of the protocol we
> > > end up with, but the ovs-ofctl philosophy has always been that you
> > > tell it what you want and it'll pick an acceptable protocol for doing
> > > what you asked for.  There's also an issue of some confusion over
> > > whether, say, port 65535 is OFPP_NONE (OF1.0) or just an
> > > undistinguished "physical" port (OF1.1).  I think we can do better.
> > > 
> > > Here's a counterproposal.  What do you think?
> > > 
> > > Thanks,
> > > 
> > > Ben.
> > > 
> > > --8<--------------------------cut here-------------------------->8--
> > > 
> > > From: Ben Pfaff <blp at nicira.com>
> > > Date: Mon, 17 Sep 2012 15:49:59 -0700
> > > Subject: [PATCH] ovs-ofctl: Accept port keywords, OF1.1 port numbers, reject port number 0.
> > > 
> > > OpenFlow 1.0 has special reserved ports in the range 0xfff8 to 0xffff.
> > > OpenFlow 1.1 and later has the same ports in the range 0xfffffff8 to
> > > 0xffffffff and allows the OF1.0 range to be used for ordinary ("physical")
> > > switch ports.  This means that, naively, the meaning of a port number in
> > > the range 0xfff8 to 0xffff given on the ovs-ofctl command line depends on
> > > the protocol in use.  This commit implements something a little smarter:
> > > 
> > >     - Accept keyword names (e.g. LOCAL) for special reserved ports
> > >       everywhere that such a port can plausibly be used (previously they
> > >       were only accepted in some places).
> > > 
> > >     - Translate 0xfff8...0xffff to 0xfffffff8...0xffffffff for now, since
> > >       OF1.1+ isn't in widespread use and those particular ports aren't
> > >       likely to be in use in OF1.1+ anyway.
> > 
> > I don't really like the above assumption, 0xfff8...0xffff
> > are valid OF1.1+ port numbers, it seems that it would
> > cause rather a surprise if they were used as non-reserved ports
> > but Open vSwtich interpreted them as reserved ports.
> > 
> > I am prepared to live with it, but I don't like it.
> 
> For now, we couldn't use those OF1.1+ port numbers anyway, because
> internally OVS only supports 16-bit port numbers.  Presumably some
> time in the future we'll switch OVS to use 32-bit port numbers
> internally, but until then I can't really see a drawback.

True, I reflected on that after writing my previous response.

> Would it make you happier if I added some kind of formal deprecation
> notice to NEWS saying that numeric values for the reserved OpenFlow
> ports won't be supported after such-and-such a date?

I'm happy with adding that notice at a later date.

In all, I think that this is a reasonable approach given the constraints.

Acked-by: Simon Horman <horms at verge.net.au>