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.

Sign up for GitHub

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

Jump to bottom

Conditioning not taking effect #13

Open
dbrant opened this issue Nov 11, 2017 · 6 comments
Open

Conditioning not taking effect #13

dbrant opened this issue Nov 11, 2017 · 6 comments

Comments

@dbrant
Copy link

dbrant commented Nov 11, 2017

I have a Raspberry Pi 3 (built-in wifi), and I've gotten as far as configuring it as an access point, running the PiNC service, and looking at the web interface.

However, when I actually enable throttling for a connected device, it doesn't seem to have any effect. The connection quality from the target device appears the same. In the PiNC web interface, the throttling icon is shown next to the device, but when I refresh the page, the icon goes away and the device is no longer shown as throttled.
@phuedx
Copy link
Owner

phuedx commented Dec 10, 2017 •
edited
Loading

OK. I spent an hour setting up a Debian Stretch VM in the same way I'd set up a Raspberry Pi. Here are my notes:

Symptoms

  1. The setup script isn't creating the traffic control queueing disciplines.
  2. Conditioning traffic for a device isn't persisting on the web UI.

Cause

In a31e0a8, I replaced testing Boolean(process.env.NO_EXEC) with process.env.NODE_ENV !== 'prod') in the ./src/server/exec module. Previously, if the NO_EXEC environment variable //wasn't// set, then PiNC would execute all commands. Now, if the NODE_ENV variable isn't set then PiNC won't execute all commands. Since the setup script wasn't updated to reflect this change, it's been broken (a NOP) since that change.

Now, if there aren't any traffic control queueing disciplines, then conditioning traffic for a device via the web UI should fail. Either:

  • The service isn't failing; or
  • The service is failing but silently; or
  • The service is failing loudly but the web UI isn't reflecting that failure.

Related Issues

  1. It's impossible to debug the service (part of which backs the setup script).
  2. The setup script fails to create the traffic control queueing disciplines for the "3G (UK)" profile.

@phuedx
Copy link
Owner

phuedx commented Dec 10, 2017 •
edited
Loading

  • The service is failing loudly but the web UI isn't reflecting that failure.

I've just tested this locally and it can't happen. If the server returns a 4xx or 5xx error code, then the web UI doesn't update.

@phuedx
Copy link
Owner

phuedx commented Dec 10, 2017

  • The service isn't failing; or

I've just tested this locally and it isn't the case.

phuedx added a commit that referenced this issue Dec 11, 2017
@phuedx
fix(scripts): Actually create queueing disciplines
c82bd08 
a31e0a8 introduced a serious regression, wherein the setup script
wouldn't create create the queueing disciplines that correspond to the
network throttling profiles.

See #13 for context.
phuedx added a commit that referenced this issue Dec 11, 2017
@phuedx
fix(profiles): Chain promises correctly
a5b685a 
... in DeviceService#setDeviceProfile and ProfileService#setProfile so
that:

- Errors can be handled at the application boundary; and
- The /devices/:mac end point responds when the asynchronous work
  started by ProfileService#setProfile settles.

In the case of ProfileService#setProfile, there wasn't actually any
chaining, which could lead to an UnhandledPromiseRejectionWarning being
raised when shelling out to tc fails.

See #13 for context.
@phuedx
Copy link
Owner

phuedx commented Dec 12, 2017

  1. The setup script fails to create the traffic control queueing disciplines for the "3G (UK)" profile.

This is because handles aren't being converted to hex in src/server/profiles.js.

phuedx added a commit that referenced this issue Dec 12, 2017
@phuedx
fix(profiles): Convert handles to hex
5ef8b1d 
tc requires handles to be hexadecimal rather than decimal. This
requirement isn't documented anywhere, however...

Not converting handles to hexadecimal causes the setup script to fail
once there are seven or more profiles: the root prio qdisc has the
correct number of classes (10) but the seventh child qdisc refers to the
16th (0x10), which doesn't exist.

See #13 for context.
@phuedx
Copy link
Owner

phuedx commented Dec 12, 2017 •
edited
Loading

@dbrant: I'd appreciate if you could retry this with PiNC@5ef8b1d.

@dbrant
Copy link
Author

dbrant commented Dec 16, 2017

Retrying from that commit, it looks like the throttle setting is being correctly persisted in the web UI. However, it still doesn't seem like any client connections are actually being throttled. The only error message I can see is the following from the setup script:
RTNETLINK answers: No such file or directory
(I'm not sure where else to look for additional debugging.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@phuedx @dbrant