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

Bcc func load attachtype for bcc > 0.24.0 #311

Merged
merged 3 commits into from
Oct 5, 2022

Conversation

chantra
Copy link
Contributor

@chantra chantra commented Jul 5, 2022

[bcc] Support for new bcc_func_load signature.

In iovisor/bcc@ffff0ed
a new parameter was added to control the attach type. Subsequently, in iovisor/bcc@815d1b8,
-1 was used to signal the default behaviour.

This change adds this new parameter to the call to C.bcc_func_load
when LIBBCC_VERSION is greater than 0.24.0, the last cut version which
did not introduce this change.

This is what BCC is going to be [using as base version in the
future](iovisor/bcc#3808), so
eventually, the current LLVM 9 will break.
@chantra
Copy link
Contributor Author

chantra commented Jul 5, 2022

This patch will be needed once BCC cuts a new release and gobpf wants to support it.

Currently, there is no good way to use the right function signature at compile time. All that BCC provide is LIBBCC_VERSION, which is a string and cannot be compared easily.

What is the stand of gobpf in term of bcc library support? Do we want to have gobpf backward compatible? If this is the case, having BCC expose major/minor/patch would probably help as we could add a wrapper for go to call one variant or the other depending on version.

chantra added a commit to chantra/bcc that referenced this pull request Jul 5, 2022
Summary: Currently, libbcc only exposes its version via a string constant.
This makes it complicated for libraries built on top of libbcc to be compatible across multiple versions when there is API changes for instances, as happens in iovisor/gobpf#311

Exposing MAJOR/MINOR/PATCH versions would allow libraries which are not shipped as part of iovisor/bcc to handle those API changes.

This diffs exposes those values, provides a macro to perform version comparison (a la `LINUX_VERSION()`).

One thing it does not address currently is exposing whether this is an "exact tag" version, e.g a release, or in between releases.

Test plan:

When building:
```
-- Latest recognized Git tag is v0.24.0
-- Git HEAD is 3bab069745fe201fb032574edf87da4c3a8020c7
-- Revision major 0, minor 24, patch 0
-- Revision is 0.24.0-3bab0697
-- Found LLVM: /usr/lib/llvm-11/include 11.1.0 (Use LLVM_ROOT envronment variable for another version of LLVM)
...
...
```

and the produced `bcc_version.h`:
```
$ cat build/src/cc/bcc_version.h

```
chantra added a commit to chantra/bcc that referenced this pull request Jul 10, 2022
Summary: Currently, libbcc only exposes its version via a string constant.
This makes it complicated for libraries built on top of libbcc to be compatible across multiple versions when there is API changes for instances, as happens in iovisor/gobpf#311

Exposing MAJOR/MINOR/PATCH versions would allow libraries which are not shipped as part of iovisor/bcc to handle those API changes.

This diffs exposes those values, provides a macro to perform version comparison (a la `LINUX_VERSION()`).

One thing it does not address currently is exposing whether this is an "exact tag" version, e.g a release, or in between releases.

Test plan:

When building:
```
-- Latest recognized Git tag is v0.24.0
-- Git HEAD is 3bab069745fe201fb032574edf87da4c3a8020c7
-- Revision major 0, minor 24, patch 0
-- Revision is 0.24.0-3bab0697
-- Found LLVM: /usr/lib/llvm-11/include 11.1.0 (Use LLVM_ROOT envronment variable for another version of LLVM)
...
...
```

and the produced `bcc_version.h`:
```
$ cat build/src/cc/bcc_version.h

```
chantra added a commit to chantra/bcc that referenced this pull request Jul 20, 2022
Summary: Currently, libbcc only exposes its version via a string constant.
This makes it complicated for libraries built on top of libbcc to be compatible across multiple versions when there is API changes for instances, as happens in iovisor/gobpf#311

Exposing MAJOR/MINOR/PATCH versions would allow libraries which are not shipped as part of iovisor/bcc to handle those API changes.

This diffs exposes those values, provides a macro to perform version comparison (a la `LINUX_VERSION()`).

One thing it does not address currently is exposing whether this is an "exact tag" version, e.g a release, or in between releases.

Test plan:

When building:
```
-- Latest recognized Git tag is v0.24.0
-- Git HEAD is f26cdb33068187d04d9dc058834985df841e28fa
-- Revision is 0.24.0-f26cdb33 (major 0, minor 24, patch 0)
-- Found LLVM: /usr/lib/llvm-11/include 11.1.0 (Use LLVM_ROOT envronment variable for another version of LLVM)
...
...
```

and the produced `bcc_version.h`:
```
$ cat build/src/cc/bcc_version.h

```
chantra added a commit to chantra/bcc that referenced this pull request Jul 21, 2022
Summary: Currently, libbcc only exposes its version via a string constant.
This makes it complicated for libraries built on top of libbcc to be compatible across multiple versions when there is API changes for instances, as happens in iovisor/gobpf#311

Exposing MAJOR/MINOR/PATCH versions would allow libraries which are not shipped as part of iovisor/bcc to handle those API changes.

This diffs exposes those values, provides a macro to perform version comparison (a la `LINUX_VERSION()`).

One thing it does not address currently is exposing whether this is an "exact tag" version, e.g a release, or in between releases.

Test plan:

When building:
```
-- Latest recognized Git tag is v0.24.0
-- Git HEAD is f26cdb33068187d04d9dc058834985df841e28fa
-- Revision is 0.24.0-f26cdb33 (major 0, minor 24, patch 0)
-- Found LLVM: /usr/lib/llvm-11/include 11.1.0 (Use LLVM_ROOT envronment variable for another version of LLVM)
...
...
```

and the produced `bcc_version.h`:
```
$ cat build/src/cc/bcc_version.h

```
yonghong-song pushed a commit to iovisor/bcc that referenced this pull request Jul 21, 2022
Summary: Currently, libbcc only exposes its version via a string constant.
This makes it complicated for libraries built on top of libbcc to be compatible across multiple versions when there is API changes for instances, as happens in iovisor/gobpf#311

Exposing MAJOR/MINOR/PATCH versions would allow libraries which are not shipped as part of iovisor/bcc to handle those API changes.

This diffs exposes those values, provides a macro to perform version comparison (a la `LINUX_VERSION()`).

One thing it does not address currently is exposing whether this is an "exact tag" version, e.g a release, or in between releases.

Test plan:

When building:
```
-- Latest recognized Git tag is v0.24.0
-- Git HEAD is f26cdb33068187d04d9dc058834985df841e28fa
-- Revision is 0.24.0-f26cdb33 (major 0, minor 24, patch 0)
-- Found LLVM: /usr/lib/llvm-11/include 11.1.0 (Use LLVM_ROOT envronment variable for another version of LLVM)
...
...
```

and the produced `bcc_version.h`:
```
$ cat build/src/cc/bcc_version.h

```
@chantra
Copy link
Contributor Author

chantra commented Jul 22, 2022

iovisor/bcc#4091 made it upstream, so this should be good as it is.

@chantra
Copy link
Contributor Author

chantra commented Jul 22, 2022

@schu what do you think of this PR? Anything you would like me to address?

@muhammednagy
Copy link

Would love to see this get merged, Thank you @chantra

v0.25.0 introduce BCC versioning macros:
iovisor/bcc@83a379e

which can be used to handle API changes.
iovisor/bcc@ffff0ed
a new parameter was added to control the attach type.
Subsequently, in
iovisor/bcc@815d1b8
, `-1` was used to signal the default behaviour.

This change adds this new parameter to the call to `C.bcc_func_load`
when LIBBCC_VERSION_CODE is greater or equal than 0.25.0, the next version to
include the aforementioned change.

Test plan:
Against v0.25.0 is being tested as part of this project CI.

also ran against the current v0.20.0 and ran CI: https://github.com/chantra/gobpf/runs/7436803048
@chantra
Copy link
Contributor Author

chantra commented Aug 30, 2022

bcc v0.25.0 was released. I have updated this PR to reflect it.

@schu this should be good to go.

@daurnimator
Copy link

Will this solve #314 ?

@martinetd
Copy link

Will this solve #314 ?

yes, it should.
Also waiting for a release here before we upgrade bcc in nixpkgs :)

