tohojo / sqm-scripts

SQM scripts traffic shaper
http://www.bufferbloat.net/projects/cerowrt/wiki/Smart_Queue_Management
235 stars 63 forks source link

Nftables support? (OpenWrt firewall is now nftables based) #146

Closed hnyman closed 2 years ago

hnyman commented 2 years ago

OpenWrt switched to nftables based firewall yesterday (in master), and the default OpenWrt build does not contain iptables any more.

https://github.com/openwrt/openwrt/commit/08d9f6e3020b4a149b2007b6ed7d684c49af9bbf

SQM still depends on iptables.

Is there hope about nftables support?

tohojo commented 2 years ago

Iptables is only used by one script IIRC, and I think it should work just fine with the iptables-nft compat binary...

hnyman commented 2 years ago

Could you then change the dependency for that iptables-nft?

https://github.com/openwrt/packages/blob/master/net/sqm-scripts/Makefile#L27-L28

(My concern is mainly that installing SQM will now also install iptables, which is not optimal in the long run.)

hnyman commented 2 years ago

Similarly, I am unsure about iptables-mod-conntrack-extra and iptables-mod-ipopt role/impact/dependencies in the nftables context.

tohojo commented 2 years ago

Hannu Nyman @.***> writes:

Similarly, I am unsure about iptables-mod-conntrack-extra and iptables-mod-ipopt role/impact/dependencies in the nftables context.

AFAICT the stuff we're using (DSCP and fwmark) is part of nft-core, so I don't think we need any extra kernel modules...

Created https://github.com/openwrt/packages/pull/17671

tiagogaspar8 commented 2 years ago

Hi everyone!

Can't the ipt commands be converted to nft ones? Is there a need for someone to do it?

tohojo commented 2 years ago

Tiago Gaspar @.***> writes:

Can't the ipt commands be converted to nft ones? Is there a need for someone to do it?

It could, but then we'd have to support both; which is a bit of a hassle when iptables-nft should be able to do the same thing?

An alternative would be do deprecate the simple.qos script entirely and tell people to use sch_cake instead, of course...

moeller0 commented 2 years ago

So to speak for simple.qos for a bit. This is the best starting point for people to start their own qos script from, as it exercises/shows a few relevant tricks one could employ. I have set a number of people with peculiar QoS demands on that route, so think it would be sad to loose this....

Also there are still situations in which cake is not performing as well as htb/tbf +fq_codel. And finally current sqm scripts are overly complex if the main goal is to just instantiate sch_cake ;)

tiagogaspar8 commented 2 years ago

It could, but then we'd have to support both; which is a bit of a hassle when iptables-nft should be able to do the same thing?

I agree, I haven't thought of that.

An alternative would be do deprecate the simple.qos script entirely and tell people to use sch_cake instead, of course...

That's beyond my understanding, actually, I'd like to but have no "deep" knowledge of what each script does and which is "better", I use cake with layer of cake but I've heard fq_codel is good too... If that can be dropped it is up to you as the maintainer, I trust your judgement 😁

Also there are still situations in which cake is not performing as well as htb/tbf +fq_codel.

Well, I believe @tohojo mentioned cake, but people could choose a different one if cake does not fit their interests

hnyman commented 2 years ago

I hope that OpenWrt devs will figure out a way to implement those symlinks so that iptables will lead to iptables-nft in a next few days. (I wrote to the main packages feed's firewall4 issue thread, and at least dangowrt took some interest.)

That would simplify the SQM changes or make them unnecessary.

Ps. The cake religion is funny. Cake is not the solve-all-problems solution for every situation. Especially with low-power routers with high-speed internet, cake is too CPU intensive.

(I like simple.qos myself, although have a rather powerful router.)

And finally current sqm scripts are overly complex if the main goal is to just instantiate sch_cake ;)

Yeah, and now the new qosify package offers an alternative cake launcher with DSCP classifying support.

