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

Networking tracing limitations #245

Closed
dalehamel opened this issue Nov 10, 2018 · 24 comments

Comments

@dalehamel
Copy link
Contributor

commented Nov 10, 2018

I noticed that there is already an issue for #30, which I believe would help some here, but isn't the whole solution.

A number of the network tracing tools implemented in bcc have not been implemented in bpftrace (tcpaccept, tcplife, etc). It looks like there are two issues here:

  1. Obtaining the data in the first place, given limitations of bpftrace's struct navigation for kernel functions (more details below).
  2. Displaying the data in a sensible way (covered mostly I think by #30), since the syntax in bpftrace is so limited.

With regards to point 1 above, I discovered a systemtap probe written by cloudflare to make it easy to detect if the tcp accept queue has overflowed (see their blog post for more information).

I took a crack at implementing it in bpftrace (see below) and was able to re-implement much of it. In terms of reading the source and destination addresses for the socket connections though, that's where I had a much harder time. I was expecting to be able to get this information from the socket in my call to tcp_v4_conn_request, where I have access to both the socket sk and socket buffer skb.

The systemtap probe seems to read this information from the socket buffer's network headers, but this data seems more trivially accessible through the socket sk. Other bcc tools use this approach exactly as I intended to, by getting this information out of __sk_common, such as sk->__sk_common.skc_rcv_saddr to get the source address for the socket.

When I try to do this in bpftrace however, it claims that the fields I want are not a member of the __sk_common struct:

Struct/union of type 'sock_common' does not contain a field named 'skc_rcv_saddr'

The bizarre thing is that this works fine if I try to resolve another member of this struct, such as skc_state or skc_family. My theory is that the fields I'm interested in (source/dest host and port) are all stored as unions of structs on the struct, and that bpftrace cannot see these for whatever reason, but for the simpler data types it has no problem. I also tried looking at the socket buffer skb to get what I need (as this is what the systemtap script is doing), but that is even more difficult, as I appear to need to actually be able to copy memory (such as with memcpy) to be able to get these values out of the network headers.

So, figuring out how to overcome getting this data is the first step, the second hard part is to display it. As a start, I would be happy just being able to access these values and print them in hex, as at least they could be decoded elsewhere if needed. in the long term though, it would be great to display them more nicely.

This brings us back to 2 above. For displaying the port, It's likely that it is stored as a simple integer and that there'll be no problem in displaying that, once I'm able to access it. For the addresses, I'm assuming that I'll at least need to do some bit-shifting and masking to be able to display them as human-readable IPv4 addresses (I won't even get into ipv6). Perhaps this will be covered by #30, in which case that would be spectacular.

Here is the progress I've made so far on my script "acceptq.bt", re-implementing the work cloudflare has done:

#!/usr/bin/env bpftrace

#include <linux/tcp.h>
#include <net/sock.h>

BEGIN
{
  printf("Tracing tcp acceptq overflows  Hit Ctrl-C to end.\n");
  printf("%-8s %-16s %-8s %-6s %-6s\n", "TIME", "COMM", "PID", "LEN", "MAX")
}

kprobe:tcp_v4_conn_request
{
  $sk = ((sock *)arg0);
  if ( $sk->sk_ack_backlog > $sk->sk_max_ack_backlog ) {
    time("%H:%M:%S ");
    printf("%-16s %-8d %-6d %-6d\n",
      comm,
      pid,
      $sk->sk_ack_backlog,
      $sk->sk_max_ack_backlog
    )
  }
}

This should be able to show the the processes (which the systemtap script doesn't do!) that hit their maximum accept queue, but unfortunately won't be able to show what client connection(s) caused this to occur, which would be very useful in that event.

Beyond my script here, I'd like to see more network debugging tools be ported over to bpftrace, and I think that overcoming these to problems of 1.) Getting the data and 2.) Displaying it in a human readable way are the two issues.

For 1), I would really appreciate your thoughts / an explanation of why I'm unable to retrieve these struct members, despite bcc being able to. Is this a known limitation, or something new?

@brendangregg

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

Looks like it's an issue with union parsing. Trying to reduce this to something simpler to test:

struct my_sock_common {
        union {
                int      skc_addrpair;
                struct {
                        int  skc_daddr;
                        int  skc_rcv_saddr;
                };
        };
}

BEGIN
{
	$s = (my_sock_common *)0;
	printf("%d\n", $s->skc_addrpair);
}

That hits:

Unknown struct/union: 'my_sock_common'

But should work. I think only @ajor so far knows how the struct parsing works.

As for #30, that's straightforward to add. I'd start with how, say, usym() is implemented: which stores the binary data, then userspace identifies that Type::usym needs to be run through resolve_usym() for printing. Same with inet_ntop(): we'd add a Type::inet and then a resolve_inet() to do the printing. Just need someone who has the time to add this. :)

@brendangregg

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

BTW, I like your tool. We should definitely get it working. It's so nice to see all the probes, arguments, and logic in one screen. Would even fit on a slide.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 13, 2018

BTW, I like your tool. We should definitely get it working. It's so nice to see all the probes, arguments, and logic in one screen. Would even fit on a slide.

Thanks, I'll be happy to share the completed version once it is done, of course. We were able to use it to detect some issues today even without the connection info, so seems to be working.

Looks like it's an issue with union parsing. ... I think only @ajor so far knows how the struct parsing works.

Yeah, my initial guess was that some pretty special handling is required when importing headers and that unions weren't being handled correctly, but I don't know where this code is so can't confirm.

Just need someone who has the time to add this. :)

I'll take a look at the code for usym and give the development docs a read to see if I can come up with a cromulent implementation for inet_ntop as you described, time permitting.

Edit: don't fully understand how the struct parsing is working (not familiar with the clang apis) but I stepped through the bpftrace code mentally using the error message that I got as a starting point and this bit looks really suspicious, I'll see if I can get a bit more info about what this does with some basic debug prints.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

It looks like these are actually getting parsed here:

Parsed: union sock_common::(anonymous at include/net/sock.h:155:2) parent:  ident: skc_addrpair
Parsed: sock_common::(anonymous at include/net/sock.h:157:3) parent:  ident: skc_daddr
Parsed: sock_common::(anonymous at include/net/sock.h:157:3) parent:  ident: skc_rcv_saddr
Parsed: union sock_common::(anonymous at include/net/sock.h:162:2) parent:  ident: skc_hash
Parsed: union sock_common::(anonymous at include/net/sock.h:162:2) parent:  ident: skc_u16hashes
Parsed: union sock_common::(anonymous at include/net/sock.h:167:2) parent:  ident: skc_portpair
Parsed: sock_common::(anonymous at include/net/sock.h:169:3) parent:  ident: skc_dport
Parsed: sock_common::(anonymous at include/net/sock.h:169:3) parent:  ident: skc_num
Parsed: sock_common parent: sock_common ident: skc_family
Parsed: sock_common parent: sock_common ident: skc_state

I notice right away that the struct fields i'm looking for are showing up as anonymous, where as the ones that work (skc_family, skc_state) show up without this. It seems that this is showing up as the actual string identifier, so maybe these idents are getting attached to a struct with a key based on sock_common::(anonymous at include/net/sock.h:169:3) instead of sock_common?

But that might just be appended by the to_string function for this, I haven't looked at the clang code... but this is really fishy, as this is right before this value is used as a key to the map. If this is the case, perhaps we can detect 'anonymous' in the string, and handle it specially? Perhaps there is a more elegant solution, I'll need to read up on clang_getCursorSpelling and clang_getCursorType and a few of the other functions that come up with the name or else a more reliable way of generating the identifier here.

Full output in this gist:
https://gist.github.com/dalehamel/769989b138ff5dc3f31e5fe4d4175a05

This gist has the patch, which shows the fields I'm printing here:
https://gist.github.com/dalehamel/2cbe83e142cf45cfc4cb5511264ba309

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

So the name is being overridden here

And the name is coming from clang_getTypeSpelling

The definition for this function calls getLangOpts(), the (real) definition of which I'm having a hard time finding in the clang source code, but that seems to clearly be where this identifier is coming from.

So, we either need to track this down and fix it at the source, or do something similar to remove_struct_prefix, detecting these sorts of keys and correcting them to match the format that remove_struct_prefix shows. In this case, I think it should:

  • Detect the in the identifier all of union, :: and anonymous, but we should try and figure out what the pretty printer will reliably output for an identifier
  • Use the bytes between union and :: as the identifier (which would, at least in this case, fix the issue)

I worry that this might have unintended side-effects as it's string-based and the "schema" of the pretty printing output isn't something I understand well (yet), but this hints that at least a hacky workaround should be possible.

