[DTrace-devel] [PATCH 1/3] Add support for the link_ntop() subroutine
Kris Van Hees
kris.van.hees at oracle.com
Sat Dec 9 05:29:06 UTC 2023
On Tue, Aug 29, 2023 at 09:54:31PM -0400, eugene.loh at oracle.com wrote:
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> bpf/Build | 1 +
> bpf/link_ntop.S | 127 ++++++++++++++++++
> libdtrace/dt_cg.c | 27 +++-
> test/unittest/funcs/err.link_ntopbadaddr.r | 3 +
> test/unittest/funcs/err.link_ntopbadarg.r | 3 +
> test/unittest/funcs/tst.link_ntop.d | 1 -
> .../funcs/tst.link_ntop_runtime_type.d | 85 ++++++++++++
> .../funcs/tst.link_ntop_runtime_type.r | 8 ++
> 8 files changed, 253 insertions(+), 2 deletions(-)
> create mode 100644 bpf/link_ntop.S
> create mode 100644 test/unittest/funcs/err.link_ntopbadaddr.r
> create mode 100644 test/unittest/funcs/err.link_ntopbadarg.r
> create mode 100644 test/unittest/funcs/tst.link_ntop_runtime_type.d
> create mode 100644 test/unittest/funcs/tst.link_ntop_runtime_type.r
>
> diff --git a/bpf/Build b/bpf/Build
> index 917e8a0e..a165cbd3 100644
> --- a/bpf/Build
> +++ b/bpf/Build
> @@ -30,6 +30,7 @@ bpf_dlib_SOURCES = \
> index.S \
> inet_ntoa.S \
> inet_ntoa6.S \
> + link_ntop.S \
> lltostr.S \
> mutex_owned.S \
> mutex_owner.S \
> diff --git a/bpf/link_ntop.S b/bpf/link_ntop.S
> new file mode 100644
> index 00000000..20a6ee42
> --- /dev/null
> +++ b/bpf/link_ntop.S
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> + */
> +
> +#define BPF_FUNC_probe_read 4
> +
> +#define ARPHRD_ETHER 1
> +#define ARPHRD_INFINIBAND 32
Maybe add that these coe from <linux/if_arp.h> ?
> +
> +#define ETH_ALEN 6 /* Octets in ethernet addr <linux/if_ether.h> */
> +#define INFINIBAND_ALEN 20 /* Octets in IPoIB HW addr <linux/if_infiniband.h> */
> +
> +#define MAX_ALEN 20
> +
> +#define LENGTH 72
> +
> + .text
> +
> +/*
> + * uint64_t write_byte2hex(const uint8_t src, char *dst)
> + */
> + .align 4
> + .global write_byte2hex
> + .type write_byte2hex, @function
> +write_byte2hex:
> + /*
> + * Branch-free implementation of byte-to-hex function.
> + *
> + * Process 4 bits (one hex digit) at a time.
> + *
> + * Given: r3 in [0, 15].
> + * Then, ((-(r3 - 9)) >> 63) is 1 if r3 in [10, 15], and 0 otherwise.
> + * Therefore, the hex digit (character) representing r3 is:
> + * r3 + '0' + ((-(r3 - 9)) >> 63) * ('a' - '0' - 10)
> + */
> +
> +.macro WRITE_DIGIT n
> + /* Put digit n from %r1 into %r3. */
> + mov %r3, %r1
> + rsh %r3, 4 * (1 - \n)
> + and %r3, 0xf
> +
> + /* Store the converted value into %r2. */
> + mov %r4, %r3
> + sub %r4, 9
> + neg %r4
> + rsh %r4, 63
> + mul %r4, 'a' - '0' - 10
> + add %r4, '0'
> + add %r4, %r3
> + stxb [%r2 + \n], %r4
> +.endm
> +
> + WRITE_DIGIT 0
> + WRITE_DIGIT 1
> +
> + exit
> + .size write_byte2hex, .-write_byte2hex
This might be worth separating out along with the write_hex16 in inet_ntoa6,
as most generic functions. Something for a later patch.
> +
> +/*
> + * void link_ntop(const dt_dctx_t *dctx, const uint8_t *src, char *dst, uint32 type)
> + */
> + .align 4
> + .global dt_link_ntop
> + .type dt_link_ntop, @function
> +dt_link_ntop:
> + /*
> + * %r9 dst pointer
> + * %r8 src copy
> + * %r7 len left
> + *
> + * We copy the src string so that we can safely dereference individual
> + * bytes. The src string is at most MAX_ALEN bytes. The dst string is
> + * three times the src length. The dst space is a tstring, which is
> + * known to be at least 72 bytes. This is long enough for both src copy
> + * and dst string since, as we write the dst string, src bytes are
> + * being used up. So the dst string will start at byte 0 while src
> + * will be copied to offset LENGTH-MAX_ALEN.
> + */
> +
> + /* Write pointer to dst. */
> + mov %r9, %r3
> +
> + /* Copy of src data. */
> + mov %r8, %r3
> + add %r8, LENGTH - MAX_ALEN
> +
> + /* Store len in %r7 based on arg4. */
> + mov %r7, ETH_ALEN
> + jeq %r4, ARPHRD_INFINIBAND, .Lib
> + jne %r4, ARPHRD_ETHER, .Lerror
> + ja .Lcomp
> +.Lib:
> + mov %r7, INFINIBAND_ALEN
> +.Lcomp:
> +
> + /* Copy src data. */
> + mov %r3, %r2
> + mov %r2, %r7
> + mov %r1, %r8
> + call BPF_FUNC_probe_read
> + jne %r0, 0, .Lerror
> +
> + /* Loop over bytes. Each byte is appended with a ':'. */
> +.Lloop:
> + jle %r7, 0, .Ldone
> + ldxb %r1, [%r8 + 0]
> + mov %r2, %r9
> + call write_byte2hex
> + stb [%r9 + 2], ':'
> + add %r9, 3
> + add %r8, 1
> + sub %r7, 1
> + ja .Lloop
> +.Ldone:
> + stb [%r9 + -1], 0
> + mov %r0, 0
> + exit
> +
> +.Lerror:
> + /* Output the terminating NUL byte and return. */
> + stb [%r9 + 0], 0
> + mov %r0, 0
> + exit
> +
> + .size dt_link_ntop, .-dt_link_ntop
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index bb73efd7..5210d71d 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -6039,6 +6039,31 @@ dt_cg_subr_inet_ntop(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_reg, tstring_offset));
> }
>
> +/* Definitions from libdtrace/net.d or /usr/include/linux/if_arp.h. */
> +#define ARPHRD_ETHER 1
> +#define ARPHRD_INFINIBAND 32
Why not simply include <linux/if_arp.h> ?
> +
> +static void
> +dt_cg_subr_link_ntop(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> +{
> + dt_node_t *type = dnp->dn_args;
> + uint_t Lokay = dt_irlist_label(dlp);
> +
> + /* Determine type and check it is okay. */
> + dt_cg_node(type, dlp, drp);
> + emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, type->dn_reg, ARPHRD_INFINIBAND, Lokay));
> + emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, type->dn_reg, ARPHRD_ETHER, Lokay));
> + dt_cg_probe_error(yypcb, DTRACEFLT_ILLOP, DT_ISREG, type->dn_reg);
> + emitl(dlp, Lokay,
> + BPF_NOP());
> +
> + /* Move addr into dnp->dn_args so we can call dt_cg_subr_arg_to_tstring(). */
> + dnp->dn_args = type->dn_list;
You probably should do this in a way that after the dt_cg_subr_arg_to_tstring()
call, you can restore the node structure. I.e. dnp->dn_args = type. That way,
depending on what type is, dt_node_free() will ensure proper cleanup is done
when dt_node_free( is done on dnp.
But worse, this causes the dt_cg_check_ptr_arg() in dt_cg_subr_arg_to_tstring()
to get bypassed (and thus err.link_ntopbadaddr.d fails) because you move the
2nd argument into the 1st argument spot, and then dt_cg_subr_arg_to_tstring()
checks the type of the *1st* argument of link_type() according to its
prototype. And since that is not a pointer, the check is never done.
Rather than remapping the args (exchanging type and addr), I think you need
to keep them as they are, and do (assuming addr = dnp->dn_args):
dt_cg_node(addr, dlp, drp);
dt_cg_check_ptr_arg(dlp, drp, addr, NULL);
dt_cg_subr_arg_to_tstring(dnp, dlp, drp, "dt_link_ntop", DT_ISREG, addr->dn_reg, DT_IGNOR, 0);
But then you do need to rework dt_link_ntop to accept type as the main argument
and addr as the extra argument.
> +
> + dt_cg_subr_arg_to_tstring(dnp, dlp, drp, "dt_link_ntop", DT_ISREG, type->dn_reg);
> + dt_regset_free(drp, type->dn_reg);
> +}
> +
> typedef void dt_cg_subr_f(dt_node_t *, dt_irlist_t *, dt_regset_t *);
>
> static dt_cg_subr_f *_dt_cg_subr[DIF_SUBR_MAX + 1] = {
> @@ -6087,7 +6112,7 @@ static dt_cg_subr_f *_dt_cg_subr[DIF_SUBR_MAX + 1] = {
> [DIF_SUBR_INET_NTOA] = &dt_cg_subr_inet_ntoa,
> [DIF_SUBR_INET_NTOA6] = &dt_cg_subr_inet_ntoa6,
> [DIF_SUBR_D_PATH] = NULL,
> - [DIF_SUBR_LINK_NTOP] = NULL,
> + [DIF_SUBR_LINK_NTOP] = &dt_cg_subr_link_ntop,
> };
>
> static void
> diff --git a/test/unittest/funcs/err.link_ntopbadaddr.r b/test/unittest/funcs/err.link_ntopbadaddr.r
> new file mode 100644
> index 00000000..b798b5e0
> --- /dev/null
> +++ b/test/unittest/funcs/err.link_ntopbadaddr.r
> @@ -0,0 +1,3 @@
> +-- @@stderr --
> +dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): invalid address ({ptr}) in action #1 at BPF pc NNN
> +
> diff --git a/test/unittest/funcs/err.link_ntopbadarg.r b/test/unittest/funcs/err.link_ntopbadarg.r
> new file mode 100644
> index 00000000..e386a67c
> --- /dev/null
> +++ b/test/unittest/funcs/err.link_ntopbadarg.r
> @@ -0,0 +1,3 @@
> +-- @@stderr --
> +dtrace: error on enabled probe ID 3 (ID 1: dtrace:::BEGIN): illegal operation in action #1 at BPF pc NNN
> +
> diff --git a/test/unittest/funcs/tst.link_ntop.d b/test/unittest/funcs/tst.link_ntop.d
> index 0548ffec..7a9ceea2 100644
> --- a/test/unittest/funcs/tst.link_ntop.d
> +++ b/test/unittest/funcs/tst.link_ntop.d
> @@ -4,7 +4,6 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
>
> #pragma D option quiet
>
> diff --git a/test/unittest/funcs/tst.link_ntop_runtime_type.d b/test/unittest/funcs/tst.link_ntop_runtime_type.d
> new file mode 100644
> index 00000000..d28056de
> --- /dev/null
> +++ b/test/unittest/funcs/tst.link_ntop_runtime_type.d
> @@ -0,0 +1,85 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +#pragma D option quiet
> +
> +uint8_t ibaddr[20];
> +
> +BEGIN
> +{
> + mytype = ARPHRD_ETHER;
> +
> + this->buf_eth = (uint64_t *)alloca(sizeof(uint64_t));
> +
> + /* Ethernet MAC address 00:01:02:03:04:05 */
> + *(this->buf_eth) = htonll(0x000102030405 << 16);
> + printf("%s\n", link_ntop(mytype, this->buf_eth));
> +
> + /* Ethernet MAC address ff:ff:ff:ff:ff:ff */
> + *(this->buf_eth) = htonll(0xffffffffffff << 16);
> + printf("%s\n", link_ntop(mytype, this->buf_eth));
> +
> + /* Ethernet MAC address 01:33:0a:00:00:01 */
> + *(this->buf_eth) = htonll(0x01330a000001 << 16);
> + printf("%s\n", link_ntop(mytype, this->buf_eth));
> +
> + /* Ethernet MAC address 00:10:e0:8a:71:5e */
> + *(this->buf_eth) = htonll(0x0010e08a715e << 16);
> + printf("%s\n", link_ntop(mytype, this->buf_eth));
> +
> + /* Ethernet MAC address 2:8:20:ac:4:e */
> + *(this->buf_eth) = htonll(0x020820ac040e << 16);
> + printf("%s\n", link_ntop(mytype, this->buf_eth));
> +
> + mytype = ARPHRD_INFINIBAND;
> +
> + ibaddr[0] = 0x10;
> + ibaddr[1] = 0x80;
> + ibaddr[2] = 0x08;
> + ibaddr[3] = 0x08;
> + ibaddr[4] = 0x20;
> + ibaddr[5] = 0x0c;
> + ibaddr[6] = 0x41;
> + ibaddr[7] = 0x7a;
> + ibaddr[8] = 0x01;
> + ibaddr[9] = 0x7f;
> + ibaddr[10] = 0x01;
> + ibaddr[11] = 0xff;
> + ibaddr[12] = 0xff;
> + ibaddr[13] = 0x7f;
> + ibaddr[14] = 0x01;
> + ibaddr[15] = 0xff;
> + ibaddr[16] = 0xfe;
> + ibaddr[17] = 0x7f;
> + ibaddr[18] = 0x01;
> + ibaddr[19] = 0xff;
> + printf("%s\n", link_ntop(mytype, ibaddr));
> +
> + ibaddr[0] = 0x80;
> + ibaddr[1] = 0x00;
> + ibaddr[2] = 0x04;
> + ibaddr[3] = 0x04;
> + ibaddr[4] = 0xFE;
> + ibaddr[5] = 0x8C;
> + ibaddr[6] = 0x41;
> + ibaddr[7] = 0x7A;
> + ibaddr[8] = 0x00;
> + ibaddr[9] = 0x00;
> + ibaddr[10] = 0x00;
> + ibaddr[11] = 0x00;
> + ibaddr[12] = 0x00;
> + ibaddr[13] = 0x02;
> + ibaddr[14] = 0xC9;
> + ibaddr[15] = 0x02;
> + ibaddr[16] = 0x00;
> + ibaddr[17] = 0x29;
> + ibaddr[18] = 0x31;
> + ibaddr[19] = 0xCD;
> + printf("%s\n", link_ntop(mytype, ibaddr));
> +
> + exit(0);
> +}
> diff --git a/test/unittest/funcs/tst.link_ntop_runtime_type.r b/test/unittest/funcs/tst.link_ntop_runtime_type.r
> new file mode 100644
> index 00000000..0aee1c23
> --- /dev/null
> +++ b/test/unittest/funcs/tst.link_ntop_runtime_type.r
> @@ -0,0 +1,8 @@
> +00:01:02:03:04:05
> +ff:ff:ff:ff:ff:ff
> +01:33:0a:00:00:01
> +00:10:e0:8a:71:5e
> +02:08:20:ac:04:0e
> +10:80:08:08:20:0c:41:7a:01:7f:01:ff:ff:7f:01:ff:fe:7f:01:ff
> +80:00:04:04:fe:8c:41:7a:00:00:00:00:00:02:c9:02:00:29:31:cd
> +
> --
> 2.18.4
>
>
More information about the DTrace-devel
mailing list