tohojo commented 2 years ago

Hannu Nyman @.***> writes:

I hope that OpenWrt devs will figure out a way to implement those symlinks so that iptables will lead to iptables-nft in a next few days. (I wrote to the main packages feed's firewall4 issue thread, and at least dangowrt took some interest.)

That would simplify the SQM changes or make them unnecessary.

Right, awesome, thanks! If that doesn't work out we can also add iptables-nft as one of the candidates that sqm-scripts looks for to find an iptables binary; but I really do think it makes more sense for the iptables-nft package to just install a symlink.

Ps. The cake religion is funny. Cake is not the solve-all-problems solution for every situation. Especially with low-power routers with high-speed internet, cake is too CPU intensive.

Well, Cake did start out as a way of writing a qdisc that includes all of sqm-script's features, so it's only natural to periodically check if it succeeded :)

(I like simple.qos myself, although have a rather powerful router.)

And finally current sqm scripts are overly complex if the main goal is to just instantiate sch_cake ;)

Yeah, and now the new qosify package offers an alternative cake launcher with DSCP classifying support.

I always considered all the diffserv-based classification to be mostly placebo, so I haven't really been paying much attention to that (and usually run simplest.qos or piece_of_cake.qos on the machines that do need sqm-scripts). But great that there's an alternative solution catering specifically to the QoS optimisation use case :)

moeller0 commented 2 years ago

Well, as long as people only sparingly and judiciously use higher priority marking for a few select and latency sensitive flows, it works pretty well. I keep trying to remind people that prioritization is a zero-sum-game, for every packet that gets preferential treatment some other packet(s) will get worse treatment; I have the feeling people generally are not listening/caring. This is evident when people argue about (theoretical) properties of different DSCPs as if these would mean anything unless the local network implements the implied PHB...

That said qosify offers a way to use some simple rules to change DSCPs even fo ingress traffic that IMHO allow anything from the careful and sensible to the over-the-top and garish, and that is IMHO a good thing, a toll should come with reasonable defaults but not arbitrarily disallow configurations that the tools author did not notice or care for.

yutayu commented 2 years ago

difference is simple.qos cannot fix ingress classfying. layer_cake.qos does it.

moeller0 commented 2 years ago

Hmm, as far as I can tell simple.qos will respond to different select DSCPs by moving them into its own three priority tiers. But I have not tested that recently.

yutayu commented 2 years ago

It is enough for me :) sqm-script offers not only typical but also specific traffic shaping.

tiagogaspar8 commented 2 years ago

Hi everyone,

So, about this recursive dependency issue... I tried to fix it, yet I give up, the entire iptables package is built around selecting iptables kmods, (not to mention the notion that people always want ipv4 and only want to disable ipv6) when the purpose of iptables-nft is to replace those and use nft... So I'll just remove the iptables dependency from my fork and go on with it. My recommendation? Iptables is going to be dropped soon, so I'd recommend you to do it just do as I did with jool and just move to nftables fully, it's time to stop working with legacy tools and get some stability on the new ones, either way I always appreciate the effort!😁

PS:tc is another tool that for some reason depends on the tchiptableshmod which in turn depends on iptables, why? 🤦

wulfy23 commented 2 years ago

longer term an internal function can 'prefer' iptables-nft if present and supported with each qos file (for the 1? script that needs it)...

unlike alot of other packages sqm has the hooks/smarts to deal with most of this stuff internally...

my hackorrific qos file (iptables based) works perfectly fine in parallel with nft, ymmv... so remove the iptables dep and prerr in .qos that need it is all that's needed here...

tiagogaspar8 commented 2 years ago

When I say drop iptables support I mean convert everything in the scripts from iptables commands to nftables ones. Or even another alternative, have two sqm-scrips one -ipt that has iptables commands and depends on iptables and another -nft that depends on nftables and has the nftables scripts, just an example

moeller0 commented 2 years ago

