On Tue, Jun 26, 2012 at 04:03:32PM -0700, Pravin Shelar wrote:
> 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>
[...]
> > @@ -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.
Unless I misunderstand what you're saying, that's intentional. With
OFPP_ALL it's unlikely that every port has a given queue, so we only
report an error if no port has the specified queue.
> > 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.
That's not the way that we've had ovs-ofctl work in the past. I think
that's a reasonable idea, that is, that ovs-ofctl should exit with
status 1 if the switch reports an error, but it would be a separate
change.
> > +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.
OK, what's in the test's testsuite.log?
I've reposted these two patches now.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
