On Wed, Jun 27, 2012 at 10:05 AM, Ben Pfaff <b...@nicira.com> wrote:
> 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.

right. I misunderstood it.


>
>> > 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.

I do not see any failure with new patches.

Thanks,
Pravin.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to