Mmmh, how about we drop the dependency to iptables/nftables and give stern warning if we can not find either iptables/iptables-nft at run time with hints how to install them?

hnyman commented 2 years ago

tc is another tool that for some reason depends on the tc-iptables-mod which in turn depends on iptables, why?

That discusison is more to the developer mailing list than here. It is pretty obvious that the whole iptables/nftables/tc/iproute2 universe has been built around the priciples that you mention: "people always want ipv4 and only want to disable ipv6".
Depending on user-space iptables has been a simple way to guarantee that the underlying kernel modules have been included. Now we need to configure us out from that. But the main dependency challenge is in the core OpenWrt, not here in the packages.

tohojo commented 2 years ago

Right, opened https://github.com/tohojo/sqm-scripts/pull/147 - unless someone complains loudly, my plan is to push that to the openwrt package and drop the iptables dependency entirely... :)

dtaht commented 2 years ago

SQM runs on regular linux also, which has not made it to nftables.

I'm so glad to hear nftables is in there now... Is there any hope for libreQos? (more specifically XDP?)

SpareSimian commented 2 years ago

What is "regular" Linux? nftables has been in the kernel since 3.13 in 2014. Red Hat switched their firewall from iptables to nftables with RHEL 8. RHEL 7 still uses iptables underneath firewalld but I'm pretty sure nftables is available.

dtaht commented 2 years ago

There are 260+ known distros in that iptables long tail.

I note I have zero problem with the idea of essentially freezing sqm + iptables where it is today and forking sqm going forward. However, it wasn't just the move to nftables I wanted, it was to be able to shift to xdp for inbound shaping, which is the single biggest cpu hog we have.

tiagogaspar8 commented 2 years ago

Mmmh, how about we drop the dependency to iptables/nftables and give stern warning if we can not find either iptables/iptables-nft at run time with hints how to install them?

That sounds more like a hack than a sound solution, the best way would be to provide a package with only the base scripts, and then, optionally provide the nftables or the iptables scrips:

It's as easy as adding a separate install section to copy either one file or another.

moeller0 commented 2 years ago

That sounds awfully complex to deal with issues in OpenWrt itself (that hopefully get sorted out). Especially since I assume simple.qos to not care whether old-school plain iptables/ip6tables is used or iptables-nft/ip6tables-nft is used. I am with @dtaht (at least I assume so) that assuming a quick and complete change over from ip- to nf-tables to be too optimistic, and having to maintain multiple versions of say simple.qos one for iptables one for nftables for the forseeable future sounds like a sure way for these to diverge.

tiagogaspar8 commented 2 years ago

I understand what you mean and fully support your decision and position, iptables-nft MUST work and let's hope we can get this fixed, I was just offering an alternative not depending on devs wanting to fix iptables, which for now they seem not open to/able to 😄

tohojo commented 2 years ago

Tiago Gaspar @.***> writes:

I understand what you mean and fully support your decision and position, iptables-nft MUST work and let's hope we can get this fixed, I was just offering an alternative not depending on devs wanting to fix iptables, which for now they seem not open to/able to 😄

One thing we could do purely on the packaging side, though, is to just package two (identical) versions of sqm-scripts, one that depends on iptables and one that depends on iptables-nft; that way we'd be sure there was always one of the two installed along with sqm-scripts, and the user could select the package variant that suited their needs...

Borromini commented 2 years ago

Is there any progress on this?

tohojo commented 2 years ago

@Borromini See https://github.com/openwrt/packages/pull/17963 - you should just be able to install iptables-nft and get the compatibility binary...

Borromini commented 2 years ago

Sorry, I googled and saw this issue was still open, hence why I asked. Thanks!

tohojo commented 2 years ago

Right, I guess we can close this for now at least :)

kokroo commented 2 years ago

@Borromini See openwrt/packages#17963 - you should just be able to install iptables-nft and get the compatibility binary...

Does this affect performance/compatibility in any way?