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

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 18 Jan 2024 12:04:15 +1100
David Gibson <david@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@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@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