[ovs-dev] [PATCH v2 1/4] dpif-netlink: Detect Out-Of-Resource condition on a netdev

[Sriharsha Basavapatna]

On Tue, Jul 10, 2018 at 4:13 AM, Ben Pfaff <blp at ovn.org> wrote:
> On Sun, Jul 08, 2018 at 07:15:37PM +0530, Sriharsha Basavapatna via dev wrote:
>> This is the first patch in the patch-set to support dynamic rebalancing
>> of offloaded flows.
>>
>> The patch detects OOR condition on a netdev port when ENOSPC error is
>> returned by TC-Flower while adding a flow rule. A new structure is added
>> to the netdev called "netdev_hw_info", to store OOR related information
>> required to perform dynamic offload-rebalancing.
>>
>> Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna at broadcom.com>
>> Co-authored-by: Venkat Duvvuru <venkatkumar.duvvuru at broadcom.com>
>> Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru at broadcom.com>
>> Reviewed-by: Sathya Perla <sathya.perla at broadcom.com>
>
> Thanks for the patch.
>
> This fails to build on my system, with:
>
>     In file included from ../lib/lldp/lldpd.h:32,
>                      from ../lib/ovs-lldp.h:26,
>                      from ../ofproto/ofproto-dpif-xlate.h:28,
>                      from ../ofproto/ofproto-dpif-upcall.c:36:
>     ../ofproto/ofproto-dpif-upcall.c: In function 'udpif_update_flow_pps':
>     ../ofproto/ofproto-dpif-upcall.c:2630:18: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'long long unsigned int' [-Werror=format=]
>     ../include/openvswitch/vlog.h:277:41: note: in definition of macro 'VLOG'
>     ../ofproto/ofproto-dpif-upcall.c:2630:9: note: in expansion of macro 'VLOG_DBG'
>     ../ofproto/ofproto-dpif-upcall.c:2630:18: error: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'uint64_t' {aka 'long long unsigned int'} [-Werror=format=]
>     ../include/openvswitch/vlog.h:277:41: note: in definition of macro 'VLOG'
>     ../ofproto/ofproto-dpif-upcall.c:2630:9: note: in expansion of macro 'VLOG_DBG'
>
> I guess that you should use "%llu" for unsigned long long, and "%"PRIu64
> for uint64_t.  Alternatively, a lot of the debug logging here doesn't
> seem particularly useful, should it be there?  (Often __func__ is a sign
> that a log message is more of a debug aid for the programmer and not
> something that should appear in the field.)

I agree, this one was more of a debug aid. I'll remove it and revisit
the other ones that we added.
Thanks,
-Harsha