Re: [PATCH v3 05/15] flow, tcp, tcp_splice: Uniform debug helpers for new flows

On Thu, 18 Jan 2024 12:04:15 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  On Wed, Jan 17, 2024 at 08:59:22PM +0100, Stefano
Brivio wrote:
  On Thu, 21 Dec 2023 18:02:27 +1100
 David Gibson <david(a)gibson.dropbear.id.au> wrote:
   
  When debugging passt/pasta, and the flow table in
particular, one of the
 most obvious things to know is when a new flow is initiated, along with the
 details of its interface, addresses and ports.  Once we've determined to
 what interface the flow should be forwarded, it's useful to know the
 details of how it will appear on that other interface.
 
 To help present that information uniformly, introduce FLOW_NEW_DBG() and
 FLOW_FWD_DBG() helpers and use them for TCP connections, both "tap" and
 spliced.
 
 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
 ---
  flow.c       | 40 ++++++++++++++++++++++++++++++++++++++++
  flow.h       | 16 ++++++++++++++++
  tcp.c        | 11 +++++++++--
  tcp_splice.c |  3 ++-
  4 files changed, 67 insertions(+), 3 deletions(-)
 
 diff --git a/flow.c b/flow.c
 index b9c4a18..bc8cfc6 100644
 --- a/flow.c
 +++ b/flow.c
 @@ -9,6 +9,7 @@
  #include <unistd.h>
  #include <string.h>
  #include <errno.h>
 +#include <arpa/inet.h>
  
  #include "util.h"
  #include "passt.h"
 @@ -50,6 +51,45 @@ void flow_log_(const struct flow_common *f, int pri, const char *fmt,
...)
  	logmsg(pri, "Flow %u (%s): %s", flow_idx(f), FLOW_TYPE(f), msg);
  }
  
 +/**
 + * flow_new_dbg() - Print debug message for new flow
 + * @f:		Common flow structure
 + * @side:	Which side initiated the new flow
 + */
 +void flow_new_dbg(const struct flow_common *f, unsigned side)
 +{
 +	char ebuf[INET6_ADDRSTRLEN], fbuf[INET6_ADDRSTRLEN];
 +	const struct flowside *fside = &f->side[side];
 +
 +	flow_log_(f, LOG_DEBUG, "New %s from %s/%u: [%s]:%hu <-> [%s]:%hu",  
 
 I think we should always print the connection index too (passed from
 the macro, or passing 'conn' as argument here) -- especially if we want
 to correlate this to what flow_fwd_dbg() will print later.   
 
 Printing the index is built into flow_log_(), so we're already doing
 that. 
Oh, right, sorry, I forgot about it.

-- 
Stefano