hudra0 / qosmate

GNU General Public License v3.0
41 stars 7 forks source link

Do not clear bulk markings when down-prioritizing #17

Open brada4 opened 1 week ago

brada4 commented 1 week ago

first 500ms wrongly upgrades already bulk tcp connections

simplistic filter would be

ip dscp lt cs4 ip dscp ne cs1

optimize that to range with single payload extract

since we are extracting payload anyway also avoid forcing unchanged value on packets.

brada4 commented 1 week ago

Constants used.

nft describe ip dscp
payload expression, datatype dscp (Differentiated Services Code Point) (basetype integer), 6 bits

pre-defined symbolic constants (in hexadecimal):
        cs0                             0x00
        cs1                             0x08
...
        cs4                             0x20
... 
hudra0 commented 5 days ago

You PR make sense, but:

While the hex range approach {0x01-0x07, 0x09-0x1f} should work correctly, I believe the initial PR using ip dscp < cs4 ip dscp != cs1 is more maintainable and self-documenting. Using symbolic names (cs4, cs1) clearly shows the intention of not upgrading bulk traffic, and makes future adjustments easier. Though the hex ranges might be marginally more efficient, the readability and maintainability benefits of the symbolic approach outweigh this small performance difference. Would you be open to reverting to the symbolic version?

brada4 commented 5 days ago

100% agree, go for it ;-)

brada4 commented 5 days ago

Edit: it is just 1st part needing ip dscp != cs1 added.

brada4 commented 5 days ago

@hudra0 ok , done as advised.

brada4 commented 5 days ago

Please squash PR as it has mess from my broken fork.