Skip to content

Conversation

@brada4
Copy link

@brada4 brada4 commented Nov 27, 2024

Do not emit unnecessary l4proto filter for helpers. No bytecode or readback changed.
There is something better waiting on top of this cleanup

Signed-off-by: Andris PE neandris@gmail.com

Do not emit unncessary l4proto filter for helpers. No bytecode or
readback changed.
There is something better waiting on top of this cleanup

Signed-off-by:
@brada4
Copy link
Author

brada4 commented Nov 27, 2024

@jow- please treat this quicker than usual

nft -c -d netlink -f - << EOF
table inet testing {
 chain old {
  meta l4proto tcp tcp dport 45
 }
 chain intent {
  meta l4proto tcp meta l4proto tcp th dport 45
 }
 chain new {
  tcp dport 45
 }
}
EOF

Evil intent of original is luckily squashed by nft cli.

@brada4
Copy link
Author

brada4 commented Nov 27, 2024

btw 3x if rule.helper sections are never accessed.

@brada4
Copy link
Author

brada4 commented Mar 16, 2025

Feels like beating a dead horse, i want to join tcpudp in single rules, and when i try this extra decoration starts to get ugly as in middle line.

@jow-
Copy link
Contributor

jow- commented Mar 17, 2025

I do not understand the intent of this PR.

@brada4
Copy link
Author

brada4 commented Mar 17, 2025

If i want to emit rule l4proto {tcp udp} th dport 53 this ads 2x l4proto spec.

@brada4
Copy link
Author

brada4 commented Mar 24, 2025

My intent is simple -
dns intercept rule in its primitive form emits omly one firewall rule.

so i just replaced udp dport 53 with meta l4proto udp th dport 53
did not even get to add tcpudp case to rule emitting code as i found the surplus l4proto here in helper dept.

Does not impact my direction, just that there is some easy cleanup here in main code.

@brada4 brada4 changed the title Do not emit extra l4proto filter for helpers ruleset: do not emit extra l4proto filter for helpers May 19, 2025
@brada4
Copy link
Author

brada4 commented Nov 28, 2025

Happy anniversary old PR.

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

Successfully merging this pull request may close these issues.

2 participants