Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc Ipv6 fixes #4704

Merged
merged 5 commits into from
Aug 20, 2023
Merged

Misc Ipv6 fixes #4704

merged 5 commits into from
Aug 20, 2023

Conversation

jeromemarchand
Copy link
Contributor

Quite a few networking tools have trouble with IPv6. Either it's buggy and or there is no IPv6 support it at all.

Here a re a few fixes. Some more should come.

When logging ipv6 state change, journal_fields tries to pack
event.addr and event.daddr, which is not an integer in this, to
present a bytes-like object to socket.inet_ntop. This can be fixed by
having a similar type for [sd]addr for IPv4 and IPv6. Making both an
array of u32 solves the issue by presenting a bytes-like object
directly to inet_ntop, without the need for the struct packing stage.

Also  now, the similar behavior, makes it easier to factor code for
IPv4 and IPv6.

It solves the following error:
/usr/share/bcc/tools/tcpstates  -Y
SKADDR           C-PID C-COMM     LADDR           LPORT RADDR           RPORT OLDSTATE    -> NEWSTATE    MS
ffff8b2e83e56180 0     swapper/9  ::              22    ::              0     LISTEN      -> SYN_RECV    0.000
Exception ignored on calling ctypes callback function: <function PerfEventArray._open_perf_buffer.<locals>.raw_cb_ at 0x7f894c8d7f70>
Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/bcc/table.py", line 982, in raw_cb_
    callback(cpu, data, size)
  File "/usr/share/bcc/tools/tcpstates", line 419, in print_ipv6_event
    journal.send(**journal_fields(event, AF_INET6))
  File "/usr/share/bcc/tools/tcpstates", line 348, in journal_fields
    'OBJECT_' + addr_pfx + '_SOURCE_ADDRESS': inet_ntop(addr_family, pack("I", event.saddr)),
struct.error: required argument is not an integer
ffff8b2e83e56180 0     swapper/9  2620:52:0:2580:5054:ff:fe6b:6f1f 22    2620:52:0:2b11:2f5e:407d:b35d:4663 60396 SYN_RECV    -> ESTABLISHED 0.010
Exception ignored on calling ctypes callback function: <function PerfEventArray._open_perf_buffer.<locals>.raw_cb_ at 0x7f894c8d7f70>
Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/bcc/table.py", line 982, in raw_cb_
    callback(cpu, data, size)
  File "/usr/share/bcc/tools/tcpstates", line 419, in print_ipv6_event
    journal.send(**journal_fields(event, AF_INET6))
  File "/usr/share/bcc/tools/tcpstates", line 348, in journal_fields
    'OBJECT_' + addr_pfx + '_SOURCE_ADDRESS': inet_ntop(addr_family, pack("I", event.saddr)),
struct.error: required argument is not an integer

Signed-off-by: Jerome Marchand <[email protected]>
Several tcp tools have a --wide option to accommodate longer IPv6
address. However, they seem to assume that IPv6 addresses are at most
26 characters. I don't know where that numbers comes from, but that's
obviously not right: 8 number of 4 digits plus the colons, that would
make 39.

INET6_ADDRSTRLEN is defined as 46 (so 45 characters) probably to
accommodate for IPv6 mixed notation
(ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255).

I don't think that we have to align on 45 characters as I doubt
inet_ntop would return a mixed notation address, but 39 is a must.

Closes iovisor#4460

Signed-off-by: Jerome Marchand <[email protected]>
It isn't used, and it doesn't look like it ever was.

Signed-off-by: Jerome Marchand <[email protected]>
…esses

There is no need for the extra options. We can distinguish IPv4 and
IPv6 adresses by their format.

Signed-off-by: Jerome Marchand <[email protected]>
@yonghong-song
Copy link
Collaborator

Thanks for working on this. I checked the affected tools and they are all working fine.

@yonghong-song
Copy link
Collaborator

I won't consider 26->39 change as bug. '26' still works. just if the width is more than '26', the format will be a little bit mis-aligned with other entries.

@yonghong-song yonghong-song merged commit cbd24ff into iovisor:master Aug 20, 2023
7 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants