Skip to content

cgen: fix port range bytecode generation#239

Merged
qdeslandes merged 4 commits into
facebook:mainfrom
qdeslandes:fix_port_range
Mar 21, 2025
Merged

cgen: fix port range bytecode generation#239
qdeslandes merged 4 commits into
facebook:mainfrom
qdeslandes:fix_port_range

Conversation

@qdeslandes

Copy link
Copy Markdown
Contributor

A port range matcher is composed of two ports: the lower bound and the higher bound. Because those are in the host's bytes order, bpfilter will use htobe16() on both port before comparing them to the packet's data. This is done to compare the packet's port against a specific value and works flawlessly.

However, the port range matcher doesn't work as expected due to the comparison being performed in network byte order! Hence, the most significant bytes are not located where the host expects them to, which is an issue for JLT/JGT operations.

Convert the packet's source/destination port to the host byte order before comparing the port range.

A port range matcher is composed of two ports: the lower bound and the
higher bound. Because those are in the host's bytes order, bpfilter will
use htobe16() on both port before comparing them to the packet's data.
This is done to compare the packet's port against a specific value and
works flawlessly.

However, the port range matcher doesn't work as expected due to the
comparison being performed in network byte order! Hence, the most
significant bytes are not located where the host expects them to, which
is an issue for JLT/JGT operations.

Convert the packet's source/destination port to the host byte order
before comparing the port range.
End-to-end tests are run for each hook, but bpfilter will generate the
same BPF bytecode for most of them. Instead, for each test, generate
(and test) a BPF program for each flavor.
@qdeslandes qdeslandes merged commit 97851f8 into facebook:main Mar 21, 2025
@qdeslandes qdeslandes deleted the fix_port_range branch March 21, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants