Re: [DTrace-devel] [PATCH] test/utils: add more reliable "get remote address" approach

[Alan Maguire <alan.maguire@oracle.com>]

On 07/07/2025 17:53, Kris Van Hees wrote:
> On Mon, Jul 07, 2025 at 05:32:19PM +0100, Alan Maguire wrote:
>> On 03/07/2025 23:36, Kris Van Hees wrote:
>>> On Thu, Jul 03, 2025 at 04:59:44PM -0400, Kris Van Hees wrote:
>>>> On Thu, Jul 03, 2025 at 09:23:46PM +0100, Alan Maguire wrote:
>>>>> On 03/07/2025 20:03, Kris Van Hees wrote:
>>>>>> On Thu, Jul 03, 2025 at 07:41:41PM +0100, Alan Maguire wrote:
>>>>>>> On 03/07/2025 19:26, Kris Van Hees wrote:
>>>>>>>> On Thu, Jul 03, 2025 at 07:02:57PM +0100, Alan Maguire wrote:
>>>>>>>>> On 03/07/2025 18:06, Eugene Loh wrote:
>>>>>>>>>> On 7/3/25 12:59, Alan Maguire wrote:
>>>>>>>>>>
>>>>>>>>>>> On 03/07/2025 17:43, Eugene Loh wrote:
>>>>>>>>>>>
>>>>>>>>>>>> I tested and it looks good (modulo the OL8 UEK6 issue mentioned in the
>>>>>>>>>>>> patch 3/4 feedback).
>>>>>>>>>>>>
>>>>>>>>>>> Sorry I couldn't find that issue; is this the 5.15 problem with the ip
>>>>>>>>>>> send probes?
>>>>>>>>>>
>>>>>>>>>>     dtrace: failed to compile script /dev/stdin:
>>>>>>>>>>     ".../build/dlibs/5.2/tcp.d", line 177: failed to resolve type of
>>>>>>>>>> inet_ntoa arg#1 (ipaddr_t *):
>>>>>>>>>>     Unknown type name
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Ah, sorry yep I have a fix for that one in the next round. Basically we
>>>>>>>>> need to add it to the core set of typedefs and add a type for a pointer
>>>>>>>>> to ipaddr_t; we can't rely on the #pragma to include net.d unfortunately.
>>>>>>>>
>>>>>>>> Why can't we rely on the pragma?  That is how e.g. the ip provider manages
>>>>>>>> this I believe?
>>>>>>>>
>>>>>>>
>>>>>>> Unfortunately the #pragma include doesn't do enough; it just defines a
>>>>>>> type for ipaddr_t , not a type for a _pointer_ to an ipaddr_t , which is
>>>>>>> what we need as a parameter to inet_ntoa(). I tried adding the ipaddr_t
>>>>>>> typedef to net.d and doing the pointer lookup/addition but that doesn't
>>>>>>> work either. Seems we need the core typedef + pointer addition or we hit
>>>>>>> this failure.
>>>>>>
>>>>>> Actually, if you move 'typedef __be32          ipaddr_t;' from ip.d to net.d,
>>>>>> you should be set.  That is what I did in my priliminary tcp provider impl.
>>>>>> I do believe that works.  Either way, we use inet_ntoa() in the ip.d
>>>>>> translators and that works with that typedef in the file, so this really ought
>>>>>> to work.
>>>>
>>>>> Yep, I tried that in the v2 patch series; Eugene hit the undefined error
>>>>> in one test and I now hit it consistently for all tcp/ip tests
>>>>> unfortunately with "typedef __be32 ipaddr_t;" in net.d.
>>>>>
>>>>> My assumption (probably wrong) is that the include of the library does
>>>>> happen but nothing triggers the pointer type generation for "ipaddr *"
>>>>> in the CTF dict. If there was a way to force that type generation at the
>>>>> .d file level that would be great, not sure I see a way currently tho.
>>>>
>>>> Well, like I said, it does work for ip.d so I don't see why this would be
>>>> any different.  I'll have a look and see if I can figure something out.
>>>
>>> Looking into this more, I think the problem is simply that you did not sync
>>> all the dlibs for the various kernel versions with the updated ip.d, net.d, and
>>> tcp.d files.  So, if the kernel on the OL8 instance you test on does not have
>>> your change, it will fail.
>>>
>>
>> No, don't think that's it; the .d files that matched the kernel I tested
>> on (6.10) were synced; the use of the 6.10 .d files was visible in the
>> error message. The problem appears to be around the fact that tcp.d uses
>> the ipaddr_t * in inet_ntoa(), but unlike ip.d (which uses ipaddr_t in
>> translated types) it does not have any other mention of ipaddr_t.
>> Adding an explicit cast in tcp.d to the argument to inet_ntoa() to
>> ipaddr_t * resolves the issue without having to add ipaddr_t to the core
>> type list.
> 
> Can you reproduce this at will?  Can you give me specifics on OL version,
> kernel version, etc?  I'd like to be able to reproduce what you see, because
> so far, all I tried actually works once the ipaddr_t typedef is in net.d.
>

Yep, it's 100% reproducible for me on an upstream (bpf-next 6.15) kernel
+ OL9. Moving ipaddr_t to net.d works for ip.d but not tcp.d in that
environment. The extra casts for the inet_ntoa() parameters that I
mention above are needed in tcp.d to get things to work properly for me.

I pushed a branch to

https://github.com/alan-maguire/dtrace-utils/tree/remote-tcp-v3-wip-broken

that illustrates the failure.

Relative to devel, it consists of 6 commits

