-
Notifications
You must be signed in to change notification settings - Fork 182
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
Fix mask computation for v6 IPs #205
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test for _create_mask()? Thanks!
@@ -273,7 +273,7 @@ def _create_mask(self, plen): | |||
if plen >= 8: | |||
mask.append(0xff) | |||
elif plen > 0: | |||
mask.append(2 ** plen - 1) | |||
mask.append(0xff>>(8-plen)<<(8-plen)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right! This should've been mask.append(0xff & (2 ** plen - 1))
. Your approach works too. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the top plen
bits to be 1
, so I think that's 0xff ^ (2 ** (8-plen) - 1)
?
This is just dropping the bottom 8-plen
bits though, so the double shift seemed a bit more obvious.
Added a couple of test cases for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Hey
It looks like IPv6 mask fields are not being computed correctly.
For example,
_create_mask(..., 10)
returns[0xff, 0x03, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
, when it should be returning[0xff, 0xc0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
Steps to reproduce:
fe80::/10
Patched version installs the rule we wanted