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,
Ben.
--8<--------------------------cut here-------------------------->8--
From: Ben Pfaff <b...@nicira.com>
Date: Mon, 6 Feb 2012 10:10:08 -0800
Subject: [PATCH] ofp-util: Add functions for working with OpenFlow 1.1 port
numbers.
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.
---
include/openflow/openflow-1.1.h | 16 +++++++++++++++-
lib/ofp-util.c | 28 ++++++++++++++++++++++++++++
lib/ofp-util.h | 3 +++
3 files changed, 46 insertions(+), 1 deletions(-)
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 8b407e3..93002af 100644
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -54,6 +54,20 @@
#include "openflow/openflow-common.h"
-/* Nothing here yet. */
+/* OpenFlow 1.1 uses 32-bit port numbers. Open vSwitch, for now, uses OpenFlow
+ * 1.0 port numbers internally. We map them to OpenFlow 1.0 as follows:
+ *
+ * OF1.1 <=> OF1.0
+ * ----------------------- ---------------
+ * 0x00000000...0x0000feff <=> 0x0000...0xfeff "physical" ports
+ * 0x0000ff00...0xfffffeff <=> not supported
+ * 0xffffff00...0xffffffff <=> 0xff00...0xffff "reserved" OFPP_* ports
+ *
+ * OFPP11_OFFSET is the value that must be added or subtracted to convert
+ * an OpenFlow 1.0 reserved port number to or from, respectively, the
+ * corresponding OpenFlow 1.1 reserved port number.
+ */
+#define OFPP11_MAX 0xffffff00
+#define OFPP11_OFFSET (OFPP11_MAX - OFPP_MAX)
#endif /* openflow/openflow-1.1.h */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1257e6f..02fa106 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2591,6 +2591,34 @@ 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 >= OFPP11_MAX) {
+ *ofp10_port = ofp11_port_h - OFPP11_OFFSET;
+ return 0;
+ } else {
+ VLOG_WARN_RL(&bad_ofmsg_rl, "port %"PRIu32" is outside the supported "
+ "range 0 through %d or 0x%"PRIx32" through 0x%"PRIx32,
+ ofp11_port_h, OFPP_MAX - 1,
+ (uint32_t) OFPP11_MAX, UINT32_MAX);
+ 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 + OFPP11_OFFSET);
+}
+
/* Checks that 'port' is a valid output port for the OFPAT_OUTPUT action, given
* that the switch will never have more than 'max_ports' ports. Returns 0 if
* 'port' is valid, otherwise an OpenFlow return code. */
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 5991594..3422ece 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -99,6 +99,9 @@ enum ofputil_msg_code ofputil_msg_type_code(const struct
ofputil_msg_type *);
const char *ofputil_msg_type_name(const struct ofputil_msg_type *);
/* Port numbers. */
+enum ofperr ofputil_port_from_ofp11(ovs_be32 ofp11_port, uint16_t *ofp10_port);
+ovs_be32 ofputil_port_to_ofp11(uint16_t ofp10_port);
+
enum ofperr ofputil_check_output_port(uint16_t ofp_port, int max_ports);
bool ofputil_port_from_string(const char *, uint16_t *port);
void ofputil_format_port(uint16_t port, struct ds *);
--
1.7.2.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
