Signed-off-by: Simon Horman <ho...@verge.net.au>
---
This change is also needed for several other stats request messages.
I have posted this as an RFC before proceeding with updating
the decoders for other stats request messages.
---
lib/ofp-msgs.h | 4 ++--
lib/ofp-print.c | 23 +++++++++++++++--------
lib/ofp-util.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++-------
lib/ofp-util.h | 4 ++--
ofproto/ofproto.c | 38 ++++++++++++++++++++++++--------------
tests/ofproto.at | 36 ++++++++++++++++++++++++++++++++++++
6 files changed, 126 insertions(+), 33 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 26fd6a3..ea4b37d 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -281,9 +281,9 @@ enum ofpraw {
/* OFPST 1.3 (3): struct ofp13_table_stats[]. */
OFPRAW_OFPST13_TABLE_REPLY,
- /* OFPST 1.0 (4): struct ofp10_port_stats_request. */
+ /* OFPST 1.0 (4): struct ofp10_port_stats_request[]. */
OFPRAW_OFPST10_PORT_REQUEST,
- /* OFPST 1.1+ (4): struct ofp11_port_stats_request. */
+ /* OFPST 1.1+ (4): struct ofp11_port_stats_request[]. */
OFPRAW_OFPST11_PORT_REQUEST,
/* OFPST 1.0 (4): struct ofp10_port_stats[]. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 13705d0..ffeaf12 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1515,16 +1515,23 @@ static void
ofp_print_ofpst_port_request(struct ds *string, const struct ofp_header *oh)
{
ofp_port_t ofp10_port;
- enum ofperr error;
+ struct ofpbuf b;
- error = ofputil_decode_port_stats_request(oh, &ofp10_port);
- if (error) {
- ofp_print_error(string, error);
- return;
- }
+ ofpbuf_use_const(&b, oh, ntohs(oh->length));
+ for (;;) {
+ int error;
- ds_put_cstr(string, " port_no=");
- ofputil_format_port(ofp10_port, string);
+ error = ofputil_decode_port_stats_request(&b, &ofp10_port);
+ if (error) {
+ if (error != EOF) {
+ ofp_print_error(string, error);
+ }
+ return;
+ }
+
+ ds_put_cstr(string, " port_no=");
+ ofputil_format_port(ofp10_port, string);
+ }
}
static void
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 7fc4c7c..a7c3769 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -5739,23 +5739,58 @@ ofputil_decode_port_stats(struct ofputil_port_stats
*ps, struct ofpbuf *msg)
return OFPERR_OFPBRC_BAD_LEN;
}
-/* Parse a port status request message into a 16 bit OpenFlow 1.0
+/* Converts an OFPST_PORT_DESC 'msg' into a 16 bit OpenFlow 1.0
* port number and stores the latter in '*ofp10_port'.
- * Returns 0 if successful, otherwise an OFPERR_* number. */
-enum ofperr
-ofputil_decode_port_stats_request(const struct ofp_header *request,
+ *
+ * Multiple OFPST_PORT_DESC requests can be packed into a single
+ * OpenFlow message. Calling this function multiple times for a single 'msg'
+ * iterates through the replies. The caller must initially leave 'msg''s layer
+ * pointers null and not modify them between calls.
+ *
+ * Returns 0 if successful, EOF if no replies were left in this 'msg',
+ * otherwise an OFPERR_* number. */
+int
+ofputil_decode_port_stats_request(struct ofpbuf *msg,
ofp_port_t *ofp10_port)
{
- switch ((enum ofp_version)request->version) {
+ const struct ofp_header *oh;
+ enum ofperr error;
+ enum ofpraw raw;
+
+ error = (msg->l2
+ ? ofpraw_decode(&raw, msg->l2)
+ : ofpraw_pull(&raw, msg));
+ if (error) {
+ return error;
+ }
+ oh = msg->l2;
+
+ if (!msg->size) {
+ return EOF;
+ }
+
+ switch ((enum ofp_version)oh->version) {
case OFP13_VERSION:
case OFP12_VERSION:
case OFP11_VERSION: {
- const struct ofp11_port_stats_request *psr11 = ofpmsg_body(request);
+ const struct ofp11_port_stats_request *psr11;
+
+ psr11 = ofpbuf_try_pull(msg, sizeof *psr11);
+ if (!psr11) {
+ goto trailing_garbage;
+ }
+
return ofputil_port_from_ofp11(psr11->port_no, ofp10_port);
}
case OFP10_VERSION: {
- const struct ofp10_port_stats_request *psr10 = ofpmsg_body(request);
+ const struct ofp10_port_stats_request *psr10;
+
+ psr10 = ofpbuf_try_pull(msg, sizeof *psr10);
+ if (!psr10) {
+ goto trailing_garbage;
+ }
+
*ofp10_port = u16_to_ofp(ntohs(psr10->port_no));
return 0;
}
@@ -5763,6 +5798,11 @@ ofputil_decode_port_stats_request(const struct
ofp_header *request,
default:
NOT_REACHED();
}
+
+trailing_garbage:
+ VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_PORT_DESC request has "
+ "%"PRIuSIZE" leftover bytes at end", msg->size);
+ return OFPERR_OFPBRC_BAD_LEN;
}
/* Frees all of the "struct ofputil_bucket"s in the 'buckets' list. */
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index fef85e0..726cd3e 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -918,8 +918,8 @@ void ofputil_append_port_stat(struct list *replies,
const struct ofputil_port_stats *ops);
size_t ofputil_count_port_stats(const struct ofp_header *);
int ofputil_decode_port_stats(struct ofputil_port_stats *, struct ofpbuf *msg);
-enum ofperr ofputil_decode_port_stats_request(const struct ofp_header *request,
- ofp_port_t *ofp10_port);
+int ofputil_decode_port_stats_request(struct ofpbuf *request,
+ ofp_port_t *ofp10_port);
struct ofputil_queue_stats_request {
ofp_port_t port_no; /* OFPP_ANY means "all ports". */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3a60328..3a07958 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3151,23 +3151,33 @@ handle_port_stats_request(struct ofconn *ofconn,
struct ofproto *p = ofconn_get_ofproto(ofconn);
struct ofport *port;
struct list replies;
- ofp_port_t port_no;
- enum ofperr error;
-
- error = ofputil_decode_port_stats_request(request, &port_no);
- if (error) {
- return error;
- }
+ struct ofpbuf b;
ofpmp_init(&replies, request);
- if (port_no != OFPP_ANY) {
- port = ofproto_get_port(p, port_no);
- if (port) {
- append_port_stat(port, &replies);
+
+ ofpbuf_use_const(&b, request, ntohs(request->length));
+ for (;;) {
+ ofp_port_t port_no;
+ int error;
+
+ error = ofputil_decode_port_stats_request(&b, &port_no);
+ if (error) {
+ if (error == EOF) {
+ break;
+ } else {
+ return error;
+ }
}
- } else {
- HMAP_FOR_EACH (port, hmap_node, &p->ports) {
- append_port_stat(port, &replies);
+
+ if (port_no != OFPP_ANY) {
+ port = ofproto_get_port(p, port_no);
+ if (port) {
+ append_port_stat(port, &replies);
+ }
+ } else {
+ HMAP_FOR_EACH (port, hmap_node, &p->ports) {
+ append_port_stat(port, &replies);
+ }
}
}
diff --git a/tests/ofproto.at b/tests/ofproto.at
index be7387d..3137dcb 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2152,3 +2152,39 @@ OFPT_BARRIER_REPLY (OF1.3):
OVS_VSWITCHD_STOP
AT_CLEANUP
+
+AT_SETUP([ofproto - port_desc request with multiple elements (OpenFlow 1.3)])
+AT_KEYWORDS([monitor])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [2], [3], [4])
+
+# Start a monitor, use the required protocol version
+ovs-ofctl -O OpenFlow13 monitor br0 --detach --no-chdir --pidfile >monitor.log
2>&1
+AT_CAPTURE_FILE([monitor.log])
+
+# Send OpenFlow13 message (04), OFPT_MULTIPART_REQUEST (12), length (0020),
xid (0000000a)
+# OFPMP_PORT_STATS (0004), no flags (0000), pad
(00000000)
+# port_no (0001), pad (00000000)
+# port_no (0002), pad (00000000)
+ovs-appctl -t ovs-ofctl ofctl/send "041200200000000a 0004000000000000
0000000100000000 0000000200000000"
+
+ovs-appctl -t ovs-ofctl ofctl/barrier
+
+# Check default setting
+read -r -d '' expected <<'EOF'
+EOF
+
+AT_CHECK([ofctl_strip < monitor.log | STRIP_DURATION], [], [dnl
+send: OFPST_PORT request (OF1.3): port_no=1 port_no=2
+OFPST_PORT reply (OF1.3): 2 ports
+ port 1: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
+ tx pkts=0, bytes=0, drop=0, errs=0, coll=0
+ duration=?s
+ port 2: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
+ tx pkts=0, bytes=0, drop=0, errs=0, coll=0
+ duration=?s
+OFPT_BARRIER_REPLY (OF1.3):
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
--
1.8.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
