aarch64 support - Githubissues

atenart commented 7 months ago

Native compilation on aarch64 works and only performed a few tests. Before being ready, IMO we should investigate:

[x] More testing.
[x] Cross-compilation.
[x] Build in CI.
[x] vmlinux.h version unification and an utility to easily generate it.

Feel free to contribute to this branch or reach out.

vlrpl commented 7 months ago

Very happy to see this. I agree we should unify vmlinux.h generation as well I guess this is in line with all the bullets in #373 and a good step forward to complete it.

atenart commented 6 months ago

Rebased on main (and v1.4), no other change.

vlrpl commented 5 months ago

I guess having the same kernel version for both architectures is a bit annoying, isn't it?

atenart commented 5 months ago

I guess having the same kernel version for both architectures is a bit annoying, isn't it?

I agree it makes generating it a bit more complex. But then IMO it makes sense: the same BPF code is used and using a vmlinux.h file coming from a known (and the same) kernel makes things more consistent and probably avoid seeing some arch-specific bugs.

vlrpl commented 5 months ago

I guess having the same kernel version for both architectures is a bit annoying, isn't it?

I agree it makes generating it a bit more complex. But then IMO it makes sense: the same BPF code is used and using a vmlinux.h file coming from a known (and the same) kernel makes things more consistent and probably avoid seeing some arch-specific bugs.

yep, that was what I was thinking about.

vlrpl commented 5 months ago

@atenart, the PR includes some patches that can go ahead independently:

tree: nft: special case the old definitions
tools: add a script to generate vmlinux.h (can stay here as well)
x86_64/vmlinux.h: udpate to 6.9+
ci: bump Fedora image to 40

They look ok (I didn't completely review the script). If this PR require some more time, my suggestion is to split the PR and merge the ones above. WDYT?

atenart commented 5 months ago

@atenart, the PR includes some patches that can go ahead independently:
* tree: nft: special case the old definitions
* tools: add a script to generate vmlinux.h (can stay here as well)
* x86_64/vmlinux.h: udpate to 6.9+
* ci: bump Fedora image to 40 
They look ok (I didn't completely review the script). If this PR require some more time, my suggestion is to split the PR and merge the ones above. WDYT?

That's a good point and supporting runtime CI for aarch64 is quite something...

I was considering asking instead to merge this w/o runtime CI support as the aarch64 modifications are not invasive and we don't actually package an aarch64 version. This would allow to self build an image for aarch64. We could even mark it as "currently unsupported" in the doc (I personally don't think that's necessary but have nothing against it).

Of course, if you think we should keep exploring runtime CI tests as part of the aarch64 initial support I can definitively take the patches you mention in another PR.

vlrpl commented 5 months ago

@atenart, the PR includes some patches that can go ahead independently:
* tree: nft: special case the old definitions
* tools: add a script to generate vmlinux.h (can stay here as well)
* x86_64/vmlinux.h: udpate to 6.9+
* ci: bump Fedora image to 40 
They look ok (I didn't completely review the script). If this PR require some more time, my suggestion is to split the PR and merge the ones above. WDYT?
That's a good point and supporting runtime CI for aarch64 is quite something...

I was considering asking instead to merge this w/o runtime CI support as the aarch64 modifications are not invasive and we don't actually package an aarch64 version. This would allow to self build an image for aarch64. We could even mark it as "currently unsupported" in the doc (I personally don't think that's necessary but have nothing against it).

I'm ok with your proposal. We also did it in the past (for a PR required to support arm64 :) Agreed, we don't really need to call out we don't support it.

Of course, if you think we should keep exploring runtime CI tests as part of the aarch64 initial support I can definitively take the patches you mention in another PR.

it would be nice to have aarch64 CI, but let's just keep it in the background. No reason to block this.

atenart commented 5 months ago

I was considering asking instead to merge this w/o runtime CI support as the aarch64 modifications are not invasive and we don't actually package an aarch64 version. This would allow to self build an image for aarch64. We could even mark it as "currently unsupported" in the doc (I personally don't think that's necessary but have nothing against it).

I'm ok with your proposal. We also did it in the past (for a PR required to support arm64 :) Agreed, we don't really need to call out we don't support it.

Of course, if you think we should keep exploring runtime CI tests as part of the aarch64 initial support I can definitively take the patches you mention in another PR.

it would be nice to have aarch64 CI, but let's just keep it in the background. No reason to block this.

All right, thanks! I'll keeping trying a bit to see how far I can go with aarch64 runtime CI and I'll then update this PR next week w/o it. (Or with it if I'm really lucky :)).

atenart commented 5 months ago

it would be nice to have aarch64 CI, but let's just keep it in the background. No reason to block this.

All right, thanks! I'll keeping trying a bit to see how far I can go with aarch64 runtime CI and I'll then update this PR next week w/o it. (Or with it if I'm really lucky :)).

Could get it to work, I'll open another PR with aarch64 runtime CI to continue the investigations once this PR is merged.

atenart commented 4 months ago

Added cross-compilation support.

atenart commented 4 months ago

Runtime CI failed because no Vagrant libvirtd box can be found under https://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/Cloud/x86_64/images/ (image was not replace as part of the latest build[1]). Not sure if that is an intentional change or not. In any way, runtime CI was successful on its last run before I added the cross-compilation commits, which only change the build logic.

[1] https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/EDY6REA2MYMDQORL33C25HWTSSXLB75K/

atenart commented 4 months ago

Runtime CI failed because no Vagrant libvirtd box can be found under https://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/Cloud/x86_64/images/ (image was not replace as part of the latest build[1]). Not sure if that is an intentional change or not. In any way, runtime CI was successful on its last run before I added the cross-compilation commits, which only change the build logic.

[1] https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/EDY6REA2MYMDQORL33C25HWTSSXLB75K/

Fixed now. Only c8s test is failing but that is unrelated and known.

retis-org / retis

aarch64 support #379