[ovs-dev] [PATCH 6/8] ofproto: Querying port stats for individual ports (OpenFlow 1.0)

Justin Pettit jpettit at nicira.com
Sat Jan 23 02:02:40 UTC 2010


On Jan 22, 2010, at 10:15 AM, Ben Pfaff wrote:

>> static void
>> +ofp_port_stats_request(struct ds *string, const void *body_,
>> +                       size_t len UNUSED, int verbosity UNUSED)
>> +{
>> +    const struct ofp_port_stats_request *psr = body_;
>> +    ds_put_format(string, "port_no=%2"PRIu16, ntohs(psr->port_no));
> 
> Is the "2" here intentional?

It was a copy paste from elsewhere.  When printing a list of information about ports, we made room for at least two digits, so that relatively high fanout switches would have cleaner output.  However, this message always contains only one item, so it's not needed.

>> +
>> +    msg = start_stats_reply(osr, sizeof *ops * 16);
>>     PORT_ARRAY_FOR_EACH (port, &p->ports, port_no) {
>>         struct netdev_stats stats;
>> 
>> +        /* A request with "port_no" set to OFPP_NONE means that stats
>> +         * for all interfaces are wanted.  Otherwise, we just want to
>> +         * return information for the requested port. */
>> +        if (psr->port_no != htons(OFPP_NONE) &&
>> +                psr->port_no != htons(odp_port_to_ofp_port(port_no))) {
>> +            continue;
>> +        }
> 
> There shouldn't be any need to loop through all of the ports if the
> request is for a single port.  You can just use
> port_array_get(&p->ports, ofp_port_to_odp_port(ntohs(psr->port_no))) to
> fetch the port we want, and then break the code to fill the result into
> a new function.

Good idea.  I re-factored the code like you suggested.

> Does OpenFlow say what happens if the request specifies th port number
> of a nonexistent port?  Should we reply with an error?

No, it doesn't say.  That's also on my list things to bring up.

--Justin






More information about the dev mailing list