Re: [PATCH v2 0/6] Improvements to flow specific logging

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 17 Jun 2026 13:11:16 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> While working on podman bug 23739, I found some deficiences in how and
> when we report various socket errors related to specific flows.  Here
> are some preliminary patches to improve that.
> 
> I think patches 1..3 are pretty straightforward and safe.  4/4 I think
> is a good idea in principle - promoting the priority of a number of
> messages now that we have ratelimiting.  However, plumbing 'now' all
> the places we need it resuls in a *lot* of churn, so we might want to
> postpone or split it up.
> 
> Still, current drafted posted for consideration.
> 
> v2:
>  * Added patch 2/6 At Stefano's suggestion
>  * Simplified the errno Handling fix at Stefano's suggestion
>  * Added patch 6/6 which I noticed while revising 5/6.
>  * Made a handful of minor corrections based on feedback.
> 
> David Gibson (6):
>   flow: Regularise flow specific logging helpers
>   flow: Indent flow details messages
>   flow: Include flow details with higher priority log messages
>   flow, udp: Fix errno handling in udp_flow_sock()
>   flow, treewide: Promote priority of selected flow-linked messages
>   udp: Improve messages for errors getting errors

I was about to apply this without 6/6 because it leads cppcheck (at
least my version, but the warning appears obviously correct) to
complain that:

---
udp.c:583:20: style: Local variable 'uflow' shadows outer variable [shadowVariable]
  struct udp_flow *uflow = udp_at_sidx(sidx);
                   ^
udp.c:576:19: note: Shadowed declaration
 struct udp_flow *uflow;
                  ^
udp.c:583:20: note: Shadow variable
  struct udp_flow *uflow = udp_at_sidx(sidx);
                   ^
---

and Coverity Scan to report this:

---
/home/sbrivio/passt/udp.c:727:3:
  Type: Extra argument to printf format specifier (PRINTF_ARGS)

/home/sbrivio/passt/udp.c:727:3:
  extra_argument: This argument was not used by the format string: "s".
/home/sbrivio/passt/flow.c:290:2:
  printf_function: Calling "vsnprintf" which uses a "printf"-style format string. [Note: The source code implementation of the function has been overridden by a builtin model.]
---

but this warning, coming from 1/6 (oops, I didn't spot that myself),
reported by clang-tidy, is a bit more fundamental for the whole series:

/home/sbrivio/passt/flow.c:281:6: error: function 'flow_log__' is within a recursive call chain [misc-no-recursion,-warnings-as-errors]
  281 | void flow_log__(const struct flow_common *f, int pri, bool perror, bool details,
      |      ^
/home/sbrivio/passt/flow.c:281:6: note: example recursive call chain, starting from function 'flow_log__'
/home/sbrivio/passt/flow.c:312:4: note: Frame #1: function 'flow_log__' calls function 'flow_log__' here:
  312 |                         flow_log__(f, pri, false, false, state,
      |                         ^
/home/sbrivio/passt/flow.c:312:4: note: ... which was the starting point of the recursive call chain; there may be other cycles

...so I didn't proceed for the moment. I guess you meant to call
flow_log() there instead, but I'm not sure I should fix it up like
that, as I guess you checked some specific stuff with this version
which might be broken if I just fix it.

-- 
Stefano