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

fix for arm devices #20

Merged
merged 2 commits into from
Jan 9, 2024
Merged

fix for arm devices #20

merged 2 commits into from
Jan 9, 2024

Conversation

NL-TCH
Copy link
Collaborator

@NL-TCH NL-TCH commented Jan 1, 2024

Hi y'all,

this is an easy fix for ARM devices as issued in #18
There is a current bug in docker cgroups, so a permanent fix is not possible from my side, there is however a workaround.
The workaround is to mount cgroups as host and make them writable. (--cgroupns host) (-v /sys/fs/cgroup:/sys/fs/cgroup:rw )
The readme is updated with an extra section, and no code changes are needed.

Gr. TCH

/claim 18

Copy link

algora-pbc bot commented Jan 1, 2024

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe/Alipay.

@billz
Copy link
Member

billz commented Jan 1, 2024

@NL-TCH thanks for the PR. tests pass on RPi OS (64-bit) Lite bookworm.
will check bullseye and 32-bit distros, plus gather any feedback from @jrcichra

@billz
Copy link
Member

billz commented Jan 1, 2024

Only oddity I found is connected clients have no internet. DHCP leases and DNS are good, connectivity check ok within the container but not for clients. @jrcichra perhaps you've observed this? Will give a closer look.

@NL-TCH
Copy link
Collaborator Author

NL-TCH commented Jan 1, 2024

Hmmm, I'll check this out in a few hours as well. Thanks anyways for checking and verifying!

@NL-TCH
Copy link
Collaborator Author

NL-TCH commented Jan 1, 2024

hmm it is probably masquerading or NAT rules,
from the container i can reach 1.1.1.1 (via eth0)
from wlan0 i can reach the IP of the eth0 interface, but not the default gateway of eth0

i am so bad at this networking stuff

@billz
Copy link
Member

billz commented Jan 1, 2024

@NL-TCH of course, I forgot about docker's network isolation 🙈 adding a few iptables rules did the trick.

iptables -I DOCKER-USER -i src_if -o dst_if -j ACCEPT
iptables -t nat -C POSTROUTING -o eth0 -j MASQUERADE || iptables -t nat -A POSTROUTING -o eth0 -j MASQUERADE
iptables -C FORWARD -i eth0 -o wlan0 -m state --state RELATED,ESTABLISHED -j ACCEPT || iptables -A FORWARD -i eth0 -o wlan0 -m state --state RELATED,ESTABLISHED -j ACCEPT
iptables -C FORWARD -i wlan0 -o eth0 -j ACCEPT || iptables -A FORWARD -i wlan0 -o eth0 -j ACCEPT
iptables-save

https://docs.docker.com/network/packet-filtering-firewalls/#docker-on-a-router

@NL-TCH
Copy link
Collaborator Author

NL-TCH commented Jan 1, 2024

ah cool!
do i need to add that to the readme, or do i need to add this to the install script?
what do you prefer?

@billz
Copy link
Member

billz commented Jan 1, 2024

I think the readme is fine, yes. Users may want to adjust these rules for their setup.
@jrcichra sound good? I don't recall this being necessary, but I've been wrong before 😉

@NL-TCH
Copy link
Collaborator Author

NL-TCH commented Jan 1, 2024

okay, i'll add them to the readme, but raspap would've never worked without these rules. so imo quite important to document :)

@jrcichra
Copy link
Collaborator

jrcichra commented Jan 2, 2024

I think the readme is fine, yes. Users may want to adjust these rules for their setup. @jrcichra sound good? I don't recall this being necessary, but I've been wrong before 😉

Yeah this is a necessity. In my testing today there's no internet traffic without those iptables rules.

@jrcichra
Copy link
Collaborator

jrcichra commented Jan 2, 2024

-cgroupns=host is indeed required, otherwise you get:

