IP address comparison is incomplete

[lut777]

What reproduces the bug?

I tried the following code.

kprobe:tcp_connect
{
        $sk = ((struct sock *) arg0);
        if ( $sk->__sk_common.skc_daddr == pton("10.244.166.137")){
                printf("find");
        }
}

As pton(const string *addr) described in doc, it could help me to select packets.
But it failed as following error:

./find-gid.bt:40:7-59: ERROR: Type mismatch for '==': comparing 'unsigned int32' with 'unsigned int8[4]'
    if ( $sk->__sk_common.skc_daddr == pton("10.244.166.137")){
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I find the pr #2248, which tried to solve this problem. But it doesn't seems quite finished.

bpftrace --info

[root@node1 bpf-probe]# bpftrace --info
System
OS: Linux 5.14.0-316.el9.x86_64 #1 SMP PREEMPT_DYNAMIC Fri May 19 11:26:13 UTC 2023
Arch: x86_64

Build
version: v0.17.0
LLVM: 15.0.7
unsafe uprobe: no
bfd: yes
libdw (DWARF support): no

Kernel helpers
probe_read: yes
probe_read_str: yes
probe_read_user: yes
probe_read_user_str: yes
probe_read_kernel: yes
probe_read_kernel_str: yes
get_current_cgroup_id: yes
send_signal: yes
override_return: no
get_boot_ns: yes
dpath: yes
skboutput: no

Kernel features
Instruction limit: 1000000
Loop support: yes
btf: yes
map batch: yes
uprobe refcount (depends on Build:bcc bpf_attach_uprobe refcount): yes

Map types
hash: yes
percpu hash: yes
array: yes
percpu array: yes
stack_trace: yes
perf_event_array: yes

Probe types
kprobe: yes
tracepoint: yes
perf_event: yes
kfunc: yes
iter:task: yes
iter:task_file: yes
kprobe_multi: yes
raw_tp_special: yes

[chengshuyi]

Hi, i have tested the upstream code, this problem also exists. Maybe we can provide the itoa function to convert an integer to an array. Or atoi to convert array to integer.

[lut777]

In fact I tried to convert int32 into int8. but I failed.
What I expect is to get IP based in (int8+0), (int8+1), (int8+2), (int8+3). but seems like it is not supported.

[chengshuyi]

In fact I tried to convert int32 into int8. but I failed. What I expect is to get IP based in (int8+0), (int8+1), (int8+2), (int8+3). but seems like it is not supported.

Ok, does itoa which convert integer into byte array meet your needs?

[lut777]

In fact I tried to convert int32 into int8. but I failed. What I expect is to get IP based in (int8+0), (int8+1), (int8+2), (int8+3). but seems like it is not supported.

Ok, does itoa which convert integer into byte array meet your needs?

I think the conversion could fulfil my need.
But I am not quite sure if this conversion could include all the IP storage methods in kernel.

[chengshuyi]

In fact I tried to convert int32 into int8. but I failed. What I expect is to get IP based in (int8+0), (int8+1), (int8+2), (int8+3). but seems like it is not supported.

Ok, does itoa which convert integer into byte array meet your needs?

I think the conversion could fulfil my need. But I am not quite sure if this conversion could include all the IP storage methods in kernel.

emm, what's IP storage methods? Could you give some examples? Thanks.

[lut777]

Now what I know are uint32, *int8, int8[4], I don't know if there is more.

[chengshuyi]

Now what I know are uint32, *int8, int8[4], I don't know if there is more.

ok， see what you mean

[viktormalik]

The problem I see here is that pton returns an array of bytes, however that doesn't always fit the way IP address is stored.

Looking at the pton implementation, it already does int->bytearray conversion so converting it back to int (by adding atoi) doesn't make much sense.

I'd prefer one of the following options:

• Let pton always return an integer (as is done by inet_pton that is used underneath) and then add an itoa builtin to let user convert when necessary. The main issue is that this breaks backwards compatibility by changing the behaviour of pton. Also implementation of itoa will not be simple for a generic case but doing something like "convert int to bytearray" should be easier.
• Extend pton by additional parameter specifying the desired output format/type, i.e. allowing something like pton("127.0.0.1", int32) or pton("127.0.0.1", int8[4]). While this way, we'd avoid the compatibility breakage, the syntax is not very nice, it's pton-specific, and we'd probably have to add some complicated type parsing.
• We could also allow the == operator on (u)int32 and int8[4] operands but I don't really like the language doing such implicit conversions.

I prefer the first option (the pton builtin hasn't been around for long, so not many existing scripts would break) but I'd like to see other's opinions.

[ajor]

+1 for the first option

Although I don't think we should call the function(s) itoa/atoi, as those already have well known meanings different from what is being described here (converting between integers and strings).

Would a casting syntax work instead of a new builtin function?

Relatedly, I've encountered problems where I wanted to treat an "unsigned int8[]" as a string within bpftrace.

[viktormalik]

Although I don't think we should call the function(s) itoa/atoi, as those already have well known meanings different from what is being described here (converting between integers and strings).

Would a casting syntax work instead of a new builtin function?

I think so, something like (int8[])pton("127.0.0.1") should be doable. If not, we should come with a better name than atoi/itoa.

Relatedly, I've encountered problems where I wanted to treat an "unsigned int8[]" as a string within bpftrace.

Yeah, once we introduce casting from/to arrays, we can approach this, too.

[lut777]

I like this idea, so the user could choose the right way to convert.
But how about ipv6? I think an example for IP6 is also necessary.

[xh4n3]

@lut777 I suppose IPv6 addresses in the Linux Kernel are stored as arrays of 16 uint8 elements.

So the example in the reference guide should be sufficient already.

[lut777]

@lut777 I suppose IPv6 addresses in the Linux Kernel are stored as arrays of 16 uint8 elements.

So the example in the reference guide should be sufficient already.

Good to learn that, Thank you! I will try this later in IPv6 comparison.