1: the v2 of the remote IP address change (ensuring the remote address
tests won't fail);
2-4: a few prep patches for the tcp provider; and
5: the tcp provider patch (in a v3 work-in-progress form); and finally
6: the top-level commit then removes the casts I added to tcp.d in the
previous "tcp: new provider" commit. With that change in place on my
system, the previously-passing IP tests start failing.

If I "git reset --hard HEAD~1" on that branch (reestablishing those
ipaddr_t * casts) and rebuild, the failures go away for me.

>> Further debug logging shows that the actual type addition happens in an
>> order which can cause potentially cause problems in some cases; for
>> example with the above changes I see:
>>
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/errno.d
>> sorted (1/2)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/io.d sorted (3/4)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/ip.d sorted (5/6)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/lockstat.d
>> sorted (7/8)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/tcp.d sorted
>> (10/11)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/udp.d sorted
>> (12/13)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/net.d sorted
>> (9/14)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/pcap.d sorted
>> (15/16)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/procfs.d
>> sorted (17/18)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/regs.d sorted
>> (19/20)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/sched.d
>> sorted (21/22)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/signal.d
>> sorted (23/24)
>> libdtrace DEBUG 1751904270: library /usr/lib64/dtrace/6.10/unistd.d
>> sorted (25/26)
>> libdtrace DEBUG 1751904270: typedef processorid_t added as id 2147483677
>> libdtrace DEBUG 1751904270: typedef psetid_t added as id 2147483678
>> libdtrace DEBUG 1751904270: typedef chipid_t added as id 2147483679
>> libdtrace DEBUG 1751904270: typedef lgrp_id_t added as id 2147483680
>> libdtrace DEBUG 1751904270: typedef cpuinfo_t added as id 2147483682
>> libdtrace DEBUG 1751904270: typedef cpuinfo_t_p added as id 2147483684
>> libdtrace DEBUG 1751904270: typedef time_t added as id 2147483688
>> libdtrace DEBUG 1751904270: typedef timestruc_t added as id 2147483690
>> libdtrace DEBUG 1751904270: typedef lwpsinfo_t added as id 2147483695
>> libdtrace DEBUG 1751904270: typedef taskid_t added as id 2147483696
>> libdtrace DEBUG 1751904270: typedef dprojid_t added as id 2147483697
>> libdtrace DEBUG 1751904270: typedef poolid_t added as id 2147483698
>> libdtrace DEBUG 1751904270: typedef zoneid_t added as id 2147483699
>> libdtrace DEBUG 1751904270: typedef psinfo_t added as id 2147490324
>>
>> [edit: following typedefs come from net.d:]
>>
>> libdtrace DEBUG 1751904270: typedef conninfo_t added as id 2147490329
>> libdtrace DEBUG 1751904270: typedef netstackid_t added as id 2147490330
>> libdtrace DEBUG 1751904270: typedef ipaddr_t added as id 2147490331
>> libdtrace DEBUG 1751904270: typedef in6_addr_t added as id 2147490332
>> libdtrace DEBUG 1751904270: typedef pktinfo_t added as id 2147490334
>> libdtrace DEBUG 1751904270: typedef csinfo_t added as id 2147490336
>> libdtrace DEBUG 1751904270: skipping library
>> /usr/lib64/dtrace/6.10/udp.d: "/usr/lib64/dtrace/6.10/udp.d", line 10:
>> program requires provider udp
>>
>> [edit: followig typedefs come from tcp.d:]
>>
>> libdtrace DEBUG 1751904270: typedef tcpinfo_t added as id 2147490338
>> libdtrace DEBUG 1751904270: typedef tcpsinfo_t added as id 2147490340
>> libdtrace DEBUG 1751904270: typedef tcplsinfo_t added as id 2147490342
>>
>> [edit: following typedefs come from ip.d;]
>>
>> libdtrace DEBUG 1751904270: typedef ipinfo_t added as id 2147490350
>> libdtrace DEBUG 1751904270: typedef ifinfo_t added as id 2147490352
>> libdtrace DEBUG 1751904270: typedef ipv4info_t added as id 2147490361
>> libdtrace DEBUG 1751904270: typedef ipv6info_t added as id 2147490369
>> libdtrace DEBUG 1751904270: typedef void_ip_t added as id 2147490370
>> libdtrace DEBUG 1751904270: typedef __dtrace_tcp_void_ip_t added as id
>> 2147490371
>> libdtrace DEBUG 1751904270: typedef caddr_t added as id 2147490380
>> libdtrace DEBUG 1751904270: typedef bufinfo_t added as id 2147490382
>> libdtrace DEBUG 1751904270: typedef devinfo_t added as id 2147490385
>>
>>
>> Notice that the ip.d typedefs are added after the tcp ones, despite
>> tcp.d having a "#pragma D depends_on provider ip.d . So it seems like
>> the "depends_on library net.d" was honoured above, but not the
>> "depends_on provider ip.d". Moving the core net-generic definitions to
> 
> Ah, but depends_on provider is different from depends_on library.  The
> dependency on the provider means that DTrace will not load the .d file if
> the provider is not available.  The dependency on a library file affects
> the sorting, because it indicates that the given library file needs to be
> loaded before the one that specifies the depends_on.
>

okay, good to know, thanks!

>> net.d alone was not enough however; including the explicit casts to the
>> argument to inet_ntoa() (to ipaddr_t *) in tcp.d was needed also.
>>  > Also, I do not understand why you removed the pragma
>>> 	#pragma D depends_on provider tcp	
>>> from tcp.d.
>>
>> Yeah, not sure why that got removed; I'll add it again for v3.
>>
>> Alan