Skip to content
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

Add new initialization arguments for mask class to support set mask with less code #206

Merged
merged 5 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 56 additions & 32 deletions src/ptf/mask.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ class MaskException(Exception):


class Mask:
def __init__(self, exp_pkt, ignore_extra_bytes=False):
def __init__(self, exp_pkt, ignore_extra_bytes=False, dont_care_all=False):
self.exp_pkt = exp_pkt
self.size = len(exp_pkt)
self.valid = True
self.mask = [0xFF] * self.size
self.mask = [0] * self.size if dont_care_all else [0xFF] * self.size
self.ignore_extra_bytes = ignore_extra_bytes

def set_do_not_care(self, offset, bitwidth):
Expand All @@ -28,7 +28,60 @@ def set_do_not_care(self, offset, bitwidth):
offsetb = idx % 8
self.mask[offsetB] = self.mask[offsetB] & (~(1 << (7 - offsetb)))

def set_do_not_care_all(self):
self.mask = [0] * self.size

def set_do_not_care_packet(self, hdr_type, field_name):
offset, bitwidth = self._calculate_fields_offset_and_bitwidth(
hdr_type, field_name
)
self.set_do_not_care(offset, bitwidth)

def set_do_not_care_scapy(self, hdr_type, field_name):
warnings.warn(
'"set_do_not_care_scapy" is going to be deprecated, please '
'switch to the new one: "set_do_not_care_packet"',
DeprecationWarning,
)
self.set_do_not_care_packet(hdr_type, field_name)

def set_care(self, offset, bitwidth):
for idx in range(offset, offset + bitwidth):
offsetB = idx // 8
offsetb = idx % 8
self.mask[offsetB] = self.mask[offsetB] | (1 << (7 - offsetb))

def set_care_all(self):
self.mask = [0xFF] * self.size

def set_care_packet(self, hdr_type, field_name):
offset, bitwidth = self._calculate_fields_offset_and_bitwidth(
hdr_type, field_name
)
self.set_care(offset, bitwidth)

def set_ignore_extra_bytes(self):
self.ignore_extra_bytes = True

def is_valid(self):
return self.valid

def pkt_match(self, pkt):
# just to be on the safe side
pkt = bytearray(bytes(pkt))
# we fail if we don't match on sizes, or if ignore_extra_bytes is set,
# fail if we have not received at least size bytes
if (not self.ignore_extra_bytes and len(pkt) != self.size) or len(
pkt
) < self.size:
return False
exp_pkt = bytearray(bytes(self.exp_pkt))
for i in range(self.size):
if (exp_pkt[i] & self.mask[i]) != (pkt[i] & self.mask[i]):
return False
return True

def _calculate_fields_offset_and_bitwidth(self, hdr_type, field_name):
if hdr_type not in self.exp_pkt:
self.valid = False
raise MaskException("Unknown header type")
Expand Down Expand Up @@ -63,36 +116,7 @@ def set_do_not_care_packet(self, hdr_type, field_name):
break
else:
offset += bits
self.set_do_not_care(hdr_offset * 8 + offset, bitwidth)

def set_do_not_care_scapy(self, hdr_type, field_name):
warnings.warn(
'"set_do_not_care_scapy" is going to be deprecated, please '
'switch to the new one: "set_do_not_care_packet"',
DeprecationWarning,
)
self.set_do_not_care_packet(hdr_type, field_name)

def set_ignore_extra_bytes(self):
self.ignore_extra_bytes = True

def is_valid(self):
return self.valid

def pkt_match(self, pkt):
# just to be on the safe side
pkt = bytearray(bytes(pkt))
# we fail if we don't match on sizes, or if ignore_extra_bytes is set,
# fail if we have not received at least size bytes
if (not self.ignore_extra_bytes and len(pkt) != self.size) or len(
pkt
) < self.size:
return False
exp_pkt = bytearray(bytes(self.exp_pkt))
for i in range(self.size):
if (exp_pkt[i] & self.mask[i]) != (pkt[i] & self.mask[i]):
return False
return True
return hdr_offset * 8 + offset, bitwidth
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the set_do_not_care call here? Shouldn't this be handled by the mask?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't remove the set_do_not_care call, this diff displaying is quite confused and incorrect. I will try to adjust the order of those functions to get a better diff display.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried, but reorder doesn't works well..

To help reviewer, I can explain what have I changed.

  1. I added a new arugument dont_care_all for init
  2. I added function called set_care,set_care_packet and set_care_all versus set_do_not_care,set_do_not_care_scapy.

That's all, the diff showing here is quite confused, for example, if you review the function pkt_match, you can see, I didn't change any single character of it, but the diff tell us I have removed the function and wrote a new one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying, the diff makes it difficult to understand what has been changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the confusion comes from adding _calculate_fields_offset_and_bitwidth. If you move that function above set_do_not_care_packet the diff should look better.

However, set_do_not_care_packet now calls into _calculate_fields_offset_and_bitwidth which does not use self.set_do_not_care(hdr_offset * 8 + offset, bitwidth)


def __str__(self):
old_stdout = sys.stdout
Expand Down
33 changes: 33 additions & 0 deletions utests/tests/ptf/test_mask.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,46 @@ def test_mask__mask_simple_packet(self, scapy_simple_tcp_packet):
mask_packet.set_do_not_care_packet(TCP, "chksum")
assert mask_packet.pkt_match(modified_packet)

def test_mask__set_do_not_care_all(self):
expected_packet = "\x01\x02\x03\x04\x05\x06"
packet = "\x08\x07\x06\x05\x04\x03\x02\x01"
mask = Mask(expected_packet.encode(), ignore_extra_bytes=True)
mask.set_do_not_care_all()
assert mask.pkt_match(packet.encode())

def test_mask__set_do_not_care(self):
expected_packet = "\x01\x02\x03\x04\x05\x06"
packet = "\x01\x00\x00\x04\x05\x06\x07\x08"
mask = Mask(expected_packet.encode(), ignore_extra_bytes=True)
mask.set_do_not_care(8, 16)
assert mask.pkt_match(packet.encode())

def test_mask__set_care_all(self):
expected_packet = "\x01\x02\x03\x04\x05\x06"
packet = "\x00\x02\x03\x04\x05\x06"
mask = Mask(expected_packet.encode(), ignore_extra_bytes=True)
mask.set_care_all()
assert not mask.pkt_match(packet.encode())

def test_mask__set_care(self):
expected_packet = "\x01\x02\x03\x04\x05\x06"
packet = "\x01\x02\x00\x04\x05\x06\x07\x08"
mask = Mask(
expected_packet.encode(), ignore_extra_bytes=True, dont_care_all=True
)
mask.set_care(0, 16)
assert mask.pkt_match(packet.encode())
mask.set_care(16, 8)
assert not mask.pkt_match(packet.encode())

def test_mask__set_care_packet(self):
packet = IP(src="1.1.1.1")
mask = Mask(packet.copy(), ignore_extra_bytes=True, dont_care_all=True)
packet[IP].src = "2.2.2.2"
assert mask.pkt_match(packet)
mask.set_care_packet(IP, "src")
assert not mask.pkt_match(packet)

def test_mask__check_masking_conditional_field(self, scapy_simple_vxlan_packet):
simple_vxlan = scapy_simple_vxlan_packet
simple_vxlan[VXLAN].flags = "G"
Expand Down