Skip to content

wip: new: runtime tracepoints management#577

Closed
FedeDP wants to merge 1 commit intocleanup/unify_simpleconsumer_simpledriverfrom
new/runtime_tp_management
Closed

wip: new: runtime tracepoints management#577
FedeDP wants to merge 1 commit intocleanup/unify_simpleconsumer_simpledriverfrom
new/runtime_tp_management

Conversation

@FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Sep 2, 2022

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap

/area libsinsp

Does this PR require a change in the driver versions?

/version driver-API-version-major

new ioctl introduced in kmod.

What this PR does / why we need it:

This PR completes part of #521 allowing even tracepoints to be runtime modified, just like we allow it for ppm_sc.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@FedeDP
Copy link
Contributor Author

FedeDP commented Sep 2, 2022

This PR was branched from #521.

@poiana poiana requested review from LucaGuerra and leogr September 2, 2022 13:57
@poiana poiana added the approved label Sep 2, 2022
}
else
{
if (ret == 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a small bug fix on top of #521 that is not actually needed in that PR, as tracepoints are only set at startup time and we don't need g_tracepoints_attached to be actually synced (and nobody will ever touch 'g_tracepoints_attached` at runtime).

if(put_user(PPM_SCHEMA_CURRENT_VERSION, out))
ret = -EINVAL;
goto cleanup_ioctl_nolock;
} else if (cmd == PPM_IOCTL_GET_TPMASK) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New ioctl to get current g_tracepoints_attached.
We could also cache this info userspace side, but i think driver is always more accurate (ie: if some tracepoints could not be attached/detached, that is reflected on the kmod side g_tracepoints_attached but not in the userspace-side requested tp_of_interest).

size_t m_ncpus;
char* m_lasterr;
int m_bpf_prog_fds[BPF_PROGS_MAX];
struct bpf_prog m_bpf_progs[BPF_PROGS_MAX];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merged prog_fds and event_fd; plus, store tracepoint name for further usage.

int m_bpf_event_fd[BPF_PROGS_MAX];
int m_bpf_map_fds[BPF_MAPS_MAX];
int m_bpf_prog_array_map_idx;
char m_filepath[PATH_MAX];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Store probe filepath; used in scap_bpf.

{
if (strcmp(handle->m_bpf_progs[i].name, shname) == 0)
{
already_attached = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check if the requested tracepoint is already attached (ie: if we already have its name in m_bpf_progs).
Avoid attaching it twice.

ASSERT(false);
return SCAP_FAILURE;
uint32_t op = oargs->tp_of_interest.tp[i] ? SCAP_TPMASK_SET : SCAP_TPMASK_UNSET;
scap_kmod_handle_tp_mask(engine, op, i);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the new function here.

}

switch(tp)
static int32_t update_single_tp_of_interest(int tp, bool interesting)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New API to manage a single tp of interest.

break;
}

if (ppm_sc >= PPM_SC_MAX)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the checks at scap level: there were indeed no checks here, and one could provide invalid data.

break;
}

if (tp >= TP_VAL_MAX)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

{
throw sinsp_exception("you cannot use this method before opening the inspector!");
}
if (ppm_sc >= PPM_SC_MAX || ppm_sc < 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Managed at scap level now.

This is the same API we introduced for runtime ppm_sc management in #521,
but for tracepoints.
It is implemented for kmod, bpf and modern_bpf.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP FedeDP force-pushed the new/runtime_tp_management branch from 4f94c74 to 5054494 Compare September 2, 2022 14:09
@poiana
Copy link
Contributor

poiana commented Sep 2, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana deleted the branch cleanup/unify_simpleconsumer_simpledriver September 6, 2022 14:11
@poiana poiana closed this Sep 6, 2022
@FedeDP
Copy link
Contributor Author

FedeDP commented Sep 6, 2022

/reopen

@poiana
Copy link
Contributor

poiana commented Sep 6, 2022

@FedeDP: Failed to re-open PR: state cannot be changed. The cleanup/unify_simpleconsumer_simpledriver branch has been deleted.

Details

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants