Skip to content

More arithmetic operations (and tests) for PrefixSize #687

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

Merged
merged 6 commits into from
Jul 11, 2025

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Jul 9, 2025

Improve implementations for Add and Sub, add implementations for Mul, Div, and the -Assign variants for PrefixSize. This should help with future work with prefix size manipulation.

First commit is a fix.

@qmonnet qmonnet added this to the GW R1 milestone Jul 9, 2025
@qmonnet qmonnet requested a review from mvachhar July 9, 2025 15:10
@qmonnet qmonnet self-assigned this Jul 9, 2025
@qmonnet qmonnet requested a review from a team as a code owner July 9, 2025 15:10
@qmonnet qmonnet added the area/nat Related to Network Address Translation (NAT) label Jul 9, 2025
@qmonnet qmonnet force-pushed the pr/qmonnet/prefixsize-ops branch from 3c58e68 to 375355d Compare July 9, 2025 15:26
qmonnet added 2 commits July 9, 2025 16:39
We based our implementation on the addition of a PrefixSize and a u128,
making all other cases return a PrefixSize::Overflow, but the addition
between two PrefixSize objects needs to take another corner case into
account: for PrefixSize::U128(0) + PrefixSize::Ipv6MaxAddrs, the result
is not an overflow but PrefixSize::Ipv6MaxAddrs.

Found while working on fuzzing tests for another component.

Signed-off-by: Quentin Monnet <[email protected]>
@qmonnet qmonnet force-pushed the pr/qmonnet/prefixsize-ops branch from 375355d to 3f0ad5d Compare July 9, 2025 15:39
qmonnet added 3 commits July 9, 2025 16:54
Implement trait SubAssign for PrefixSize, given that we already suppot
the Sub trait.

Speaking of, we also adjust our implementation of the Sub trait for
PrefixSize - PrefixSize, so that subtracting Overflow from something
triggers an overflow panic (instead of returning Overflow). Also make
sure that we panic and never wrap in case of subtaction overflow (it
doesn't make sense to wrap around in case of PrefixSize objects).

Signed-off-by: Quentin Monnet <[email protected]>
We have Add, Sub, and their -Assign variants. Let's support
multiplication for prefix sizes. This can be handy when growing or
splitting prefixes and comparing their sizes.

Signed-off-by: Quentin Monnet <[email protected]>
This is useful when manipulating prefixes, to easily obtain the size of
a half-prefix, for example.

Signed-off-by: Quentin Monnet <[email protected]>
@qmonnet qmonnet force-pushed the pr/qmonnet/prefixsize-ops branch from 3f0ad5d to 765c46f Compare July 9, 2025 15:54
@qmonnet qmonnet force-pushed the pr/qmonnet/prefixsize-ops branch from 765c46f to 4ac08d7 Compare July 10, 2025 11:12
@qmonnet qmonnet requested a review from daniel-noland July 10, 2025 19:14
@mvachhar mvachhar added this pull request to the merge queue Jul 11, 2025
Merged via the queue into main with commit b15a1ce Jul 11, 2025
16 checks passed
@mvachhar mvachhar deleted the pr/qmonnet/prefixsize-ops branch July 11, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/nat Related to Network Address Translation (NAT)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants