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

RFC: Add skboutput function #1272

Closed
wants to merge 4 commits into from
Closed

Conversation

olsajiri
Copy link
Contributor

heya,
please take as RFC, it's by no means final code ;-)
I just need to hear some feedback on wether you guys think this is useful

I tried to put together something we discussed with Arnaldo and Jiri Benc
on LPC - storing skb_buff via bpf helper from any probe with 'skb' argument

the new bpftrace function has following syntax:

skboutput(file, skb, caplen)

it will store packet from 'skb' object to tcpdump 'file' and limit the size by 'caplen', for example:

# bpftrace -e 'kfunc:napi_gro_receive { skboutput("krava.pcap", args->skb, 1500);  }'
Attaching 1 probe...
...

# tcpdump -n -r ./krava.pcap
reading from file ./krava.pcap, link-type RAW (Raw IP)
dropped privs to tcpdump
02:43:07.069621 unknown ip 0
02:43:07.069640 unknown ip 0
02:43:07.084865 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 3619840828, win 1867, options [nop,nop,TS val 862106286 ecr 226183539], length 0
02:43:07.097883 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 53, win 1867, options [nop,nop,TS val 862106298 ecr 226183554], length 0
02:43:07.110942 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 105, win 1867, options [nop,nop,TS val 862106311 ecr 226183567], length 0
02:43:07.125958 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 157, win 1867, options [nop,nop,TS val 862106326 ecr 226183581], length 0
02:43:07.126282 unknown ip 0
02:43:07.138008 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 209, win 1867, options [nop,nop,TS val 862106339 ecr 226183596], length 0
02:43:07.138969 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 261, win 1867, options [nop,nop,TS val 862106339 ecr 226183596], length 0
02:43:07.151006 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 313, win 1867, options [nop,nop,TS val 862106351 ecr 226183608], length 0
02:43:07.152040 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 365, win 1867, options [nop,nop,TS val 862106352 ecr 226183609], length 0
02:43:07.153369 unknown ip 0
02:43:07.164084 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 417, win 1867, options [nop,nop,TS val 862106364 ecr 226183621], length 0
02:43:07.165010 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 469, win 1867, options [nop,nop,TS val 862106365 ecr 226183622], length 0
02:43:07.165020 IP 10.40.192.62.40032 > 10.37.152.221.ssh: Flags [.], ack 521, win 1867, options [nop,nop,TS val 862106366 ecr 226183623], length 0
...

you can use skboutput function from multiple places/probes with different files

it seems useful for debuging network stuff, but what do I know..? ;-)
I haven't got too much positive feedback yet to continue on with this

thanks for any thoughts on this

@fbs fbs added do-not-merge Changes are not ready to be merged into master yet RFC Request for comment labels Apr 16, 2020
@fbs
Copy link
Contributor

fbs commented Apr 16, 2020

I like it. It is a useful thing to have and easier than digging into a skb straight away.

Being able to do bpftrace -e '{ magic($skb) } | tcpdump -r - would be useful too.

@danobi
Copy link
Member

danobi commented Apr 16, 2020

Does libpcap handle changes to struct sk_buff? skb layout probably doesn't change that often but I still wonder.

@olsajiri
Copy link
Contributor Author

Does libpcap handle changes to struct sk_buff? skb layout probably doesn't change that often but I still wonder.

so there's skboutput bpf kernel helper that gets you skb data and store it to perf map/buffer,
so it's quite easy code, similar in the process as printf/syscat/cat functions

the pcap writer on the user space side is quite basic at the moment, but I've got few pointers where to get some examples of how to make it better and more robust

@liuhangbin
Copy link

Hi Jiri,

Is there any progress on this RFC? I feel this feature is useful for networking debug. And it could be used on my bpftrace version of dropwath.

Thanks
Hangbin

@olsajiri
Copy link
Contributor Author

Hi Jiri,

Is there any progress on this RFC? I feel this feature is useful for networking debug. And it could be used on my bpftrace version of dropwath.

heya,
@liuhangbin the code is rebased, but I think we need small kernel change to work more user friendly, because now it drops packets that are bigger than caplen, and I think we could add flag to skb_output not to drop them but cut them

also it'd be great if you paste in here some of your usage examples so I have some case when asking for the kernel change

thanks,
jirka

@liuhangbin
Copy link

@olsajiri , thanks for your works. I will clone the code and try it first. After that, I will paste my testing script here.

@liuhangbin
Copy link

liuhangbin commented Jul 14, 2021

Basically, I would use this feature for bpftrace version of dropwatch . The current version only implements the dropwatch feature. But for bpftrace version of dwdump, the skboutput function is needed.

And an example use case looks like
bpftrace -e 't:skb:kfree_skb { skboutput("kfree_skb.pcap", args->skbaddr, 1500); }'

Detect pcap developement libraries and set LIBPCAP_FOUND
if the library is detected.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri olsajiri force-pushed the skboutput branch 2 times, most recently from a279d76 to dd6a1d4 Compare July 29, 2021 14:00
Adding PCAPwriter class as a wrapper to write tcpdump
output files.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Adding skb_output check to features and display
its state in --info output.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Adding skboutput function that allows to dump 'struct sk_buff'
object to the tcpdump output file, like:

  # bpftrace -e 'kfunc:napi_gro_receive { skboutput("krava.pcap", args->skb, 1500); }'
  ...

  # tcpdump -n -r ./krava.pcap  | head -3
  reading from file ./krava.pcap, link-type RAW (Raw IP)
  dropped privs to tcpdump
  19:24:36.665948 IP 10.40.195.119.57218 > 10.37.152.221.ssh: Flags [.], ack 576407458, win 502, options [nop,nop,TS val 697277009 ecr 2101346304], length 0
  19:24:36.679313 IP 10.40.195.119.57218 > 10.37.152.221.ssh: Flags [.], ack 53, win 502, options [nop,nop,TS val 697277022 ecr 2101346304], length 0
  19:24:36.692242 IP 10.40.195.119.57218 > 10.37.152.221.ssh: Flags [.], ack 105, win 502, options [nop,nop,TS val 697277035 ecr 2101346304], length 0

the function has following syntax:

  skboutput(file, skb, caplen)

it will store packet from 'skb' object to tcpdump 'file' and limit the size by 'caplen

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@@ -1077,6 +1077,39 @@ void CodegenLLVM::visit(Call &call)
b_.CreateLifetimeEnd(buf);
expr_ = nullptr;
}
else if (call.func == "skboutput")
{
std::vector<llvm::Type *> elements = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we keep these in src/ast/async_event_types.h so theyre all defined in one place

@xh4n3
Copy link
Contributor

xh4n3 commented May 16, 2022

@olsajiri Hi Jiri, are you still working on this feature? It seems very helpful for digging network issues.
I've rebased your commits to the latest branch on my machine, and it works. If you need some help on the remaining job, I can take a look at it.

If you use skboutput("krava.pcap", args->skb, args->skb->len - 4);, no packet will be dropped, only some truncated by less than 4 bytes. Looks like an alignment issue, I will dig into it.

@olsajiri
Copy link
Contributor Author

@olsajiri Hi Jiri, are you still working on this feature? It seems very helpful for digging network issues. I've rebased your commits to the latest branch on my machine, and it works. If you need some help on the remaining job, I can take a look at it.

If you use skboutput("krava.pcap", args->skb, args->skb->len - 4);, no packet will be dropped, only some truncated by less than 4 bytes. Looks like an alignment issue, I will dig into it.

@xh4n3 great, feel free to rebase/fix/push it ;-) I dropped that because I thought there's no interest in this,
not sure when I could get to it, so it'll be faster if you can do that, thanks

@xh4n3 xh4n3 mentioned this pull request May 19, 2022
3 tasks
@fbs fbs closed this May 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge Changes are not ready to be merged into master yet RFC Request for comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants