[ovs-dev] [PATCH 2/2] ofp-util: Implement OpenFlow 1.1 packet-in message.
Ben Pfaff
blp at nicira.com
Mon Dec 2 21:20:20 UTC 2013
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
DESIGN | 47 +++++++++++++++++++++++++++++++++++++++
include/openflow/openflow-1.1.h | 7 +-----
lib/ofp-util.c | 41 +++++++++++++++++++++++++++++++++-
tests/ofp-print.at | 15 +++++++++++++
tests/ofproto.at | 33 +++++++++++++++++++++++++++
5 files changed, 136 insertions(+), 7 deletions(-)
diff --git a/DESIGN b/DESIGN
index d36b025..4d654d0 100644
--- a/DESIGN
+++ b/DESIGN
@@ -266,6 +266,53 @@ OpenFlow 1.3 makes these changes:
The table for 1.3 is the same as the one shown above for 1.2.
+OFPT_PACKET_IN
+==============
+
+The OpenFlow 1.1 specification for OFPT_PACKET_IN is confusing. The
+definition in OF1.1 openflow.h is[*]:
+
+ /* Packet received on port (datapath -> controller). */
+ struct ofp_packet_in {
+ struct ofp_header header;
+ uint32_t buffer_id; /* ID assigned by datapath. */
+ uint32_t in_port; /* Port on which frame was received. */
+ uint32_t in_phy_port; /* Physical Port on which frame was received. */
+ uint16_t total_len; /* Full length of frame. */
+ uint8_t reason; /* Reason packet is being sent (one of OFPR_*) */
+ uint8_t table_id; /* ID of the table that was looked up */
+ uint8_t data[0]; /* Ethernet frame, halfway through 32-bit word,
+ so the IP header is 32-bit aligned. The
+ amount of data is inferred from the length
+ field in the header. Because of padding,
+ offsetof(struct ofp_packet_in, data) ==
+ sizeof(struct ofp_packet_in) - 2. */
+ };
+ OFP_ASSERT(sizeof(struct ofp_packet_in) == 24);
+
+The confusing part is the comment on the data[] member. This comment
+is a leftover from OF1.0 openflow.h, in which the comment was correct:
+sizeof(struct ofp_packet_in) is 20 in OF1.0 and offsetof(struct
+ofp_packet_in, data) is 18. When OF1.1 was written, the structure
+members were changed but the comment was carelessly not updated, and
+the comment became wrong: sizeof(struct ofp_packet_in) and
+offsetof(struct ofp_packet_in, data) are both 24 in OF1.1.
+
+That leaves the question of how to implement ofp_packet_in in OF1.1.
+The OpenFlow reference implementation for OF1.1 does not include any
+padding, that is, the first byte of the encapsulated frame immediately
+follows the 'table_id' member without a gap. Open vSwitch therefore
+implements it the same way for compatibility.
+
+For an earlier discussion, please see the thread archived at:
+https://mailman.stanford.edu/pipermail/openflow-discuss/2011-August/002604.html
+
+[*] The quoted definition is directly from OF1.1. Definitions used
+ inside OVS omit the 8-byte ofp_header members, so the sizes in
+ this discussion are 8 bytes larger than those declared in OVS
+ header files.
+
+
VLAN Matching
=============
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index 4ee9c5c..004e61b 100644
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -753,12 +753,7 @@ struct ofp11_packet_in {
ovs_be16 total_len; /* Full length of frame. */
uint8_t reason; /* Reason packet is being sent (one of OFPR_*) */
uint8_t table_id; /* ID of the table that was looked up */
- /* uint8_t data[0]; Ethernet frame, halfway through 32-bit word,
- so the IP header is 32-bit aligned. The
- amount of data is inferred from the length
- field in the header. Because of padding,
- offsetof(struct ofp_packet_in, data) ==
- sizeof(struct ofp_packet_in) - 2. */
+ /* Followed by Ethernet frame. */
};
OFP_ASSERT(sizeof(struct ofp11_packet_in) == 16);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index b4723b8..7fc4c7c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3196,6 +3196,23 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
pin->reason = opi->reason;
pin->buffer_id = ntohl(opi->buffer_id);
pin->total_len = ntohs(opi->total_len);
+ } else if (raw == OFPRAW_OFPT11_PACKET_IN) {
+ const struct ofp11_packet_in *opi;
+ enum ofperr error;
+
+ opi = ofpbuf_pull(&b, sizeof *opi);
+
+ pin->packet = b.data;
+ pin->packet_len = b.size;
+
+ pin->buffer_id = ntohl(opi->buffer_id);
+ error = ofputil_port_from_ofp11(opi->in_port, &pin->fmd.in_port);
+ if (error) {
+ return error;
+ }
+ pin->total_len = ntohs(opi->total_len);
+ pin->reason = opi->reason;
+ pin->table_id = opi->table_id;
} else if (raw == OFPRAW_NXT_PACKET_IN) {
const struct nx_packet_in *npi;
struct match match;
@@ -3310,6 +3327,27 @@ ofputil_encode_nx_packet_in(const struct ofputil_packet_in *pin)
}
static struct ofpbuf *
+ofputil_encode_ofp11_packet_in(const struct ofputil_packet_in *pin)
+{
+ struct ofp11_packet_in *opi;
+ struct ofpbuf *packet;
+
+ packet = ofpraw_alloc_xid(OFPRAW_OFPT11_PACKET_IN, OFP11_VERSION,
+ htonl(0), pin->packet_len);
+ opi = ofpbuf_put_zeros(packet, sizeof *opi);
+ opi->buffer_id = htonl(pin->buffer_id);
+ opi->in_port = ofputil_port_to_ofp11(pin->fmd.in_port);
+ opi->in_phy_port = opi->in_port;
+ opi->total_len = htons(pin->total_len);
+ opi->reason = pin->reason;
+ opi->table_id = pin->table_id;
+
+ ofpbuf_put(packet, pin->packet, pin->packet_len);
+
+ return packet;
+}
+
+static struct ofpbuf *
ofputil_encode_ofp12_packet_in(const struct ofputil_packet_in *pin,
enum ofputil_protocol protocol)
{
@@ -3373,7 +3411,8 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
break;
case OFPUTIL_P_OF11_STD:
- NOT_REACHED();
+ packet = ofputil_encode_ofp11_packet_in(pin);
+ break;
case OFPUTIL_P_OF12_OXM:
case OFPUTIL_P_OF13_OXM:
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 82e8c3d..2f09c1b 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -469,6 +469,21 @@ tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:0
])
AT_CLEANUP
+AT_SETUP([OFPT_PACKET_IN - OF1.1])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "\
+02 0a 00 54 00 00 00 00 00 00 01 11 00 00 00 03 \
+00 00 00 03 00 3c 00 00 \
+50 54 00 00 00 06 50 54 00 00 00 05 08 00 \
+45 00 00 28 bd 12 00 00 40 06 3c 6a c0 a8 00 01 \
+c0 a8 00 02 27 2f 00 00 78 50 cc 5b 57 af 42 1e \
+50 02 02 00 26 e8 00 00 00 00 00 00 00 00 \
+"], [0], [dnl
+OFPT_PACKET_IN (OF1.1) (xid=0x0): total_len=60 in_port=3 (via no_match) data_len=60 buffer=0x00000111
+tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=10031,tp_dst=0,tcp_flags=0x002 tcp_csum:26e8
+])
+AT_CLEANUP
+
AT_SETUP([OFPT_PACKET_IN - OF1.2])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print "\
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 27b6b34..be7387d 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1757,6 +1757,39 @@ OFPT_BARRIER_REPLY (OF1.2):
OVS_VSWITCHD_STOP
AT_CLEANUP
+dnl This test checks that OFPT_PACKET_OUT accepts both OFPP_NONE (as
+dnl specified by OpenFlow 1.1) and OFPP_CONTROLLER (used by some
+dnl controllers despite the spec) as meaning a packet that was generated
+dnl by the controller.
+AT_SETUP([ofproto - packet-out from controller (OpenFlow 1.1)])
+OVS_VSWITCHD_START
+
+# Start a monitor listening for packet-ins.
+AT_CHECK([ovs-ofctl -O OpenFlow11 monitor br0 --detach --no-chdir --pidfile])
+ovs-appctl -t ovs-ofctl ofctl/send 0209000c0123456700000080
+ovs-appctl -t ovs-ofctl ofctl/barrier
+ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
+AT_CAPTURE_FILE([monitor.log])
+
+# Send some packet-outs with OFPP_NONE and OFPP_CONTROLLER (65533) as in_port.
+AT_CHECK([ovs-ofctl -O OpenFlow11 packet-out br0 none controller '0001020304050010203040501234'])
+AT_CHECK([ovs-ofctl -O OpenFlow11 packet-out br0 4294967293 controller '0001020304050010203040505678'])
+
+# Stop the monitor and check its output.
+ovs-appctl -t ovs-ofctl ofctl/barrier
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
+OFPT_PACKET_IN (OF1.1): total_len=14 in_port=ANY (via action) data_len=14 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
+OFPT_PACKET_IN (OF1.1): total_len=14 in_port=CONTROLLER (via action) data_len=14 (unbuffered)
+metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x5678
+OFPT_BARRIER_REPLY (OF1.1):
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
dnl This test checks that metadata is encoded in packet_in structures,
dnl supported by NXAST.
AT_SETUP([ofproto - packet-out with metadata (NXM)])
--
1.7.10.4
More information about the dev
mailing list