Looks good.
On Wed, Jun 27, 2012 at 10:05 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> > --- > v1 was here: http://openvswitch.org/pipermail/dev/2012-June/017780.html > v2: no change > > AUTHORS | 1 + > NEWS | 4 ++++ > ofproto/ofproto.c | 28 ++++++++++++++++++---------- > tests/ofproto.at | 8 ++++++++ > 4 files changed, 31 insertions(+), 10 deletions(-) > > diff --git a/AUTHORS b/AUTHORS > index bdfcb33..dc98ebe 100644 > --- a/AUTHORS > +++ b/AUTHORS > @@ -140,6 +140,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 > Ralf Heiringhoff r...@frosty-geek.net > Ram Jothikumar rjothiku...@nicira.com > Ramana Reddy gtvrre...@gmail.com > diff --git a/NEWS b/NEWS > index c2ec953..f0b2490 100644 > --- a/NEWS > +++ b/NEWS > @@ -11,6 +11,10 @@ post-v1.7.0 > the multicast bit in the destination address could be individually > masked.) > - New field OXM_OF_METADATA, to align with OpenFlow 1.1. > + - 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 1a2f712..ce4da9d 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -2684,7 +2684,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) > { > @@ -2697,8 +2697,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 > @@ -2707,9 +2710,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); > > @@ -2718,21 +2722,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; > + } > } > - } 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 28adf74..d703fa8 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 > +]) > +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 > > -- > 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
