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

Selftests are not run against each version of libbpf #95

Open
grantseltzer opened this issue Nov 29, 2021 · 13 comments
Open

Selftests are not run against each version of libbpf #95

grantseltzer opened this issue Nov 29, 2021 · 13 comments

Comments

@grantseltzer
Copy link
Contributor

We have the file selftests/.libbpf-versions.txt which should allow us to run each selftest against each version of libbpf which would be checked out in the git submodule. This doesn't appear to be the case.

@grantseltzer grantseltzer added the bug Something isn't working label Nov 29, 2021
@grantseltzer
Copy link
Contributor Author

I can handle this task, but can you confirm I'm not misunderstanding something @rafaeldtinoco ?

@rafaeldtinoco
Copy link
Contributor

This doesn't appear to be the case.

What do you mean here ? My idea is that, since we have libbpf git submodule, we can checkout a specific commit ID of that git tree (by entering libbpf directory and checking the commit/tag specifically) and run "make selftest-static-run".

This procedure will build libbpfgo with the checked out libbpf version and run selftests with that libbpf version.

Does that make sense to you ?

@rafaeldtinoco
Copy link
Contributor

This was done in a way that you could rapidly bisect libbpf OR libbpfgo to find out incompatibility issues in between them (thus the -static- Makefile targets).

@grantseltzer
Copy link
Contributor Author

This doesn't appear to be the case.

What do you mean here ? My idea is that, since we have libbpf git submodule, we can checkout a specific commit ID of that git tree (by entering libbpf directory and checking the commit/tag specifically) and run "make selftest-static-run".

This procedure will build libbpfgo with the checked out libbpf version and run selftests with that libbpf version.

Does that make sense to you ?

That makese sense, but running make selftest-static-run doesn't seem to accomplish this. Does it automatically checkout/build the different versions of libbpf and re-build each selftest? I don't see the .libbpf-versions.txt file referenced anywhere.

@rafaeldtinoco
Copy link
Contributor

This doesn't appear to be the case.

What do you mean here ? My idea is that, since we have libbpf git submodule, we can checkout a specific commit ID of that git tree (by entering libbpf directory and checking the commit/tag specifically) and run "make selftest-static-run".
This procedure will build libbpfgo with the checked out libbpf version and run selftests with that libbpf version.
Does that make sense to you ?

That makese sense, but running make selftest-static-run doesn't seem to accomplish this. Does it automatically checkout/build the different versions of libbpf and re-build each selftest? I don't see the .libbpf-versions.txt file referenced anywhere.

This was meant to be a manual test, not an automatic one. Do you think we should automate different libbpf versions selftests ? Why ? My original intent was to find out changes to blame if something was broken... but maybe I missed something else this could be executed for.

@grantseltzer
Copy link
Contributor Author

I see no reason not to automate it. Exactly for that reason, this way we know what features do or don't work with particular versions of libbpf. I'll go ahead and remove the bug label. Do you feel against automating it?

@grantseltzer grantseltzer removed the bug Something isn't working label Nov 30, 2021
@rafaeldtinoco
Copy link
Contributor

I'll go ahead and remove the bug label. Do you feel against automating it?

Never! =). Thanks for checking this!

@itaysk
Copy link
Collaborator

itaysk commented Dec 1, 2021

libbpfgo is released for specific libbpf version (as indicated by libbpfgo version name), so don't we need to test it only with that compatible version?

@rafaeldtinoco
Copy link
Contributor

libbpfgo is released for specific libbpf version (as indicated by libbpfgo version name), so don't we need to test it only with that compatible version?

automating will allow us to know if recent changes to libbpfgo broke compatibility for previous released versions of libbpf. executing by hand will allow us to bisect libbpfgo (and libbpf) development to know what commit to blame for in case something brakes (already used couple of times).

considering that libbpfgo might be used with libbpf shared library (and not only with the static version of the library), it could be useful to detect that automatically (but not mandatory to have this automated).

@itaysk
Copy link
Collaborator

itaysk commented Dec 1, 2021

know if recent changes to libbpfgo broke compatibility for previous released versions of libbpf

why do we need this, if each release of libbpfgo supports a specific release of libbpf? we only need to test with currently supported libbpf, no?

@rafaeldtinoco
Copy link
Contributor

know if recent changes to libbpfgo broke compatibility for previous released versions of libbpf

why do we need this, if each release of libbpfgo supports a specific release of libbpf? we only need to test with currently supported libbpf, no?

well can't we position "being backwards compatible" as a "best effort" at least ? Mainly because Linux distros will have their shared library (libbpf) in place and others might have to rely on it (or prefer to rely on it). Or else we should get rid of the "dynamic" library support IMO (and stick with the static only method).

@itaysk
Copy link
Collaborator

itaysk commented Dec 2, 2021

I don't know what's the value in this. let's say we have these automated tests, and we discover that libbpfgo for libbpf 5 doesn't work with libbpf 4. Do we try to fix that? I think not, since that libbpfgo was made specifically for libbpf 5. I think we want automated tests to test that every libbpfgo version works with it's companion libbpf, and that's it. But maybe I'm missing something.
Regarding dynamic vs static loading that's a separate discussion IMO. I think I'd be ok if we decide to remove dynamic, but we can discuss in a new issue.

@rafaeldtinoco
Copy link
Contributor

I don't know what's the value in this. let's say we have these automated tests, and we discover that libbpfgo for libbpf 5 doesn't work with libbpf 4. Do we try to fix that?

Depends on the root cause, no ? If it is a change in the ABI (likely, because something was deprecated/removed before the ABI stabilization at 1.0), then it's a question of "do we want to support that and be compatible to both ? Is it even possible ?".

Knowing that all distros with libbpf shared libraries released, until that moment, will only support v0.3 and v0.4 and, if an application is dynamically linked to libbpf, it will fail for those distros (or containers with those distros).

OR we only want apps using libbpfgo to use static libbpf linking (which would be ok as well, and would avoid the distribution versioning dependency).

I think not, since that libbpfgo was made specifically for libbpf 5. I think we want automated tests to test that every libbpfgo version works with it's companion libbpf, and that's it. But maybe I'm missing something.

libbpfgo was made specifically for libbpf 5.

If allowing dynamic linking, we can't guarantee users will only try a specific libbpf version. Yes, it can be a requirement, but OTOH, if we're only supporting static compilation, there is no need to worry about distribution versions, the project using libbpfgo will always work (as libbpf is statically linked).

Regarding dynamic vs static loading that's a separate discussion IMO. I think I'd be ok if we decide to remove dynamic, but we can discuss in a new issue.

Why separate ? The breaking changes are likely related to the change of a, not yet, stable ABI.

@grantseltzer grantseltzer added this to the v0.3.0-libbpf-0.8.0 milestone Jun 20, 2022
@rafaeldtinoco rafaeldtinoco removed this from the v0.3.0-libbpf-0.8.0 milestone Jun 27, 2022
@grantseltzer grantseltzer modified the milestones: v0.4.0-libbpf-1.0.0, v0.4.1-libbpf-1.0.0 Aug 26, 2022
@rafaeldtinoco rafaeldtinoco removed this from the v0.4.1-libbpf-1.0.0 milestone Dec 12, 2022
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

No branches or pull requests

3 participants