systemd 252.19-1~deb12u1 running in system mode (+PAM +AUDIT +SELINUX +APPARMOR +IMA +SMACK +SECCOMP +GCRYPT -GNUTLS +OPENSSL +ACL +BLKID +CURL +ELFUTILS +FIDO2 +IDN2 -IDN +IPTC +KMOD +LIBCRYPTSETUP +LIBFDISK +PCRE2 -PWQUALITY +P11KIT +QRENCODE +TPM2 +BZIP2 +LZ4 +XZ +ZLIB +ZSTD -BPF_FRAMEWORK -XKBCOMMON +UTMP +SYSVINIT default-hierarchy=unified)
Detected virtualization docker.
Detected architecture arm64.

Welcome to Debian GNU/Linux 12 (bookworm)!

Failed to create /init.scope control group: No such file or directory
Failed to allocate manager object: No such file or directory
[!!!!!!] Failed to allocate manager object.
Exiting PID 1...

@billz
Copy link
Member

billz commented Jan 6, 2024

Basic AP functionality tests pass on RPi OS (64-bit) Lite bullseye.

One issue I should have anticipated with WireGuard:

  1. The docker container uses Debian 10 buster which requires adding the backport repositories to apt's source lists (wireguard is a built-in module from the 5.6 Linux kernel onwards). During RaspAP's non-interactive install this results in Unable to locate package wireguard.

@NL-TCH curious if you also observed this. I used ghcr.io/raspap/raspap-docker which is built on @jrcichra's jrei/systemd-debian:10. I assume this should be updated to debian 11/12. Also possible I overlooked something.

Will repeat for 32-bit armhf bullseye + bookworm next. Looking very promising thus far.

@NL-TCH
Copy link
Collaborator Author

NL-TCH commented Jan 6, 2024

Hey @billz good one, i didn't catch that one.
honest question, why do you want to have multiple docker images with multiple debian versions?
what is the upside of using deb10 instead of deb12 ? i think you just should support only one version (newest)

just my humble opinion :)

@billz
Copy link
Member

billz commented Jan 6, 2024

what is the upside of using deb10 instead of deb12 ? i think you just should support only one version (newest)

@NL-TCH there is no real upside, IMO. I agree that we should just support the latest.

@billz billz mentioned this pull request Jan 6, 2024
@billz
Copy link
Member

billz commented Jan 6, 2024

If we agree to bump the Debian release version and ensure that container images are pushed to ghcr.io/raspap/raspap-docker I believe this PR should be selected for the bounty.

@jrcichra
Copy link
Collaborator

jrcichra commented Jan 7, 2024

what is the upside of using deb10 instead of deb12 ? i think you just should support only one version (newest)

@NL-TCH there is no real upside, IMO. I agree that we should just support the latest.

My only argument for multiple versions (11 and 12) would be if we find something where having the same Debian release on the host and the container is necessary (e.g some change to iptables or iproute2 that is incompatible).

But until we find there's a problem running a bookworm container on a bullseye host, we can keep it simple by supporting just the latest (bookworm).

README.md Outdated Show resolved Hide resolved
@jrcichra jrcichra self-requested a review January 7, 2024 02:40
README.md Outdated
## Workaround for arm devices
To use this container on arm devices you have to make cgroups writable:
```
docker run --name raspap -it -d --privileged --network=host --cgroupns host -v /sys/fs/cgroup:/sys/fs/cgroup:rw --cap-add SYS_ADMIN jrcichra/raspap-docker
Copy link
Member

Choose a reason for hiding this comment

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

as noted by @jrcichra the build pipeline is building an image on ghcr.io/raspap/raspap-docker, which would be preferable to his docker.io account. tested and confirmed this works

Copy link
Member

Choose a reason for hiding this comment

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

@NL-TCH can we replace jrcichra/raspap-docker with ghcr.io/raspap/raspap-docker?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@billz billz merged commit 84fd67a into RaspAP:master Jan 9, 2024
@NL-TCH NL-TCH mentioned this pull request Jan 9, 2024
@billz
Copy link
Member

billz commented Jan 12, 2024

Resolves #11

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.

3 participants