Re: [PATCH v2 0/4] DTrace TCP provider

[Kris Van Hees <kris.van.hees@oracle.com>]

On Tue, Jul 01, 2025 at 03:08:59PM -0400, Eugene Loh wrote:
> Incidentally, looking at the subject lines for the patch series, I see:
> 
>         dtrace: move get_member() to dt_cg.c
>         dt_impl: bump number of TSLOTS to 8
>         dtrace: add tcp provider
>         dtrace: sync dlibs with tcp.d, ip.d and net.d changes

I would suggest:

	cg: move get_member() to dt_cg.c
	cg: bump number of TSLOTS to 8
	tcp: new provider -or- Implement the tcp provider
	dlibs: sync dlibs with tcp.d, ip.d and net.d changes

> That is, each line is prefixed with a component name.  We do that a lot for
> "test:", but otherwise the practice seems to depend on... the patch author? 
> Anyhow, I claim "dtrace:" is not very useful.  For patch 1, all the prefix
> is saying is "there is movement in the dtrace code base."  I think the
> subject would be more direct without the prefix.  Same for patch 2.  For
> patch 3, if one wanted to use a component prefix, I'd think one would use
> "tcp:".
> 
> Historically, it looks like we've added new providers with subject lines
> like this:
> 
>     rawfbt: new provider
>     Implement the io provider
>     Implement the ip provider
>     Implement the lockstat provider
>     Implement the sched provider (first part)
>     Implement the proc provider
>     provider: Implement a rawtp provider
>     Add a CPC provider
>     PID provider implementation
>     Add a profile provider
>     Implement BEGIN and END probes for the dtrace provider
>     Added IO provider support for 4.14 kernels
> 
> Everyone has their own style.  Shrug.  No big deal.
> 
> Anyhow, personally, I think the prefixes don't add much, especially for
> "dtrace:".  My suggestion is to drop the prefixes, but... your call.
> 
> On 6/10/25 09:58, Alan Maguire wrote:
> 
> > This series is a first draft of TCP provider support, where the
> > probes are implemented via underlying fbt and sdt probes.
> > 
> > Due to the use of the sock/inet_sock_set_state tracepoint, intended
> > for ~5.15 kernels and later.  Tried replacing this with
> > 
> > fbt::tcp_set_state:entry
> > 
> > but this misses a few state transitions, so stuck with using
> > the tracepoint.
> > 
> > All tests under test/unittest/tcp pass unmodified on an upstream
> > (6.15) kernel and 5.15 UEK7U3 kernel.
> > 
> > It implements all documented TCP provider probes:
> > 
> > accept-established, accept-refused, connnect-request,
> > connect-established, connect-refused, receive, send,
> > state-change
> > 
> > Changes since RFC:
> > 
> > - fixed issues with test failures on UEK7 due to missing
> >    SYN_RCV state change
> > - moved get_member() to dt_cg.c (patch 1)
> > 
> > Alan Maguire (4):
> >    dtrace: move get_member() to dt_cg.c
> >    dt_impl: bump number of TSLOTS to 8
> >    dtrace: add tcp provider
> >    dtrace: sync dlibs with tcp.d, ip.d and net.d changes
> > 
> >   dlibs/aarch64/5.14/ip.d  |   1 -
> >   dlibs/aarch64/5.14/net.d |   6 +-
> >   dlibs/aarch64/5.14/tcp.d |  52 ++---
> >   dlibs/aarch64/5.16/ip.d  |   1 -
> >   dlibs/aarch64/5.16/net.d |   6 +-
> >   dlibs/aarch64/5.16/tcp.d |  52 ++---
> >   dlibs/aarch64/6.1/ip.d   |   1 -
> >   dlibs/aarch64/6.1/net.d  |   6 +-
> >   dlibs/aarch64/6.1/tcp.d  |  52 ++---
> >   dlibs/aarch64/6.10/ip.d  |   1 -
> >   dlibs/aarch64/6.10/net.d |   6 +-
> >   dlibs/aarch64/6.10/tcp.d |  52 ++---
> >   dlibs/x86_64/5.14/ip.d   |   1 -
> >   dlibs/x86_64/5.14/net.d  |   6 +-
> >   dlibs/x86_64/5.14/tcp.d  |  52 ++---
> >   dlibs/x86_64/5.16/ip.d   |   1 -
> >   dlibs/x86_64/5.16/net.d  |   6 +-
> >   dlibs/x86_64/5.16/tcp.d  |  52 ++---
> >   dlibs/x86_64/6.1/ip.d    |   1 -
> >   dlibs/x86_64/6.1/net.d   |   6 +-
> >   dlibs/x86_64/6.1/tcp.d   |  52 ++---
> >   dlibs/x86_64/6.10/ip.d   |   1 -
> >   dlibs/x86_64/6.10/net.d  |   6 +-
> >   dlibs/x86_64/6.10/tcp.d  |  52 ++---
> >   libdtrace/Build          |   2 +
> >   libdtrace/dt_cg.c        |  39 ++++
> >   libdtrace/dt_cg.h        |   2 +
> >   libdtrace/dt_impl.h      |   2 +-
> >   libdtrace/dt_prov_ip.c   |  45 +----
> >   libdtrace/dt_prov_tcp.c  | 405 +++++++++++++++++++++++++++++++++++++++
> >   libdtrace/dt_provider.c  |   1 +
> >   libdtrace/dt_provider.h  |   1 +
> >   libdtrace/ip.d           |   1 -
> >   libdtrace/net.d          |   6 +-
> >   libdtrace/tcp.d          |  52 ++---
> >   35 files changed, 761 insertions(+), 267 deletions(-)
> >   create mode 100644 libdtrace/dt_prov_tcp.c
> >