Skip to content

Optimizer does not completely remove leading noop statements #1266

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

Closed
fenner opened this issue Feb 2, 2024 · 1 comment
Closed

Optimizer does not completely remove leading noop statements #1266

fenner opened this issue Feb 2, 2024 · 1 comment
Labels
BPF related optimizer Issues with the filter compiler's optimizer

Comments

@fenner
Copy link
Collaborator

fenner commented Feb 2, 2024

When creating a simple filter for ICMP type 42, starting at the IPv4 layer:

filtertest IPv4 'icmp[icmptype] = 42'

we end up with a dangling instruction at the start:

(000) ld       #0x0
(001) ldb      [9]
(002) jeq      #0x1             jt 3    jf 9
(003) ldh      [6]
(004) jset     #0x1fff          jt 9    jf 5
...

This loads a constant 0, and then obviously overwrites it with the IP protocol number ([9]).

The pre-optimized code, from filtertest -O IPv4 'icmp[icmptype] = 42', makes it a little more clear what could be happening:

(000) ld       #0x0
(001) jeq      #0x0             jt 2    jf 23
(002) ld       #0x0
(003) jeq      #0x0             jt 4    jf 23
(004) ldb      [9]

My guess is it's trying to generate code to look at a link type, and since there is no link type, it's just generating "load 0, if 0, load 0, if 0". Unfortunately the optimizer fails to completely optimize this away.

I've tried this with 5bfcce9 since I know there was some recent optimizer work.

Unsurprisingly, the same happens with DLT_IPv6 - filtertest IPv6 'icmp6[icmptype] = 160'

@fenner fenner added the optimizer Issues with the filter compiler's optimizer label Feb 2, 2024
@infrastation
Copy link
Member

The origin of the first ld, which is a no-op, is from the icmp keyword, which implies IPv4. The latter means that the generator for DLTs that support more than one address family produces a check for the protocol family:

$ filtertest -O RAW 'icmp'
(000) ldb      [0]                                  ; IPv4
(001) and      #0xf0                                ; IPv4
(002) jeq      #0x40            jt 3	jf 6        ; IPV4
(003) ldb      [9]
(004) jeq      #0x1             jt 5	jf 6
(005) ret      #262144
(006) ret      #0

For DLTs that are IPv4-only the generator does not omit the check, but replaces it with the result of gen_true():

$ filtertest -O IPV4 'icmp'
(000) ld       #0x0
(001) jeq      #0x0             jt 2	jf 5        ; TRUE
(002) ldb      [9]
(003) jeq      #0x1             jt 4	jf 5
(004) ret      #262144
(005) ret      #0

Then, as you note, the optimizer removes the always-true jeq, which makes the ld a no-op, but does not remove the ld:

$ filtertest IPV4 'icmp'
(000) ld       #0x0                                 ; no-op
(001) ldb      [9]
(002) jeq      #0x1             jt 3	jf 4
(003) ret      #262144
(004) ret      #0

This is a specific case of #1022, so I am closing this as a duplicate and adding this case into the latter issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BPF related optimizer Issues with the filter compiler's optimizer
Development

No branches or pull requests

2 participants