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

Support for other units than mbits #73

Open
interduo opened this issue Sep 9, 2022 · 13 comments
Open

Support for other units than mbits #73

interduo opened this issue Sep 9, 2022 · 13 comments
Milestone

Comments

@interduo
Copy link
Collaborator

interduo commented Sep 9, 2022

I just try to integrate with our CRM where is the delaration of kbits instead of mbits. I am thinking what to do now.
I did a little research how others are generating queue configs.
I think a good approach for that is to allow: K, M, G into Shaper rates.

tc supports:

              bit or a bare number Bits per second
              kbit   Kilobits per second
              mbit   Megabits per second
              gbit   Gigabits per second
              tbit   Terabits per second
              bps    Bytes per second
              kbps   Kilobytes per second
              mbps   Megabytes per second
              gbps   Gigabytes per second
              tbps   Terabytes per second

We should passthrouh the unit from ShapedDevices.csv to tc rules in linux-tc.txt also.

Currently it stores them as integers, so currently fractions would not be possible. Do you require that for your service plans? If so we can change to decimal if it's really needed.

Some few old customers just got some values - but this is not the point, because I could upper thirs limit in db one time.
I create ShapedDevices.csv using mbit or kbit unit declaration. With mbit there is a problem that You need to round kbit value and it gets You a decimal or 0 ;-)

The point is its better to have more clear configs to debug and definition in linux-tc.txt.

@rchac
Copy link
Member

rchac commented Sep 9, 2022

Part of the issue is that we need to really validate the input and convert to a common measure for the internal calculations (htb rates, ceil, etc). It would take a bit of work. But it can be done. i may not be able to do it right away but it's a good consideration for long term.

@interduo
Copy link
Collaborator Author

interduo commented Sep 9, 2022

I added validation to require minimum bandwidth for each entry to avoid the case you described. A device's min bandwidth must be 1Mbps or more. A device's max bandwidth must be 3Mbps or more. The 3Mbps is because there is a counter that decreases by 1 at some point in the code when setting rate and I just want to avoid issues there.

It's recommended by tc to set rates at multiplication of 8 only in using kbits.

Currently it stores them as integers, so currently fractions would not be possible. Do you require that for your service plans? If so we can change to decimal if it's really needed.

There are also some edge cases that requires queues at 256K or less to queue. I would not remove that feature.

Maybe there is another way to archieve that?

@rchac
Copy link
Member

rchac commented Sep 9, 2022

There are also some edge cases that requires queues at 256K or less to queue. I would not remove that feature.

This makes sense. I just assumed ISPs would no longer offer service below 3Mbps in either direction so that it wouldn't matter if we had a 3Mbps min - but perhaps there are situations there worth considering.

@interduo
Copy link
Collaborator Author

interduo commented Sep 9, 2022

I analised deeply the situation we got also some clients with a wireless service 10/2 and they don't want to change their agreement maybe from 5-7years ago. I know that we could bump up the speed but then the client will have less motivation to change tariff.

@rchac
Copy link
Member

rchac commented Sep 9, 2022

Ok in that case let's definitely see about at least lowering the threshold to 2Mbps min. That may hold us over until a bigger rework can be done.

@interduo
Copy link
Collaborator Author

interduo commented Sep 9, 2022

i may not be able to do it right away but it's a good consideration for long term.

So now:
As a workaroud I make a changes at my export script to:

  • generate mbit instead kbits,
  • setting the minium rates ceil to 2mbits,
    as a workaround

@rchac
Copy link
Member

rchac commented Sep 9, 2022

I need to also change something in LibreQoS.py to allow 2Mbps or more, one sec

@rchac
Copy link
Member

rchac commented Sep 9, 2022

Ok now your 2Mbps use case should work cffbfa8

@interduo
Copy link
Collaborator Author

interduo commented Sep 9, 2022

Executed 3993 XDP-CPUMAP-TC IP filter commands
Queue and IP filter reload completed in 8 seconds
refreshShapers completed on 09/09/2022 19:34:59

real	0m8.650s
user	0m3.776s
sys	0m5.172s

:)

@rchac rchac closed this as not planned Won't fix, can't repro, duplicate, stale Oct 20, 2022
@dtaht
Copy link
Collaborator

dtaht commented Oct 20, 2022

I like the ability to be clear about mbits in decimal or binary.

@dtaht dtaht reopened this Oct 20, 2022
@rchac
Copy link
Member

rchac commented Oct 20, 2022

Currently in ShapedDevices.csv it's Mbit in decimal form only. Would it be helpful to some operators to be able to provide it in binary?

@dtaht
Copy link
Collaborator

dtaht commented Oct 21, 2022

See! I think about it in binary, and so do many devices. I imagine operators think of it in decimal? It would explain a lot. It's not as bad a problem as newtons vs other measures but... binary....

@dtaht
Copy link
Collaborator

dtaht commented Nov 13, 2022

Seriously. Straightening out decimal vs binary would be useful.

@dtaht dtaht added this to the v1.4 milestone Nov 13, 2022
@dtaht dtaht modified the milestones: v1.4, v1.5 Jan 12, 2023
@rchac rchac modified the milestones: v1.5 Beta, v1.6 May 30, 2024
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