From: Nick Alcock <nick.alcock@oracle.com>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com,
Eugene Loh <eugene.loh@oracle.com>
Subject: Re: [PATCH] cg: fix offset for > 8 bit bitfields in dt_cg_ctf_offsetof()
Date: Tue, 26 Aug 2025 18:40:33 +0100 [thread overview]
Message-ID: <87frdegewe.fsf@esperi.org.uk> (raw)
In-Reply-To: <20250826081226.591245-1-alan.maguire@oracle.com> (Alan Maguire's message of "Tue, 26 Aug 2025 09:12:26 +0100")
On 26 Aug 2025, Alan Maguire verbalised:
> The tcp provider uses dt_cg_tramp_get_member() to retrieve the
> offset of the sk_protocol field in struct sock. However it
> returns the wrong value on UEK6 since it is an 8-bit bitfield.
> From pahole we see:
>
> unsigned int __sk_flags_offset[0]; /* 560 0 */
> unsigned int sk_padding:1; /* 560: 0 4 */
> unsigned int sk_kern_sock:1; /* 560: 1 4 */
> unsigned int sk_no_check_tx:1; /* 560: 2 4 */
> unsigned int sk_no_check_rx:1; /* 560: 3 4 */
> unsigned int sk_userlocks:4; /* 560: 4 4 */
> unsigned int sk_protocol:8; /* 560: 8 4 */
>
> In other words it is really at offset 561 but because we just
> lookup the member offset and not the member type offset we get the
> wrong value for the sk_protoocol.
Excellent description! It's annoying as hell that BTF still has this
redundant representation :( DTrace literally always got this wrong,
right back to the year dot.
> The fix is to look up the member _type_ offset and add it to the
> bit offset we get for the member itself. With this in place the
> state-change probes fire, but the local tcp tests still fail due
> to separate issues with the tcp:::accept-established probe.
This is certainly necessary (and your code looks good), but we also need
to ensure that the load is suitably shifted and masked, particularly for
bitfields that cross machine words. DTrace never used to do this
either... e.g. the size returned by dt_cg_ldsize() assumes that the
entire bitfield can be read in one load op, which depending on its
offset may not be true.
It looks like dt_cg_field_get might get this *mostly* right (once you
delete the #if 0'ed stuff -- but I'd be suspicious until I'd tested it
on a big-endian platform), but even that assumes it only needs to do one
load. As soon as a bitfield crosses machine words, I suspect we'll only
get the first half of it :( we might need to do a second load to fetch
the second half of the bitfield, then a shift and | to combine the two,
and DTrace has never had any code for that.
next prev parent reply other threads:[~2025-08-26 17:40 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-26 8:12 [PATCH] cg: fix offset for > 8 bit bitfields in dt_cg_ctf_offsetof() Alan Maguire
2025-08-26 17:40 ` Nick Alcock [this message]
2025-08-28 11:50 ` Alan Maguire
2025-09-02 18:05 ` Eugene Loh
2025-09-03 14:11 ` Alan Maguire
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87frdegewe.fsf@esperi.org.uk \
--to=nick.alcock@oracle.com \
--cc=alan.maguire@oracle.com \
--cc=dtrace-devel@oss.oracle.com \
--cc=dtrace@lists.linux.dev \
--cc=eugene.loh@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox