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

execsnoop: argument to change the number of arguments parsed #1369

Merged
merged 1 commit into from
Oct 12, 2017

Conversation

pchaigno
Copy link
Contributor

@pchaigno pchaigno commented Sep 29, 2017

New argument to change the maximum number of arguments parsed and
displayed.

The maximum number is currently limited to 188 due to the maximum
number of instructions authorized by the verifier. The new smoke test
ensures we get a failing test if the actual maximum decreases.

Note: I proactively limited the number of arguments in argparse to
avoid exposing the user to the verifier's error message.

/cc @brendangregg

@yonghong-song
Copy link
Collaborator

ubuntu 16 failure:

28: Exception: Failed to load BPF program kprobe__sys_execve: Invalid argument
I guess some version of llvm compiler is smart enough to unroll the loop.

@pchaigno pchaigno force-pushed the execsnoop-max-args branch 2 times, most recently from b6b2d2c to d042937 Compare October 1, 2017 16:51
@pchaigno
Copy link
Contributor Author

pchaigno commented Oct 1, 2017

Should be fixed.

I removed the limit on the maximum number of arguments since we cannot know what the actual limit (from the verifier) will be. It depends on how the loop content is compiled. Don't know what I was thinking.

@yonghong-song
Copy link
Collaborator

@brendangregg @goldshtn could you check the patch? This involves an user interface change.

@brendangregg
Copy link
Member

Thanks. It needs an entry on the man page.

I also tried it with "--max-args 1000", and I think the error message is good enough for now:

bpf: Invalid argument. Program too large (7741 insns), at most 4096 insns

New argument to change the maximum number of arguments parsed and
displayed.
@pchaigno
Copy link
Contributor Author

It needs an entry on the man page.

Fixed.

I also tried it with "--max-args 1000", and I think the error message is good enough for now:

Hm, I guess it's not so bad given the option. We could do better if the bcc API exposed the error messages returned by the verifier.

@drzaeus77
Copy link
Collaborator

[buildbot, test this please]

@brendangregg brendangregg merged commit 7bb5233 into iovisor:master Oct 12, 2017
@brendangregg
Copy link
Member

Thanks!

@pchaigno pchaigno deleted the execsnoop-max-args branch October 13, 2017 08:38
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.

4 participants