It looks like the code prefers to use parent as an identifier, and this is special case handling anyways.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

I have a hack that does the above (see gist) but the values that it returns are empty. I would expect that if I look at skc_portpair and print it as hex, I should see the integer representation of the port pair, but I don't see anything.

It might be that the values just can't be read for a reason I don't understand, I'll need to understand how this structs dictionary is used by bpftrace, but for now, i'll take this as progress.

I wonder if this might be a symptom related to this warning in the docs:

This means that many, but not all, structs will be available, and you may need to manually define some structs. In the future, bpftrace will use the newer Linux BTF support so that all kernel structs are always available.

And that this BTF support (not sure what that is) is the solution the author of the doc had in mind?

I'll keep digging as to why this isn't working. Attached in the gist is my patch that allows dereferencing these fields, so I think my rationale above was correct:

https://gist.github.com/dalehamel/09ea1be96595aacc7d44383531157778

@tyroguru

This comment has been minimized.

Copy link
Contributor

commented Nov 14, 2018

Hey Dale - BTF is the new type subsystem for BPF that merged in 4.18. I've yet to have a good look at it but I expect it's a compact type representation suitable for the kernel (similar to the goals of Solaris' CTF type subsystem I would think). Obviously, having an first class type subsystem available for use will be fabulous.

(On a side note, we need DWARF support for userland but that's a whole different kettle of fish...).

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

thanks for the overview @tyroguru, I'll see what I can find to read about it. Do you have any familiarity with the current struct parsing limitations? Why is the DWARF support needed? It looks like the BTF comment in the reference doc is from Brendan Gregg, and that @ajor has been the only one to touch the current clang_parser code.

I'm trying to determine if this is something that can / should be fixed before implementing the BTF support alluded to above, or if that should be a correct fix for this problem.

My curiosity is getting the better of me though, I think I'll keep digging anyways as I'm learning a lot about the bpftrace source code by doing so.

Edit: it looks like behind the scenes, this code is what is being called during code generation, so i suspect something to do with the type handling here is why this data isn't showing up

Edit2: it looks like the case above the one that actually gets called is what handles nested structs, my theory is that because this struct is part of a union it isn't being correctly detected here, and the members are being treated as integers, which is why I am getting 0's for my values.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

I have a more complete theory as to why this is not working based on my debugging / reading through the code.

I believe the issue with getting these values is that, because they are anonymous structs / unions, they do not actually have a 'field' value associated with them.

For this reason, the type 'struct' isn't ever detected, and they are being handled incorrectly as if they are integer types directly on sock_common. I think that they need to be handled as structs in order to correctly retrieve these values - as a pointer, and not as a simple integer type.

So, the real bug here is that anonymous structs / unions seem to be completely ignored by the clang parser.

@tyroguru

This comment has been minimized.

Copy link
Contributor

commented Nov 14, 2018

thanks for the overview @tyroguru, I'll see what I can find to read about it. Do you have any familiarity with the current struct parsing limitations? Why is the DWARF support needed? It looks like the BTF comment in the reference doc is from Brendan Gregg, and that @ajor has been the only one to touch the current clang_parser code.

@dalehamel - sounds like you're having fun with this anyway :-) . No, I can't provide any authoritative appraisal of the current situation you find yourself in so sorry about that. BTF only merged in the 4.18 kernel so it's very new but that will hopefully provide full typing for the kernel that can be integrated into bpftrace. DWARF support is needed for userland type support. It is the de-facto type information emitted by all compilers nowadays and being able to access that for a running process opens up an incredible wealth of tracing possibilities. I have no idea at the minute how to integrate that into bpftrace but it will need to be done if userland tracing to be optimal.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

DWARF support is needed for userland type support.

Ah i see, so this would make it possible to say, deal with various userspace types / get values from userspace code in a similar way to how it is done in the kernel, if I understand you correctly.

No, I can't provide any authoritative appraisal of the current situation you find yourself in so sorry about that.

No worries, I think I've got a decent grasp of the problem now - that anonymous unions/structs are the issue with the current parsing approach. I believe that this is a legitimate bug in the parsing, and I'm not certain that BTF would help in this specific case.

Thanks for your input @tyroguru

@brendangregg

This comment has been minimized.

Copy link
Member

commented Nov 14, 2018

Just a comment on user-land type support (not kernel):

  • DWARF works, and we could support it at some point
  • BTF will probably change this space
  • USDT probes may allow type support in the future (@mmarchini will talk about this at LPC on Thursday)
  • I think a practical workaround in many cases would be people declaring a few structs in their script that they use, or create a header file for them. If I'm uprobe-ing something and I just need one user-space struct, I'm not going to wait for DWARF/BTF/USDT-structs, I could just declare that struct in the script (and its dependencies). I do this a couple of times in tools for kernel structs: see dcsnoop.bt and runqlen.bt.
@tyroguru

This comment has been minimized.

Copy link
Contributor

commented Nov 14, 2018

I completely agree @brendangregg that the workaround of including structs in scripts works but that is exactly that, a workaround. That becomes pretty untenable in big userland code bases where the code is more complex (at least in layout) and much larger than kernel code bases. Importing header files becomes a gigantic pain in the butt and past experience shows that just doesn't work.

Not sure what the userland story will be like for BTF so that will be interesting to see. However, adding another step into debug section generation is going to be a difficult sell to application developers in both time and space. I'm personally convinced that having full type availability in userland is utterly essential for a nirvana tracing system and at the minute I think that is DWARF. Obviously USDT will supplement that.

Didn't know about LPC! Hrrmph. Maybe next year.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

Didn't know about LPC! Hrrmph. Maybe next year.

Ditto, found out about it just yesterday and was quite disappointed, looks like there's lots of really cool eBPF talks this year. Hope they record them!

To the actual problem at hand in this issue though, I'm quite convinced I understand the problem, but I've run up against this foreboding warning in the dev docs:

If there's one gotcha I would like to mention, it's the use of CreateGEP() (Get Element Pointer). It's needed when dereferencing at an offset in a buffer, and it's tricky to use.

That seems to be the problem here, I believe that GetElementPointer is returning the wrong offset when an anonymous struct / union is used, which is why the values come back as empty in my case (though conceivably, they could also return non-zero values depending on whatever address is being accessed incorrectly here).

Note that this is only even possible with my other hacky patch applied. I might be able to cobble a fix together, but I hope that @ajor can chime in on some thoughts for how to handle anonymous unions/structs.

Perhaps I should file a separate issue for this, as that seems to be the actual technical challenge here (and the fact that it helps with network tracing is only incidental, given that skc_addrpair and skc_portpair are both stored as anonymous unions of a struct with another integer data type.

I'm a bit surprised, as anonymous members of nested anonymous structs are supposed to be treated as members of the parent struct. This would seem to be an odd edge-case / quirk.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 15, 2018

After all this... I believe this might actually be a simple fix.

It would appear that the offsets are just calculated incorrectly for anonymous structs, because they have a different parent than the other members of the "root" struct. Using clang_getCursorSemanticParent, followed by the existing mechanisms to resolve this to a printable name, I can see the relative distance to the sock_common struct:

             CXCursor pc = clang_getCursorSemanticParent(c);
             CXCursor gpc = clang_getCursorSemanticParent(pc);
             CXCursor ggpc = clang_getCursorSemanticParent(gpc);
             auto real_parent = get_clang_string(clang_getCursorSpelling(pc));
             auto grand_parent = get_clang_string(clang_getCursorSpelling(gpc));
             auto great_grand_parent = get_clang_string(clang_getCursorSpelling(ggpc));
             std::cout << "Parent: " << struct_name << " pc: " << real_parent << " gpc: " << grand_parent << " ggpc: " << great_grand_parent << " ident: " << ident << std::endl;

Looking at the field offsets, it makes sense that these fields are calculated relative to their parent only. In the case of something like skc_daddr though, it's parent is an anonymous struct, whose parent is an anonymous union, whose parent is then (finally) sock_common, but the offsets are displayed relative to the parent only.

This probably explains why I am getting mostly zeros in my output - most of these end up pointing to the same memory address near the start of the struct, at offsets between 0-4.

So, all I need to do is write a function to:

  1. Check if the parent as resolved by clang_getCursorSemanticParent matches the parent that was passed in to this function. If it doesn't:
  2. Check ancestry until we find the matching parent, calculating the offset at each level and building a running total.
  3. Use the resulting offset, that is correctly relative to the expected parent.

I'll see if i can prototype this, if I get something working I'll submit a pull request to fix calculating offsets for anonymous structs.

Full output (sorry it's interleaved and a bit hard to read): https://gist.github.com/dalehamel/958098d3cf3f0a2a7ee76e34d10a8514.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 21, 2018

I've figured out the issue with anonymous structs / indirect fields and have a PR up for review (#262).

I'm also taking a look at fixing #30 now that I have an easy way to get ipv4 addresses, and want to see them printed nicely instead of in hexadecimal. I'll see about getting a PR up for that as well.

Unfortunately, I don't think I'll be able to get my sample script above working with just these two PRs though, as it would require additional functionality.

Specifically, the systemtap script I was trying to re-implement got the ip adresses and ports out of the socket buffer, I believe this is the only way to get these values when tracing tcp_v4_conn_request as the fields on the socket are mostly 0's - I guess this is before the network header in the socket buffer is parsed. Currently (unlike systemtap) bpftrace doesn't offer any mechanism to read values out of a buffer instead of just reading struct fields. Perhaps I'll submit an issue asking for opinions around a new builtin to read N bytes from an arbitrary buffer at X offset, but that feels like it will be quite difficult to make safe / intuitive.

I have thought of a few other minor enhancements, such as parsing out #define macros from headers, so that things like AF_INET will be available, rather than having to specify the literal integer 2.

Sufficed to say, once the two PRs mentioned above are merged (and in the case of inet_ntop, written), it should be possible to nearly fully re-implement bcc's tcpaccept as a bpftrace script and should pave the way for more sophisticated network tracing.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 23, 2018

Using a hex/bitshifting workaround, I was able to do a proof-of-concept for tcpdrop:

#!/usr/bin/env bpftrace

/*
  Watches for all tcp_drops and examines socket state
*/

#include <linux/tcp.h>
#include <net/sock.h>

BEGIN
{
  printf("Tracing tcp drops. Hit Ctrl-C to end.\n");
  printf("%-8s %-16s %-21s %-20s\n", "TIME", "COMM", "DEST:PORT", "SRC:PORT")
}

kprobe:tcp_drop
{
  $sk = ((sock *) arg0);

  if ($sk->__sk_common.skc_family ==2) {
    $daddr = $sk->__sk_common.skc_daddr;
    $saddr = $sk->__sk_common.skc_rcv_saddr;
    $lport = $sk->__sk_common.skc_num;
    $dport = $sk->__sk_common.skc_dport;

    $mask1 = 0x000000FF;
    $mask2 = 0x0000FF00;
    $mask3 = 0x00FF0000;
    $mask4 = 0xFF000000;

    $d_q1 = ($daddr & $mask1) >> 0;
    $d_q2 = ($daddr & $mask2) >> 8;
    $d_q3 = ($daddr & $mask3) >> 16;
    $d_q4 = ($daddr & $mask4) >> 24;

    $s_q1 = ($saddr & $mask1) >> 0;
    $s_q2 = ($saddr & $mask2) >> 8;
    $s_q3 = ($saddr & $mask3) >> 16;
    $s_q4 = ($saddr & $mask4) >> 24;

    time("%H:%M:%S ");

    printf("%-16s %d.%d.%d.%d:%-4d\t", comm, $d_q1, $d_q2, $d_q3, $d_q4, $dport);
    printf("%d.%d.%d.%d:%-8d\n", $s_q1, $s_q2, $s_q3, $s_q4, $lport);
  }
}

So now that #262 has been merged, more basic network tracing tools like tcpaccept should be able to be implemented in bpftrace using a workaround like this, until #30 has been implemented.

In any case, this is big progress! The above template should be easily recyclable to build stuff like tcpaccept.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2018

I've done some basic ports of several of the tcp* utils from iovisor/bcc using the recently merged indirect struct fields patch, and the hex/bitshifting approach to ipv4 parsing.

https://gist.github.com/dalehamel/2b34a12e66c1975345b7582911db6a0f

Currently implemented:

  • tcpacceptqsnoop.bt (tcpaccept.py + backlog info)
  • tcpdrop.bt (tcpdrop.py, ipv4 only)
  • tcpconnect.bt (tcpconnect.py, ipv4 only)
  • tcpretrans.bt (tcpretrans.py, ipv4 only, no tail loss probes)

I've done side-by-side comparisons of output, and minus ipv6 support they all produce the same output as their python/bcc counterparts.

tcplife will be a bit trickier to implement but should be doable.

inet_ntop support should fix the ipv4 limitations.

@brendangregg

This comment has been minimized.

Copy link
Member

commented Nov 24, 2018

Nice work. It'd be good to have these in bpftrace/tools. Although you'll need to write example files and man pages for each. Plus I'd follow the top block comment style from other tools, eg:

#!/usr/bin/env bpftrace
/*
 * capable	Trace security capabilitiy checks (cap_capable()).
 *		For Linux, uses bpftrace and eBPF.
 *
 * USAGE: capable.bt
 *
 * This is a bpftrace version of the bcc tool of the same name.
 *
 * Copyright 2018 Netflix, Inc.
 * Licensed under the Apache License, Version 2.0 (the "License")
 *
 * 08-Sep-2018	Brendan Gregg	Created this.
 */

There's also an important line in the man page for these bcc ports:

This is a bpftrace version of the bcc tool of the same name. The bcc tool
provides options to customize the output.

That lets us write these nice short bpftrace tools, and defer complex and niche options to the bcc versions.

Plus, it helps if workarounds are commented as such, with the issue number. Eg:

$ grep workaround *.bt
opensnoop.bt:// workaround for #132: the next two code blocks can't be combined yet
opensnoop.bt:	$ret = *(ctx + 16);		// workaround for #132
runqlat.bt:	$TASK_RUNNING = 0;	// from linux/sched.h, workaround for #153

BTW, after the other work you've done, you'll probably find #30 not that hard to do, at least, for IPv4. First version can print that IPv6 isn't supported yet from the semantic analyzer.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2018

It'd be good to have these in bpftrace/tools.

That's definitely my intention. I'll get a PR started for that and follow your style / doc recommendations. I believe there's also a guide for contributing tools, I'll give that a read as well.

Plus, it helps if workarounds are commented as such, with the issue number. Eg:

I'll get the initial versions of these tools checked-in as-is, but document that they are waiting on the inet_ntop support as described.

BTW, after the other work you've done, you'll probably find #30 not that hard to do, at least, for IPv4

Yeah I believe I know where the changes are needed and already started the patch per your advice, but I've missed something (I think in codegen) that is causing my changes to not be loaded correctly. I understand conceptually what needs to happen, but I've missed something specific. I'm going to keep plugging at this, just thought I'd put my efforts into the bitshifting workarounds for now.

I'll give it another go and see if I can start with something really basic for resolving a custom type, and iterate from there. I believe I'll be able to figure it out with a bit of time, just need to gain a bit more context around the codepaths and debug this a bit more.

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Nov 25, 2018

@brendangregg I've added these scripts and ported over their associated examples and docs from bcc, and revised them to (I think) meet the standards you indicated above, I think it's ready for a review at #267.

I've also opened #268, which I think is the last piece of the puzzle to be able to close this out, as this functionality is needed to read and network and transport headers from socket buffers, as bcc and systemtap are capable of doing, per my previous comment #245 (comment)

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 4, 2018

With #269 and #285, i think much of the dominos are in place to resolve this issue, as I'll be able to finish off #267 to have inet_ntop, supporting ipv6.

In order to do that though, a solution to #284 is needed, as ipv6 addresses are 128 bit types which are currently not handled by bpftrace.

A combination of array indexing and custom struct fields should be enough to allow for casting raw socket buffers to a type that can be accessed (I think), which would cover the remaining use cases for reading data directly from socket buffers. That's probably the last outstanding bit for full network debugging support on par with what we see in the current bcc tools.

@mmarchini mmarchini added the meta label Jan 14, 2019

@dalehamel

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

with @mmarchini 's recent work for supporting ipv6, i think we might be able to close this off.

The only issue I'm currently aware of with network-related tracing is to do with cases where struct field offsets can't be reliably parsed, which usually comes up with more complex structs when doing dynamic (kprobe) tracing.

@mmarchini

This comment has been minimized.

Copy link
Member

commented May 22, 2019

The only issue I'm currently aware of with network-related tracing is to do with cases where struct field offsets can't be reliably parsed, which usually comes up with more complex structs when doing dynamic (kprobe) tracing.

I think this is covered in other issues, and since this is not exclusive to Networking I'll close this one. If there's anything left to be done here feel free to reopen it.

@mmarchini mmarchini closed this May 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.