On Mon, Jun 11, 2012 at 11:26 AM, Ben Pfaff <b...@nicira.com> wrote:
> Until now, Open vSwitch has ignored missing ports and queues in most cases
> in queue stats requests, simply returning an empty set of statistics.
> It seems that it is better to report an error, so this commit does this.
>
> Reported-by: Prabina Pattnaik <prabina.pattn...@nechclst.in>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> AUTHORS | 1 +
> NEWS | 7 ++++++-
> ofproto/ofproto.c | 28 ++++++++++++++++++----------
> tests/ofproto.at | 8 ++++++++
> 4 files changed, 33 insertions(+), 11 deletions(-)
>
> diff --git a/AUTHORS b/AUTHORS
> index 10a01ab..801d31a 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -137,6 +137,7 @@ Paul Ingram p...@nicira.com
> Paulo Cravero pcrav...@as2594.net
> Peter Balland pe...@nicira.com
> Peter Phaal peter.ph...@inmon.com
> +Prabina Pattnaik prabina.pattn...@nechclst.in
> Ram Jothikumar rjothiku...@nicira.com
> Ramana Reddy gtvrre...@gmail.com
> Rob Sherwood rob.sherw...@bigswitch.com
> diff --git a/NEWS b/NEWS
> index 8cd38dd..b96ced8 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -2,7 +2,12 @@ post-v1.7.0
> ------------------------
> - ovs-ofctl:
> - "mod-port" command can now control all OpenFlow config flags.
> - - Added support for arbitrary ethernet masks
> + - OpenFlow:
> + - Added support for arbitrary ethernet masks
> + - The OFPST_QUEUE request now reports an error if a specified port or
> + queue does not exist, or for requests for a specific queue on all
> + ports, if the specified queue does not exist on any port.
> (Previous
> + versions generally reported an empty set of results.)
> - Additional protocols are not mirrored and dropped when forward-bpdu is
> false. For a full list, see the ovs-vswitchd.conf.db man page.
> - Open vSwitch now sends RARP packets in situations where it previously
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index dee2cbc..8f7d05c 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2644,7 +2644,7 @@ handle_queue_stats_dump_cb(uint32_t queue_id,
> put_queue_stats(cbdata, queue_id, stats);
> }
>
> -static void
> +static enum ofperr
> handle_queue_stats_for_port(struct ofport *port, uint32_t queue_id,
> struct queue_stats_cbdata *cbdata)
> {
> @@ -2657,8 +2657,11 @@ handle_queue_stats_for_port(struct ofport *port,
> uint32_t queue_id,
>
> if (!netdev_get_queue_stats(port->netdev, queue_id, &stats)) {
> put_queue_stats(cbdata, queue_id, &stats);
> + } else {
> + return OFPERR_OFPQOFC_BAD_QUEUE;
> }
> }
> + return 0;
> }
>
> static enum ofperr
> @@ -2667,9 +2670,10 @@ handle_queue_stats_request(struct ofconn *ofconn,
> {
> struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
> struct queue_stats_cbdata cbdata;
> - struct ofport *port;
> unsigned int port_no;
> + struct ofport *port;
> uint32_t queue_id;
> + enum ofperr error;
>
> COVERAGE_INC(ofproto_queue_req);
>
> @@ -2678,21 +2682,25 @@ handle_queue_stats_request(struct ofconn *ofconn,
> port_no = ntohs(qsr->port_no);
> queue_id = ntohl(qsr->queue_id);
> if (port_no == OFPP_ALL) {
> + error = OFPERR_OFPQOFC_BAD_QUEUE;
> HMAP_FOR_EACH (port, hmap_node, &ofproto->ports) {
> - handle_queue_stats_for_port(port, queue_id, &cbdata);
> + if (!handle_queue_stats_for_port(port, queue_id, &cbdata)) {
> + error = 0;
> + }
in case of multiple ports this error code could reset.
> }
> - } else if (port_no < OFPP_MAX) {
> + } else {
> port = ofproto_get_port(ofproto, port_no);
> - if (port) {
> - handle_queue_stats_for_port(port, queue_id, &cbdata);
> - }
> + error = (port
> + ? handle_queue_stats_for_port(port, queue_id, &cbdata)
> + : OFPERR_OFPQOFC_BAD_PORT);
> + }
> + if (!error) {
> + ofconn_send_replies(ofconn, &cbdata.replies);
> } else {
> ofpbuf_list_delete(&cbdata.replies);
> - return OFPERR_OFPQOFC_BAD_PORT;
> }
> - ofconn_send_replies(ofconn, &cbdata.replies);
>
> - return 0;
> + return error;
> }
>
> static bool
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index dbe31c4..a193490 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -59,6 +59,14 @@ AT_CHECK([ovs-ofctl -vwarn queue-stats br0], [0], [stdout])
> AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> OFPST_QUEUE reply: 0 queues
> ])
> +AT_CHECK([ovs-ofctl -vwarn queue-stats br0 ALL 5], [0],
> + [OFPT_ERROR (xid=0x1): OFPQOFC_BAD_QUEUE
> +OFPST_QUEUE request (xid=0x1):port=ALL queue=5
> +])
expected error code from ofctl shld be 1.
> +AT_CHECK([ovs-ofctl -vwarn queue-stats br0 10], [0],
> + [OFPT_ERROR (xid=0x1): OFPQOFC_BAD_PORT
> +OFPST_QUEUE request (xid=0x1):port=10 queue=ALL
> +])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
I am seeing this test failure on my machine.
>
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