sterchelen pushed a commit to sterchelen/bcc that referenced this pull request Sep 9, 2022
Summary: Currently, libbcc only exposes its version via a string constant.
This makes it complicated for libraries built on top of libbcc to be compatible across multiple versions when there is API changes for instances, as happens in iovisor/gobpf#311

Exposing MAJOR/MINOR/PATCH versions would allow libraries which are not shipped as part of iovisor/bcc to handle those API changes.

This diffs exposes those values, provides a macro to perform version comparison (a la `LINUX_VERSION()`).

One thing it does not address currently is exposing whether this is an "exact tag" version, e.g a release, or in between releases.

Test plan:

When building:
```
-- Latest recognized Git tag is v0.24.0
-- Git HEAD is f26cdb33068187d04d9dc058834985df841e28fa
-- Revision is 0.24.0-f26cdb33 (major 0, minor 24, patch 0)
-- Found LLVM: /usr/lib/llvm-11/include 11.1.0 (Use LLVM_ROOT envronment variable for another version of LLVM)
...
...
```

and the produced `bcc_version.h`:
```
$ cat build/src/cc/bcc_version.h

```
@daurnimator
Copy link

@schu could you review this?

@martinetd
Copy link

hm, @schu does not appear to be a member of https://github.com/orgs/iovisor/people anymore; and there have not been any other committer recently. (Sorry if he has access to the repository in any other way, github does not seem to provide a way of checking if he still has commit access)

please forgive the mass notification -- @iovisor (iirc this notifies everyone in the group) -- is there anyone willing to shepherd gobpf? What would the way forward be here if not?
(We can't upgrade bcc in nixpkgs because we need a gobpf upgrade with this first)

@4ast 4ast merged commit 16120a1 into iovisor:master Oct 5, 2022
@martinetd
Copy link

Thank you @4ast!

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.

5 participants