[ovs-dev] [of1.1 draft v2 5/7] ofp-util: Add functions for working with OpenFlow 1.1 port numbers.
Simon Horman
horms at verge.net.au
Mon Feb 6 23:23:41 UTC 2012
On Mon, Feb 06, 2012 at 10:11:23AM -0800, Ben Pfaff wrote:
> On Mon, Feb 06, 2012 at 02:23:29PM +0900, Simon Horman wrote:
> > On Fri, Feb 03, 2012 at 12:43:53PM -0800, Ben Pfaff wrote:
> > > OpenFlow 1.1 extends port numbers to 32 bits. Initially we plan to support
> > > only port numbers in the 16-bit range in Open vSwitch. However the OF1.1
> > > reserved ports have high-valued fixed numbers that require translation to
> > > high fixed values in the 16-bit range for OF1.0. These new functions
> > > provide this translation.
> > >
> > > Nothing uses these functions yet.
> > >
> > > These new functions need comments before they're ready.
> > > ---
> > > lib/ofp-util.c | 24 ++++++++++++++++++++++++
> > > lib/ofp-util.h | 3 +++
> > > 2 files changed, 27 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> > > index 1257e6f..a4d13cc 100644
> > > --- a/lib/ofp-util.c
> > > +++ b/lib/ofp-util.c
> > > @@ -2591,6 +2591,30 @@ ofputil_frag_handling_from_string(const char *s, enum ofp_config_flags *flags)
> > > return true;
> > > }
> > >
> > > +enum ofperr
> > > +ofputil_port_from_ofp11(ovs_be32 ofp11_port, uint16_t *ofp10_port)
> > > +{
> > > + uint32_t ofp11_port_h = ntohl(ofp11_port);
> > > +
> > > + if (ofp11_port_h < OFPP_MAX) {
> > > + *ofp10_port = ofp11_port_h;
> > > + return 0;
> > > + } else if (ofp11_port_h >= 0xfffffff8) {
> > > + *ofp10_port = ofp11_port_h >> 16;
> > > + return 0;
> >
> > Perhaps a #define in place of 0xfffffff8 would improve readability.
> >
> > > + } else {
> > > + VLOG_WARN_RL(&bad_ofmsg_rl, "port %"PRIu32" is outside the supported "
> > > + "range 0 through %d", ofp11_port_h, OFPP_MAX - 1);
> >
> > This message doesn't seem to reflect the code above which translates
> > fake output ports in the range 0xfffffff8 - 0xffffffff
> >
> > > + return OFPERR_OFPBAC_BAD_OUT_PORT;
> > > + }
> > > +}
> > > +
> > > +ovs_be32
> > > +ofputil_port_to_ofp11(uint16_t ofp10_port)
> > > +{
> > > + return htonl(ofp10_port < OFPP_MAX ? ofp10_port : ofp10_port + 0xffff0000);
> > > +}
> >
> > Perhaps 0xffff0000 could be a #define ?
> >
> > Is it ok for ports in the range OFPP_MAX - (OFPP_IN_PORT -1) to
> > be translated to the 0xffff0000 range?
>
> Thanks for the review. I agree that these functions can be improved.
> How about the following better abstraction instead?
Thanks, that looks good to me.
More information about the dev
mailing list