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

Limit number of generated BPF programs #2141

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

viktormalik
Copy link
Contributor

@viktormalik viktormalik commented Feb 4, 2022

Originates from this comment.

In some cases (e.g. when using the probe builtin), the number of generated BPF functions may be very large which may cause bpftrace to hang. This is a safeguard that sets a limit of 512 BPF functions by default. The limit can be changed using the
BPFTRACE_MAX_BPF_PROGS environment variable.

Checklist
  • Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md
  • The new behaviour is covered by tests

@viktormalik viktormalik force-pushed the probe-expansion-limit branch from 5d6a200 to 62419e0 Compare February 4, 2022 15:00
std::to_string(probe_count_) +
" BPF programs, which exceeds the current limit of " +
std::to_string(bpftrace_.max_programs_) +
" and it could take a long time.\n"
Copy link
Member

Choose a reason for hiding this comment

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

I think we can leave this out. I'm more worried about the amount of system resources you end up stealing if you attach so many programs than it taking long

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed "take a long time" to "take a lot of resources" in docs. Should be ready for another review.

probe_count_ += matches.size();
if (probe_count_ > bpftrace_.max_programs_)
{
throw std::runtime_error(
Copy link
Member

Choose a reason for hiding this comment

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

Using LOG(FATAL) here might be nicer for the end user

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't say, the exception is caught and translated to LOG(ERROR) while LOG(FATAL) will terminate the program with SIGABRT which is not very user-friendly.

Copy link
Member

Choose a reason for hiding this comment

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

ah we catch it, nice :)

@viktormalik viktormalik force-pushed the probe-expansion-limit branch 2 times, most recently from 2c7c79e to 28af20c Compare February 8, 2022 10:50
@viktormalik viktormalik requested a review from fbs February 10, 2022 08:32
In some cases (e.g. when using the "probe" builtin), the number of
generated BPF functions may be very large which may cause BPFtrace to
hang. This change prevents that by setting a limit on the number of
generated BPF functions. The limit can be changed using the
BPFTRACE_MAX_BPF_PROGS environment variable.
@viktormalik viktormalik force-pushed the probe-expansion-limit branch from 28af20c to c0d7841 Compare February 10, 2022 08:35
@fbs fbs merged commit 79ef3e0 into bpftrace:master Feb 10, 2022
@viktormalik viktormalik deleted the probe-expansion-limit branch February 11, 2022 09:43
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