From 9db46b8363b80639022f118f6b2a67151f083c8b Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 21 Mar 2025 12:21:35 +0100 Subject: [PATCH 01/14] Add 'test/perf_packet_fields.uts' test (#4705) --- test/perf_packet_fields.uts | 128 ++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 test/perf_packet_fields.uts diff --git a/test/perf_packet_fields.uts b/test/perf_packet_fields.uts new file mode 100644 index 00000000000..fc73f60d465 --- /dev/null +++ b/test/perf_packet_fields.uts @@ -0,0 +1,128 @@ +% Packet field performance issue + +############ +############ ++ Test utils + += Prepare a `check_exec_time()` function. + +def check_exec_time(description, t0, tf, max): + print(f"{description}: {tf - t0:.3f} seconds") + assert tf - t0 <= max, f"{description!r} FAILED: {tf - t0:.3f} > {max:.3f}" + + +############ +############ ++ Test data + += Define the `C` final packet class. + +class C(Packet): + fields_desc = [ + ByteField(name="val", default=0), + ] + def extract_padding(self, s): + return (b'', s) + += Define the `B` intermediate packet class. + +class B(Packet): + NUMBER_OF_C_PER_B = 100 + fields_desc = [ + PacketField(name=f"c{i}", pkt_cls=C, default=C(val=i)) + for i in range(NUMBER_OF_C_PER_B) + ] + def extract_padding(self, s): + return (b'', s) + += Define the `A` root packet class. + +class A(Packet): + NUMBER_OF_B_PER_A = 100 + fields_desc = [ + PacketField(name=f"b{i}", pkt_cls=B, default=B()) + for i in range(NUMBER_OF_B_PER_A) + ] + += Define `MAX_INSTANTIATION_TIME_EXPECTED`. + +MAX_INSTANTIATION_TIME_EXPECTED = 1.0 + += Define `MAX_SERIALIZATION_TIME_EXPECTED`. + +MAX_SERIALIZATION_TIME_EXPECTED = 0.250 + += Define `MAX_PARSING_TIME_EXPECTED`. + +MAX_PARSING_TIME_EXPECTED = 0.250 + + +############ +############ ++ Default instantiations + += Build a default instance of `C`. + +t0 = time.time() +c = C() +tf = time.time() +check_exec_time("Default instantiation of `C`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) + += Build a default instance of `C` again. + +t0 = time.time() +c = C() +tf = time.time() +check_exec_time("Default instantiation of `C`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) + += Build a default instance of `B`. + +t0 = time.time() +b = B() +tf = time.time() +check_exec_time("Default instantiation of `B`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) + += Build a default instance of `B` again. + +t0 = time.time() +b = B() +tf = time.time() +check_exec_time("Default instantiation of `B`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) + += Build a default instance of `A`. + +t0 = time.time() +a = A() +tf = time.time() +check_exec_time("Default instantiation of `A`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) + += Build a default instance of `A` again. + +t0 = time.time() +a = A() +tf = time.time() +check_exec_time("Default instantiation of `A`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) + + +############ +############ ++ Serialization + += Launch serialization from the latest instance of `A` created. + +t0 = time.time() +b = a.build() +tf = time.time() +check_exec_time("Serialization of `A`", t0, tf, MAX_SERIALIZATION_TIME_EXPECTED) + + +############ +############ ++ Parsing + += Parse the buffer serialized before with the latest instance of `A` created. + +t0 = time.time() +a.dissect(b) +tf = time.time() +check_exec_time("Parsing of `A`", t0, tf, MAX_PARSING_TIME_EXPECTED) From 4bbaab2499945f081723a5edd4bb284127fc80ea Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 31 Mar 2025 18:47:52 +0200 Subject: [PATCH 02/14] Rename classes in 'test/perf_packet_fields.uts' (#4705) Version used for reporting of #4705. --- test/perf_packet_fields.uts | 72 ++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/test/perf_packet_fields.uts b/test/perf_packet_fields.uts index fc73f60d465..bbfc422a63a 100644 --- a/test/perf_packet_fields.uts +++ b/test/perf_packet_fields.uts @@ -15,33 +15,33 @@ def check_exec_time(description, t0, tf, max): ############ + Test data -= Define the `C` final packet class. += Define the `F` final packet class. -class C(Packet): +class F(Packet): fields_desc = [ - ByteField(name="val", default=0), + ByteField(name="value", default=0), ] def extract_padding(self, s): return (b'', s) -= Define the `B` intermediate packet class. += Define the `I` intermediate packet class. -class B(Packet): - NUMBER_OF_C_PER_B = 100 +class I(Packet): + NUMBER_OF_F_PER_I = 100 fields_desc = [ - PacketField(name=f"c{i}", pkt_cls=C, default=C(val=i)) - for i in range(NUMBER_OF_C_PER_B) + PacketField(name=f"f{i}", pkt_cls=F, default=F(value=i)) + for i in range(NUMBER_OF_F_PER_I) ] def extract_padding(self, s): return (b'', s) -= Define the `A` root packet class. += Define the `M` main packet class. -class A(Packet): - NUMBER_OF_B_PER_A = 100 +class M(Packet): + NUMBER_OF_I_PER_M = 100 fields_desc = [ - PacketField(name=f"b{i}", pkt_cls=B, default=B()) - for i in range(NUMBER_OF_B_PER_A) + PacketField(name=f"i{i}", pkt_cls=I, default=I()) + for i in range(NUMBER_OF_I_PER_M) ] = Define `MAX_INSTANTIATION_TIME_EXPECTED`. @@ -61,68 +61,68 @@ MAX_PARSING_TIME_EXPECTED = 0.250 ############ + Default instantiations -= Build a default instance of `C`. += Build a default instance of `F`. t0 = time.time() -c = C() +f = F() tf = time.time() -check_exec_time("Default instantiation of `C`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) +check_exec_time("Default instantiation of `F`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `C` again. += Build a default instance of `F` again. t0 = time.time() -c = C() +f = F() tf = time.time() -check_exec_time("Default instantiation of `C`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) +check_exec_time("Default instantiation of `F`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `B`. += Build a default instance of `I`. t0 = time.time() -b = B() +i = I() tf = time.time() -check_exec_time("Default instantiation of `B`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) +check_exec_time("Default instantiation of `I`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) = Build a default instance of `B` again. t0 = time.time() -b = B() +i = I() tf = time.time() -check_exec_time("Default instantiation of `B`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) +check_exec_time("Default instantiation of `I`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `A`. += Build a default instance of `M`. t0 = time.time() -a = A() +m = M() tf = time.time() -check_exec_time("Default instantiation of `A`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) +check_exec_time("Default instantiation of `M`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `A` again. += Build a default instance of `M` again. t0 = time.time() -a = A() +m = M() tf = time.time() -check_exec_time("Default instantiation of `A`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) +check_exec_time("Default instantiation of `M`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) ############ ############ + Serialization -= Launch serialization from the latest instance of `A` created. += Launch serialization from the latest instance of `M` created. t0 = time.time() -b = a.build() +buf = m.build() tf = time.time() -check_exec_time("Serialization of `A`", t0, tf, MAX_SERIALIZATION_TIME_EXPECTED) +check_exec_time("Serialization of `M`", t0, tf, MAX_SERIALIZATION_TIME_EXPECTED) ############ ############ + Parsing -= Parse the buffer serialized before with the latest instance of `A` created. += Parse the buffer serialized before with the latest instance of `M` created. t0 = time.time() -a.dissect(b) +m.dissect(buf) tf = time.time() -check_exec_time("Parsing of `A`", t0, tf, MAX_PARSING_TIME_EXPECTED) +check_exec_time("Parsing of `M`", t0, tf, MAX_PARSING_TIME_EXPECTED) From 9bf0c549f86afd9f828cb40a0aec42e6b27eec90 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 31 Mar 2025 19:08:23 +0200 Subject: [PATCH 03/14] Add 'test/auto_clear_cache.uts' test (#4706) --- test/auto_clear_cache.uts | 175 ++++++++++++++++++++++++++++++++++++ test/perf_packet_fields.uts | 3 + 2 files changed, 178 insertions(+) create mode 100644 test/auto_clear_cache.uts diff --git a/test/auto_clear_cache.uts b/test/auto_clear_cache.uts new file mode 100644 index 00000000000..1f5570338d6 --- /dev/null +++ b/test/auto_clear_cache.uts @@ -0,0 +1,175 @@ +% No auto clear cache issue + +# See https://github.com/secdev/scapy/issues/4706 + + +############ +############ ++ Test utils + += Prepare a `check_equal()` function. + +def check_equal(description, measured, expected): + assert measured == expected, f"{description!r} FAILED: {measured!r} != {expected!r}" + print(f"{description}: {measured!r}") + + +############ +############ ++ Test data + += Define a final packet class. + +class F(Packet): + fields_desc = [ + ByteField(name="value", default=0), + ] + + +############ +############ ++ Auto clear cache on final packet + += Instantiate a packet of the final packet class with cache. + +f = F(bytes.fromhex("01")) +f.show() +check_equal("Cache is set", f.raw_packet_cache, bytes.fromhex("01")) + += Change the value of the `F.value` field. + +f.value = 2 +f.show() + += Check the final packet has no cache anymore. + +check_equal("Cache is reset", f.raw_packet_cache, None) + += Serialize the resulting packet. + +buf = f.build() +check_equal("f.build()", buf, bytes.fromhex("02")) + += Check whether the final packet has cache after `build()`. + +# Just watch, don't assert. +# Note: Behaviour changed after #4705. +print(f"f.raw_packet_cache={f.raw_packet_cache!r}") + + +############ +############ ++ Auto clear cache with payload + += Define a packet class bound with final class after it. + +class M0(Packet): + fields_desc = [ + StrFixedLenField("foo", length_from=lambda pkt: 0, default=b''), + ] + +bind_layers(M0, F) + += Instantiate a packet of this class with cache. + +m = M0(bytes.fromhex("01")) +m.show() +check_equal("Cache is set", m.raw_packet_cache, b'') +check_equal("Cache is set", m.payload.raw_packet_cache, bytes.fromhex("01")) + += Change the value of the `F.value` field. + +m.value = 2 +m.show() + += Check the underlayer packet has no cache anymore. + +check_equal("Cache is reset", m.raw_packet_cache, None) + += Serialize the resulting packet. + +buf = m.build() +check_equal("m.build()", buf, bytes.fromhex("02")) + += Check whether the underlayer packet has cache. + +# Just watch, don't assert. +# Note: Behaviour changed after #4705. +print(f"m.raw_packet_cache={m.raw_packet_cache!r}") + + +############ +############ ++ Auto clear cache with `PacketField` + += Define a packet class using a `PacketField` + +class M1(Packet): + fields_desc = [ + PacketField("f", pkt_cls=F, default=F()), + ] + += Instantiate a packet of this class with cache. + +m = M1(bytes.fromhex("01")) +m.show() +check_equal("Cache is set", m.raw_packet_cache, bytes.fromhex("01")) + += Change the value of the `F.value` field. + +m.f.value = 2 +m.show() + += Check the parent packet has no cache anymore. + +check_equal("Cache is reset", m.raw_packet_cache, None) + += Serialize the resulting packet. + +buf = m.build() +check_equal("m.build()", buf, bytes.fromhex("02")) + += Check whether the parent packet has cache. + +# Just watch, don't assert. +# Note: Behaviour changed after #4705. +print(f"m.raw_packet_cache={m.raw_packet_cache!r}") + + +############ +############ ++ Auto clear cache with `PacketListField` + += Define a packet class using a `PacketListField` + +class M2(Packet): + fields_desc = [ + PacketListField("f", pkt_cls=F, count_from=lambda pkkt: 1, default=[]), + ] + += Instantiate a packet of this class with cache. + +m = M2(bytes.fromhex("01")) +m.show() +check_equal("Cache is set", m.raw_packet_cache, bytes.fromhex("01")) + += Change the value of the `F.value` field. + +m.f[0].value = 2 +m.show() + += Check the parent packet has no cache anymore. + +check_equal("Cache is reset", m.raw_packet_cache, None) + += Serialize the resulting packet. + +buf = m.build() +check_equal("m.build()", buf, bytes.fromhex("02")) + += Check whether the parent packet has cache. + +# Just watch, don't assert. +# Note: Not reset in v2.4.5, reset in v2.5.0. +# Note: Behaviour changed after #4705. +print(f"m.raw_packet_cache={m.raw_packet_cache!r}") diff --git a/test/perf_packet_fields.uts b/test/perf_packet_fields.uts index bbfc422a63a..0867033e1bd 100644 --- a/test/perf_packet_fields.uts +++ b/test/perf_packet_fields.uts @@ -1,5 +1,8 @@ % Packet field performance issue +# See https://github.com/secdev/scapy/issues/4705 + + ############ ############ + Test utils From 772459f6b8c1b79987681c0294c916d53220fd6a Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Sun, 6 Apr 2025 21:15:39 +0200 Subject: [PATCH 04/14] Improve 'perf_packet_fields.uts' (#4705) - Set step numbers. - Clarify max times. - Use regular parsing through instantiation in step 014. --- test/perf_packet_fields.uts | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/test/perf_packet_fields.uts b/test/perf_packet_fields.uts index 0867033e1bd..0c19f2ab733 100644 --- a/test/perf_packet_fields.uts +++ b/test/perf_packet_fields.uts @@ -7,7 +7,7 @@ ############ + Test utils -= Prepare a `check_exec_time()` function. += 000) Prepare a `check_exec_time()` function. def check_exec_time(description, t0, tf, max): print(f"{description}: {tf - t0:.3f} seconds") @@ -18,7 +18,7 @@ def check_exec_time(description, t0, tf, max): ############ + Test data -= Define the `F` final packet class. += 001) Define the `F` final packet class. class F(Packet): fields_desc = [ @@ -27,7 +27,7 @@ class F(Packet): def extract_padding(self, s): return (b'', s) -= Define the `I` intermediate packet class. += 002) Define the `I` intermediate packet class. class I(Packet): NUMBER_OF_F_PER_I = 100 @@ -38,7 +38,7 @@ class I(Packet): def extract_padding(self, s): return (b'', s) -= Define the `M` main packet class. += 003) Define the `M` main packet class. class M(Packet): NUMBER_OF_I_PER_M = 100 @@ -47,59 +47,62 @@ class M(Packet): for i in range(NUMBER_OF_I_PER_M) ] -= Define `MAX_INSTANTIATION_TIME_EXPECTED`. += 004) Define `MAX_INSTANTIATION_TIME_EXPECTED`. +# About 250ms measured with optimizations on Windows / Python 3.8.6 MAX_INSTANTIATION_TIME_EXPECTED = 1.0 -= Define `MAX_SERIALIZATION_TIME_EXPECTED`. += 005) Define `MAX_SERIALIZATION_TIME_EXPECTED`. +# About 50ms measured with optimizations on Windows / Python 3.8.6 MAX_SERIALIZATION_TIME_EXPECTED = 0.250 -= Define `MAX_PARSING_TIME_EXPECTED`. += 006) Define `MAX_PARSING_TIME_EXPECTED`. -MAX_PARSING_TIME_EXPECTED = 0.250 +# 2 * MAX_INSTANTIATION_TIME_EXPECTED: 1 for default instantiation, and 1 for parsing. +MAX_PARSING_TIME_EXPECTED = MAX_INSTANTIATION_TIME_EXPECTED * 2 ############ ############ + Default instantiations -= Build a default instance of `F`. += 007) Build a default instance of `F`. t0 = time.time() f = F() tf = time.time() check_exec_time("Default instantiation of `F`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `F` again. += 008) Build a default instance of `F` again. t0 = time.time() f = F() tf = time.time() check_exec_time("Default instantiation of `F`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `I`. += 009) Build a default instance of `I`. t0 = time.time() i = I() tf = time.time() check_exec_time("Default instantiation of `I`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `B` again. += 010) Build a default instance of `B` again. t0 = time.time() i = I() tf = time.time() check_exec_time("Default instantiation of `I`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `M`. += 011) Build a default instance of `M`. t0 = time.time() m = M() tf = time.time() check_exec_time("Default instantiation of `M`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) -= Build a default instance of `M` again. += 012) Build a default instance of `M` again. t0 = time.time() m = M() @@ -111,7 +114,7 @@ check_exec_time("Default instantiation of `M`", t0, tf, MAX_INSTANTIATION_TIME_E ############ + Serialization -= Launch serialization from the latest instance of `M` created. += 013) Launch serialization from the latest instance of `M` created. t0 = time.time() buf = m.build() @@ -123,9 +126,9 @@ check_exec_time("Serialization of `M`", t0, tf, MAX_SERIALIZATION_TIME_EXPECTED) ############ + Parsing -= Parse the buffer serialized before with the latest instance of `M` created. += 014) Parse the buffer serialized before. t0 = time.time() -m.dissect(buf) +m = M(buf) tf = time.time() check_exec_time("Parsing of `M`", t0, tf, MAX_PARSING_TIME_EXPECTED) From cc2572d3d84bc7ff00aeb7e33f0bd8e36f227fe2 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 7 Apr 2025 00:38:55 +0200 Subject: [PATCH 05/14] Improve 'auto_clear_cache.uts' (#4706, #4707) - Set step numbers. --- test/auto_clear_cache.uts | 50 +++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/test/auto_clear_cache.uts b/test/auto_clear_cache.uts index 1f5570338d6..db355fc5dbd 100644 --- a/test/auto_clear_cache.uts +++ b/test/auto_clear_cache.uts @@ -7,7 +7,7 @@ ############ + Test utils -= Prepare a `check_equal()` function. += 000) Prepare a `check_equal()` function. def check_equal(description, measured, expected): assert measured == expected, f"{description!r} FAILED: {measured!r} != {expected!r}" @@ -18,7 +18,7 @@ def check_equal(description, measured, expected): ############ + Test data -= Define a final packet class. += 001) Define a final packet class. class F(Packet): fields_desc = [ @@ -30,27 +30,27 @@ class F(Packet): ############ + Auto clear cache on final packet -= Instantiate a packet of the final packet class with cache. += 002) Instantiate a packet of the final packet class with cache. f = F(bytes.fromhex("01")) f.show() check_equal("Cache is set", f.raw_packet_cache, bytes.fromhex("01")) -= Change the value of the `F.value` field. += 003) Change the value of the `F.value` field. f.value = 2 f.show() -= Check the final packet has no cache anymore. += 004) Check the final packet has no cache anymore. check_equal("Cache is reset", f.raw_packet_cache, None) -= Serialize the resulting packet. += 005) Serialize the resulting packet. buf = f.build() check_equal("f.build()", buf, bytes.fromhex("02")) -= Check whether the final packet has cache after `build()`. += 006) Check whether the final packet has cache after `build()`. # Just watch, don't assert. # Note: Behaviour changed after #4705. @@ -61,7 +61,7 @@ print(f"f.raw_packet_cache={f.raw_packet_cache!r}") ############ + Auto clear cache with payload -= Define a packet class bound with final class after it. += 007) Define a packet class bound with final class after it. class M0(Packet): fields_desc = [ @@ -70,28 +70,28 @@ class M0(Packet): bind_layers(M0, F) -= Instantiate a packet of this class with cache. += 008) Instantiate a packet of this class with cache. m = M0(bytes.fromhex("01")) m.show() check_equal("Cache is set", m.raw_packet_cache, b'') check_equal("Cache is set", m.payload.raw_packet_cache, bytes.fromhex("01")) -= Change the value of the `F.value` field. += 009) Change the value of the `F.value` field. m.value = 2 m.show() -= Check the underlayer packet has no cache anymore. += 010) Check the underlayer packet has no cache anymore. check_equal("Cache is reset", m.raw_packet_cache, None) -= Serialize the resulting packet. += 011) Serialize the resulting packet. buf = m.build() check_equal("m.build()", buf, bytes.fromhex("02")) -= Check whether the underlayer packet has cache. += 012) Check whether the underlayer packet has cache. # Just watch, don't assert. # Note: Behaviour changed after #4705. @@ -102,34 +102,34 @@ print(f"m.raw_packet_cache={m.raw_packet_cache!r}") ############ + Auto clear cache with `PacketField` -= Define a packet class using a `PacketField` += 013) Define a packet class using a `PacketField` class M1(Packet): fields_desc = [ PacketField("f", pkt_cls=F, default=F()), ] -= Instantiate a packet of this class with cache. += 014) Instantiate a packet of this class with cache. m = M1(bytes.fromhex("01")) m.show() check_equal("Cache is set", m.raw_packet_cache, bytes.fromhex("01")) -= Change the value of the `F.value` field. += 015) Change the value of the `F.value` field. m.f.value = 2 m.show() -= Check the parent packet has no cache anymore. += 016) Check the parent packet has no cache anymore. check_equal("Cache is reset", m.raw_packet_cache, None) -= Serialize the resulting packet. += 017) Serialize the resulting packet. buf = m.build() check_equal("m.build()", buf, bytes.fromhex("02")) -= Check whether the parent packet has cache. += 018) Check whether the parent packet has cache. # Just watch, don't assert. # Note: Behaviour changed after #4705. @@ -140,34 +140,34 @@ print(f"m.raw_packet_cache={m.raw_packet_cache!r}") ############ + Auto clear cache with `PacketListField` -= Define a packet class using a `PacketListField` += 019) Define a packet class using a `PacketListField` class M2(Packet): fields_desc = [ PacketListField("f", pkt_cls=F, count_from=lambda pkkt: 1, default=[]), ] -= Instantiate a packet of this class with cache. += 020) Instantiate a packet of this class with cache. m = M2(bytes.fromhex("01")) m.show() check_equal("Cache is set", m.raw_packet_cache, bytes.fromhex("01")) -= Change the value of the `F.value` field. += 021) Change the value of the `F.value` field. m.f[0].value = 2 m.show() -= Check the parent packet has no cache anymore. += 022) Check the parent packet has no cache anymore. check_equal("Cache is reset", m.raw_packet_cache, None) -= Serialize the resulting packet. += 023) Serialize the resulting packet. buf = m.build() check_equal("m.build()", buf, bytes.fromhex("02")) -= Check whether the parent packet has cache. += 024) Check whether the parent packet has cache. # Just watch, don't assert. # Note: Not reset in v2.4.5, reset in v2.5.0. From 2d3c00529b4f6179eeec3931d50ef30ed4d927a9 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 23 Apr 2025 22:23:43 +0200 Subject: [PATCH 06/14] Improve 'perf_packet_fields.uts' with operation counts (#4705) - Test configurations extracted as global constants. - Base `TestPacket` class added. Made `M`, `I` and `F` inherit from it. - Max time computed from expected number of operations. - Default instantiations ran once (not twice anymore). - Serialization test cases added to highlight cache improvements. --- test/perf_packet_fields.uts | 182 ++++++++++++++++++++++-------------- 1 file changed, 112 insertions(+), 70 deletions(-) diff --git a/test/perf_packet_fields.uts b/test/perf_packet_fields.uts index 0c19f2ab733..7ddbaead096 100644 --- a/test/perf_packet_fields.uts +++ b/test/perf_packet_fields.uts @@ -7,11 +7,61 @@ ############ + Test utils -= 000) Prepare a `check_exec_time()` function. - -def check_exec_time(description, t0, tf, max): - print(f"{description}: {tf - t0:.3f} seconds") - assert tf - t0 <= max, f"{description!r} FAILED: {tf - t0:.3f} > {max:.3f}" += 000) Define `NUMBER_OF_I_PER_M` and `NUMBER_OF_F_PER_I` constants, and prepare a base `TestPacket` class. + +NUMBER_OF_I_PER_M = 100 +NUMBER_OF_F_PER_I = 100 + +# Other configurations: +# - Make `TestPacket.end_case()` check for exact operation counts. +ASSERT_COUNTS = False +# - About 1.5s measured with optimizations on Windows / Python 3.8.6 with NUMBER_OF_I_PER_M=100, NUMBER_OF_F_PER_I=100. +MAX_TIME_PER_OP = (1.5 / (100.0 * 100.0)) * 1.10 # 10% margin. + +class TestPacket(Packet): + """Base class for `M`, `I` and `F`.""" + inits = {} + builds = {} + t0 = 0.0 + @staticmethod + def begin_case(): + """Start a test case.""" + TestPacket.inits = {M: 0, I: 0, F: 0} + TestPacket.builds = {M: 0, I: 0, F: 0} + TestPacket.t0 = time.time() + def __init__(self, *args, **kwargs): + """Count inits.""" + Packet.__init__(self, *args, **kwargs) + TestPacket.inits[type(self)] = TestPacket.inits.get(type(self), 0) + 1 + def self_build(self): + """Count builds.""" + TestPacket.builds[type(self)] = TestPacket.builds.get(type(self), 0) + 1 + return Packet.self_build(self) + def extract_padding(self, s): + """Final packet fields (no payload).""" + return (b'', s) + @staticmethod + def end_case( + init_m=0, init_i=0, init_f=0, + build_m=0, build_i=0, build_f=0, + ): + """End a test case.""" + tf = time.time() + operations = 0 + for cls, expected in [(M, init_m), (I, init_i), (F, init_f)]: + print(f"Number of {cls.__name__} inits: {TestPacket.inits[cls]} (expected: {expected})") + if ASSERT_COUNTS: + assert TestPacket.inits[cls] == expected, f"Number of {cls.__name__} inits FAILED: {TestPacket.inits[cls]} != {expected}" + operations += expected + for cls, expected in [(M, build_m), (I, build_i), (F, build_f)]: + print(f"Number of {cls.__name__} builds: {TestPacket.builds[cls]} (expected: {expected})") + if ASSERT_COUNTS: + assert TestPacket.builds[cls] == expected, f"Number of {cls.__name__} builds FAILED: {TestPacket.builds[cls]} != {expected}" + operations += expected + print(f"Total number of operations expected: {operations}") + max_time = max(operations * MAX_TIME_PER_OP, 0.010) # Minimal time of 10 ms. + print(f"Execution time: {tf - TestPacket.t0:.3f} seconds (max: {max_time:.3f})") + assert tf - TestPacket.t0 <= max_time, f"Execution time FAILED: {tf - TestPacket.t0:.3f} > {max_time:.3f}" ############ @@ -20,115 +70,107 @@ def check_exec_time(description, t0, tf, max): = 001) Define the `F` final packet class. -class F(Packet): +class F(TestPacket): fields_desc = [ ByteField(name="value", default=0), ] - def extract_padding(self, s): - return (b'', s) = 002) Define the `I` intermediate packet class. -class I(Packet): - NUMBER_OF_F_PER_I = 100 +class I(TestPacket): fields_desc = [ PacketField(name=f"f{i}", pkt_cls=F, default=F(value=i)) for i in range(NUMBER_OF_F_PER_I) ] - def extract_padding(self, s): - return (b'', s) = 003) Define the `M` main packet class. -class M(Packet): - NUMBER_OF_I_PER_M = 100 +class M(TestPacket): fields_desc = [ PacketField(name=f"i{i}", pkt_cls=I, default=I()) for i in range(NUMBER_OF_I_PER_M) ] -= 004) Define `MAX_INSTANTIATION_TIME_EXPECTED`. - -# About 250ms measured with optimizations on Windows / Python 3.8.6 -MAX_INSTANTIATION_TIME_EXPECTED = 1.0 - -= 005) Define `MAX_SERIALIZATION_TIME_EXPECTED`. - -# About 50ms measured with optimizations on Windows / Python 3.8.6 -MAX_SERIALIZATION_TIME_EXPECTED = 0.250 - -= 006) Define `MAX_PARSING_TIME_EXPECTED`. - -# 2 * MAX_INSTANTIATION_TIME_EXPECTED: 1 for default instantiation, and 1 for parsing. -MAX_PARSING_TIME_EXPECTED = MAX_INSTANTIATION_TIME_EXPECTED * 2 - ############ ############ + Default instantiations -= 007) Build a default instance of `F`. += 004) Build a default instance of `F`. -t0 = time.time() +TestPacket.begin_case() f = F() -tf = time.time() -check_exec_time("Default instantiation of `F`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) - -= 008) Build a default instance of `F` again. +TestPacket.end_case( + init_f=1, +) -t0 = time.time() -f = F() -tf = time.time() -check_exec_time("Default instantiation of `F`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) += 005) Build a default instance of `I`. -= 009) Build a default instance of `I`. - -t0 = time.time() -i = I() -tf = time.time() -check_exec_time("Default instantiation of `I`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) - -= 010) Build a default instance of `B` again. - -t0 = time.time() +TestPacket.begin_case() i = I() -tf = time.time() -check_exec_time("Default instantiation of `I`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) - -= 011) Build a default instance of `M`. - -t0 = time.time() -m = M() -tf = time.time() -check_exec_time("Default instantiation of `M`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) +TestPacket.end_case( + init_i=1, + init_f=NUMBER_OF_F_PER_I, +) -= 012) Build a default instance of `M` again. += 006) Build a default instance of `M`. -t0 = time.time() +TestPacket.begin_case() m = M() -tf = time.time() -check_exec_time("Default instantiation of `M`", t0, tf, MAX_INSTANTIATION_TIME_EXPECTED) +TestPacket.end_case( + init_m=1, + init_i=NUMBER_OF_I_PER_M, + init_f=NUMBER_OF_I_PER_M * NUMBER_OF_F_PER_I, +) ############ ############ + Serialization -= 013) Launch serialization from the latest instance of `M` created. += 007) Launch serialization from the latest instance of `M` created. + +TestPacket.begin_case() +buf = m.build() +TestPacket.end_case( + build_m=1, + build_i=NUMBER_OF_I_PER_M, + build_f=NUMBER_OF_I_PER_M * NUMBER_OF_F_PER_I, +) + += 008) Launch serialization again from the same instance of `M`. + +TestPacket.begin_case() +buf = m.build() +TestPacket.end_case( + build_m=1, # `m` is cached, `build()` not spreaded on `i{n}` fields (thus, no `f{n}`). + build_i=0, + build_f=0, +) + += 009) Update one `F` from the same instance of `M` and launch serialization again. + +m.i0.f0.value += 1 -t0 = time.time() +TestPacket.begin_case() buf = m.build() -tf = time.time() -check_exec_time("Serialization of `M`", t0, tf, MAX_SERIALIZATION_TIME_EXPECTED) +TestPacket.end_case( + build_m=1, + build_i=NUMBER_OF_I_PER_M, # `i0` gets rebuilts, next `i{n}` fields are just asked for their cache. + build_f=1 * NUMBER_OF_F_PER_I, # `i0.f0` gets rebuilt, next `i0.f{n}` fields are just asked for their cache. +) ############ ############ + Parsing -= 014) Parse the buffer serialized before. += 010) Parse the buffer serialized before. -t0 = time.time() +TestPacket.begin_case() m = M(buf) -tf = time.time() -check_exec_time("Parsing of `M`", t0, tf, MAX_PARSING_TIME_EXPECTED) +TestPacket.end_case( + init_m=1, + init_i=NUMBER_OF_I_PER_M, + init_f=NUMBER_OF_I_PER_M * NUMBER_OF_F_PER_I, +) From 6e72bc3db0360a44f745b645d648feeda0cca3b1 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 23 Apr 2025 23:27:59 +0200 Subject: [PATCH 07/14] Fix 'perf_packet_fields.uts' when `NUMBER_OF_F_PER_I>255` (#4705) --- test/perf_packet_fields.uts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/perf_packet_fields.uts b/test/perf_packet_fields.uts index 7ddbaead096..bb2e54b57cd 100644 --- a/test/perf_packet_fields.uts +++ b/test/perf_packet_fields.uts @@ -79,7 +79,7 @@ class F(TestPacket): class I(TestPacket): fields_desc = [ - PacketField(name=f"f{i}", pkt_cls=F, default=F(value=i)) + PacketField(name=f"f{i}", pkt_cls=F, default=F(value=(i%256))) for i in range(NUMBER_OF_F_PER_I) ] From af50227bf4b27970f99d78a2256a1e2ab43341a6 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Thu, 3 Apr 2025 15:43:20 +0200 Subject: [PATCH 08/14] Report scapy@2.4.5 optimizations in scapy@2.6.1 (#4705, #4706, #4707) - #4705: Performance issues with nested `PacketField`s: - Cache saved in `Packet.self_build()`. - `Packet`s marked as explicit systematically in `Packet.build()` to avoid a useless clone in `do_build()`. - `Packet.do_init_cached_fields()` optimized: - `init_fields` parameter added to `Packet.init_fields()` and `do_init_cached_fields()`. - Misc: `do_init_cached_fields()` fixed for list field copies. - `Packet.prepare_cached_fields()` optimized: - Default fields not copied anymore. - `Packet.copy()` and `clone_with()` optimized with `_fast_copy()`: - Avoid instantiating *ref fields* with default instantiation. - `Packet.copy_fields_dict()` optimized with new `use_fields_already_set` parameter and reworked as an inner method to work directly on the `clone` instance. - Misc: `_T` variable type at the module level made local and renamed as `_VarDictFieldType` in `_fast_copy()`. - `Packet.setfieldval()` optimized: - New value discarded if unchanged.: - `_PacketField.m2i()` optimized: - Avoid useless copies of fields with default instance. - Then `dissect()` is used on it. - #4707: [enhancement] Enable clear_cache() to spread upward in the packet tree: - `Packet.clear_cache()` reworked. - #4706: Force cache reset as soon as the payload or a packet field is modified: - `Packet.clear_cache()` (reworked as per #4707) called in `setfieldval()`, `delfieldval()`, `add_payload()` and `remove_payload()`. - `Packet.raw_packet_cache_fields` now useless, thus removed. - Same for `Packet._raw_packet_cache_field_value()`. - `Packet.self_build()` simplified by the way. - `Packet.no_cache` flag added to avoid setting `raw_packet_cache` when specified, used in `Packet.self_build()` and `do_dissect()`. --- scapy/fields.py | 33 ++++- scapy/packet.py | 349 +++++++++++++++++++++++++++++++++--------------- 2 files changed, 266 insertions(+), 116 deletions(-) diff --git a/scapy/fields.py b/scapy/fields.py index 06d73225589..e075faef7c8 100644 --- a/scapy/fields.py +++ b/scapy/fields.py @@ -1554,11 +1554,34 @@ def i2m(self, def m2i(self, pkt, m): # type: ignore # type: (Optional[Packet], bytes) -> Packet - try: - # we want to set parent wherever possible - return self.cls(m, _parent=pkt) # type: ignore - except TypeError: - return self.cls(m) + # Do not import at module level on runtime ! (import loop) => local import + from scapy.packet import Packet + + # Build a default instance. + if isinstance(self.cls, type): + # Instantiation from packet class. + if TYPE_CHECKING: + assert issubclass(self.cls, Packet) + # Pre-set all default references with `None` values in order to avoid useless copies of default instances. + # They will be parsed after with the `dissect()` call below. + init_fields = { + field_name: None + for field_name in Packet.class_default_fields_ref.get(self.cls, {}) + } # type: Dict[str, None] + new_pkt = self.cls(**init_fields) # type: Packet + else: + # Instantiation from callback. + new_pkt = self.cls(b'') + + # we want to set parent wherever possible + # (only if `pkt` and `new_pkt` are both packets). + if isinstance(pkt, Packet) and isinstance(new_pkt, Packet): + new_pkt.parent = pkt + + # Eventually dissect the buffer to parse. + new_pkt.dissect(m) + + return new_pkt class _PacketFieldSingle(_PacketField[K]): diff --git a/scapy/packet.py b/scapy/packet.py index 0e7edee4938..ac4ef8986c4 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -67,6 +67,7 @@ Union, Sequence, cast, + TYPE_CHECKING, ) from scapy.compat import Self @@ -76,9 +77,6 @@ pass -_T = TypeVar("_T", Dict[str, Any], Optional[Dict[str, Any]]) - - class Packet( BasePacket, _CanvasDumpExtended, @@ -90,7 +88,7 @@ class Packet( "overload_fields", "overloaded_fields", "packetfields", "original", "explicit", "raw_packet_cache", - "raw_packet_cache_fields", "_pkt", "post_transforms", + "_pkt", "post_transforms", "stop_dissection_after", # then payload, underlayer and parent "payload", "underlayer", "parent", @@ -118,6 +116,12 @@ class Packet( class_default_fields_ref = {} # type: Dict[Type[Packet], List[str]] class_fieldtype = {} # type: Dict[Type[Packet], Dict[str, AnyField]] # noqa: E501 + #: Set to ``True`` for classes which should not save a :attr:`raw_packet_cache`. + #: + #: Useful when the packet contains auto fields depending on external content + #: (parent or neighbour packets). + no_cache = False # type: bool + @classmethod def from_hexcap(cls): # type: (Type[Packet]) -> Packet @@ -167,15 +171,17 @@ def __init__(self, self.fieldtype = {} # type: Dict[str, AnyField] self.packetfields = [] # type: List[AnyField] self.payload = NoPayload() # type: Packet - self.init_fields(bool(_pkt)) + self.init_fields(for_dissect_only=bool(_pkt), init_fields=fields) self.underlayer = _underlayer self.parent = _parent if isinstance(_pkt, bytearray): _pkt = bytes(_pkt) self.original = _pkt + # TODO: + # Clarify the meaning for this `explicit` flag. + # Now that cache is cleared as soon as the packet is modified, possibly we could get rid of it? self.explicit = 0 self.raw_packet_cache = None # type: Optional[bytes] - self.raw_packet_cache_fields = None # type: Optional[Dict[str, Any]] # noqa: E501 self.wirelen = None # type: Optional[int] self.direction = None # type: Optional[int] self.sniffed_on = None # type: Optional[_GlobInterfaceType] @@ -253,8 +259,8 @@ def __deepcopy__(self, """Used by copy.deepcopy""" return self.copy() - def init_fields(self, for_dissect_only=False): - # type: (bool) -> None + def init_fields(self, for_dissect_only=False, init_fields=None): + # type: (bool, Optional[Dict[str, Any]]) -> None """ Initialize each fields of the fields_desc dict """ @@ -262,7 +268,7 @@ def init_fields(self, for_dissect_only=False): if self.class_dont_cache.get(self.__class__, False): self.do_init_fields(self.fields_desc) else: - self.do_init_cached_fields(for_dissect_only=for_dissect_only) + self.do_init_cached_fields(for_dissect_only=for_dissect_only, init_fields=init_fields) def do_init_fields(self, flist, # type: Sequence[AnyField] @@ -280,8 +286,8 @@ def do_init_fields(self, # We set default_fields last to avoid race issues self.default_fields = default_fields - def do_init_cached_fields(self, for_dissect_only=False): - # type: (bool) -> None + def do_init_cached_fields(self, for_dissect_only=False, init_fields=None): + # type: (bool, Optional[Dict[str, Any]]) -> None """ Initialize each fields of the fields_desc dict, or use the cached fields information @@ -300,18 +306,18 @@ def do_init_cached_fields(self, for_dissect_only=False): self.fieldtype = Packet.class_fieldtype[cls_name] self.packetfields = Packet.class_packetfields[cls_name] - # Optimization: no need for references when only dissecting. + # Optimization: no need for default references when only dissecting. if for_dissect_only: return # Deepcopy default references for fname in Packet.class_default_fields_ref[cls_name]: - value = self.default_fields[fname] - try: - self.fields[fname] = value.copy() - except AttributeError: - # Python 2.7 - list only - self.fields[fname] = value[:] + # Optimization: no need for a default reference when the value is given on init. + if init_fields and (fname in init_fields): + continue + + # Fix: Use `copy_field_value()` instead of just `value.copy()`, in order to duplicate list items as well in case of a list. + self.fields[fname] = self.copy_field_value(fname, self.default_fields[fname]) def prepare_cached_fields(self, flist): # type: (Sequence[AnyField]) -> None @@ -338,8 +344,11 @@ def prepare_cached_fields(self, flist): self.do_init_fields(self.fields_desc) return - class_default_fields[f.name] = copy.deepcopy(f.default) + # Default field values. + class_default_fields[f.name] = f.default # Optimization: No need to make a copy. + # Field types. class_fieldtype[f.name] = f + # Packet fields. if f.holds_packets: class_packetfields.append(f) @@ -389,12 +398,18 @@ def add_payload(self, payload): else: raise TypeError("payload must be 'Packet', 'bytes', 'str', 'bytearray', or 'memoryview', not [%s]" % repr(payload)) # noqa: E501 + # Invalidate cache when the packet has changed. + self.clear_cache() + def remove_payload(self): # type: () -> None self.payload.remove_underlayer(self) self.payload = NoPayload() self.overloaded_fields = {} + # Invalidate cache when the packet has changed. + self.clear_cache() + def add_underlayer(self, underlayer): # type: (Packet) -> None self.underlayer = underlayer @@ -419,25 +434,151 @@ def remove_parent(self, other): def copy(self) -> Self: """Returns a deep copy of the instance.""" - clone = self.__class__() - clone.fields = self.copy_fields_dict(self.fields) - clone.default_fields = self.copy_fields_dict(self.default_fields) + return self._fast_copy(for_clone_with=False) + + def clone_with(self, payload=None, **kargs): + # type: (Optional[Any], **Any) -> Any + return self._fast_copy( + for_clone_with=True, + payload=payload, + **kargs + ) + + def _fast_copy( + self, + for_clone_with, # type: bool + payload=None, # type: Optional[Any] + **kargs # type: Any + ): # type: (...) -> Self + """ + Optimized implementation for :meth:`copy()` and :meth:`clone_with()`. + """ + + # Both `copy()` and `clone_with()` start with a default instantiation. + # + # Pre-set *ref fields* in order to avoid instantiations of useless default instances. + # + # Memo: + # *Ref fields* are fields whose default is of type `list`, `dict`, `set`, `RandField` or `Packet`. + # See `prepare_cached_fields()`. + def _init_fields(): # type: (...) -> Dict[str, Any] + source_fields = kargs if for_clone_with else self.fields # type: Dict[str, Any] + init_fields = {} # type: Dict[str, Any] + # Walk *ref field* names. + for field_name in Packet.class_default_fields_ref.get(self.__class__, {}): # type: str + if field_name in source_fields: + if for_clone_with: + # `clone_with()`: Take value from `kargs` without making a copy. + init_fields[field_name] = source_fields[field_name] + else: + # `copy()`: Prepare a copy of the *ref field* installed in `self.fields` for the new instance. + init_fields[field_name] = self.copy_field_value(field_name, source_fields[field_name]) + return init_fields + clone = self.__class__(**_init_fields()) # type: Self + + if TYPE_CHECKING: + _VarDictFieldType = TypeVar( + "_VarDictFieldType", + Dict[str, Any], + Optional[Dict[str, Any]], + ) + + def _fast_copy_fields_dict( + fields, # type: _VarDictFieldType + use_fields_already_set=True, # type: bool + ): # type: (...) -> _VarDictFieldType + """ + Optimized version of former :meth:`copy_fields_dict()`, + defined as an inner function + in order to work directly on the ``clone`` instance being built. + """ + if fields is None: + return None + + fields_dict = {} # type: Dict[str, Any] + for fname, fval in fields.items(): # type: str, Any + if use_fields_already_set and (fname in clone.fields): + # Optimization: Don't instantiate again a field that has already been set / copied. + fields_dict[fname] = clone.fields[fname] + else: + # Default `copy_fields_dict()` behaviour. + fields_dict[fname] = self.copy_field_value(fname, fval) + return fields_dict + + # One of the first things that `copy()` and `clone_with()` do is to set the `fields` dictionary. + if for_clone_with: + # `clone_with()` explicitly uses `kwargs` directly, without making copies. + clone.explicit = 1 + clone.fields = kargs + else: + # `copy()` uses the `copy_fields_dict()` method, and copies the `explicit` attribute as is. + clone.fields = _fast_copy_fields_dict(self.fields) + clone.explicit = self.explicit + + # Both `copy()` and `clone_with()` set the `default_fields` attribute with the `copy_fields_dict()` method. + # + # Memo: + # `default_fields` is generally set with a simple copy of the related `class_default_fields` entry in `do_init_cached_fields()`. + # May be fully recomputed in `do_init_fields()`, but the latter method is called only in case of *dont-cache* packets + # (`Packet.init_fields()` terminology, different from the `Packet.no_cache` flag). + if Packet.class_dont_cache.get(self.__class__, False): + clone.default_fields = _fast_copy_fields_dict( + self.default_fields, + use_fields_already_set=False, # Default fields may not be the same as the fields currently set. + ) + else: + # Optimization: Don't make copies of default fields. + clone.default_fields = self.default_fields + + # Both `copy()` and `clone_with()` set the `overloaded_fields` attribute with `self.overloaded_fields.copy()`. + # + # Memo: + # `overloaded_fields` is a `Dict[str, Any]`. + # It describes fields contained in the payload if any. + # + # Todo: + # It seems quite dangerous to just copy the dictionary, but not clone its potential packet or list values... + # Why wasn't `copy_fields_dict()` used? + # By the way, let's stick with former implementations. clone.overloaded_fields = self.overloaded_fields.copy() + + # Both `copy()` and `clone_with()` copy the `underlayer` reference as is. clone.underlayer = self.underlayer + + # Both `copy()` and `clone_with()` copy the `parent` reference as is. clone.parent = self.parent - clone.explicit = self.explicit + + # Both `copy()` and `clone_with()` copy the `raw_packet_cache` attribute as is. clone.raw_packet_cache = self.raw_packet_cache - clone.raw_packet_cache_fields = self.copy_fields_dict( - self.raw_packet_cache_fields - ) + + # Both `copy()` and `clone_with()` copy the `wirelen` attribute as is. clone.wirelen = self.wirelen - clone.post_transforms = self.post_transforms[:] - clone.payload = self.payload.copy() - clone.payload.add_underlayer(clone) + + # `copy()` and `clone_with()` have a slight different way to handle the `payload` attribute. + # Let's stick with initial implementations. + if for_clone_with: + if payload is not None: + clone.add_payload(payload) + else: + clone.payload = self.payload.copy() + clone.payload.add_underlayer(clone) + + # Both `copy()` and `clone_with()` copy the `time` attribute as is. clone.time = self.time + + # The `post_transforms` list is cloned in `copy()`, but set as is in `clone_with()`. + # Todo: Cloning the list seems safer, to be confirmed. + clone.post_transforms = self.post_transforms[:] + + # Both `copy()` and `clone_with()` copy the `comment` attribute as is. clone.comment = self.comment + + # Both `copy()` and `clone_with()` copy the `direction` attribute as is. clone.direction = self.direction + + # Both `copy()` and `clone_with()` copy the `sniffed_on` attribute as is. clone.sniffed_on = self.sniffed_on + return clone def _resolve_alias(self, attr): @@ -486,6 +627,21 @@ def __getattr__(self, attr): def setfieldval(self, attr, val): # type: (str, Any) -> None + + # Optimization: + # Try to avoid replacing a value by the same value, + # and avoid recursive cache invalidation by the way. + old_val = self.getfieldval(attr) # type: Any + if ( + # In case of packets, check the given packet reference differs from the previous one. + (val is old_val) if isinstance(val, Packet) + # In case of lists, let's take the new value whatever (no optimization). + else False if isinstance(val, list) + # In the general case, compare the values. + else (val == old_val) + ): + return + if self.deprecated_fields and attr in self.deprecated_fields: attr = self._resolve_alias(attr) if attr in self.default_fields: @@ -497,8 +653,8 @@ def setfieldval(self, attr, val): self.fields[attr] = val if isinstance(val, RawVal) else \ any2i(self, val) self.explicit = 0 - self.raw_packet_cache = None - self.raw_packet_cache_fields = None + # Invalidate cache when the packet has changed. + self.clear_cache() self.wirelen = None elif attr == "payload": self.remove_payload() @@ -521,8 +677,8 @@ def delfieldval(self, attr): if attr in self.fields: del self.fields[attr] self.explicit = 0 # in case a default value must be explicit - self.raw_packet_cache = None - self.raw_packet_cache_fields = None + # Invalidate cache when the packet has changed. + self.clear_cache() self.wirelen = None elif attr in self.default_fields: pass @@ -653,61 +809,41 @@ def copy_field_value(self, fieldname, value): # type: (str, Any) -> Any return self.get_field(fieldname).do_copy(value) - def copy_fields_dict(self, fields): - # type: (_T) -> _T - if fields is None: - return None - return {fname: self.copy_field_value(fname, fval) - for fname, fval in fields.items()} - - def _raw_packet_cache_field_value(self, fld, val, copy=False): - # type: (AnyField, Any, bool) -> Optional[Any] - """Get a value representative of a mutable field to detect changes""" - _cpy = lambda x: fld.do_copy(x) if copy else x # type: Callable[[Any], Any] - if fld.holds_packets: - # avoid copying whole packets (perf: #GH3894) - if fld.islist: - return [ - (_cpy(x.fields), x.payload.raw_packet_cache) for x in val - ] - else: - return (_cpy(val.fields), val.payload.raw_packet_cache) - elif fld.islist or fld.ismutable: - return _cpy(val) - return None - def clear_cache(self): # type: () -> None - """Clear the raw packet cache for the field and all its subfields""" - self.raw_packet_cache = None - for fname, fval in self.fields.items(): - fld = self.get_field(fname) - if fld.holds_packets: - if isinstance(fval, Packet): - fval.clear_cache() - elif isinstance(fval, list): - for fsubval in fval: - fsubval.clear_cache() - self.payload.clear_cache() + """ + Ensure cache invalidation for all: + + - parent packet if any, + - underlayer if any. + + .. note:: + Contrary to base former implementation, don't invalidate cache for: + + - packet fields if any, + - payload if any. + + .. todo:: + Shall we restore a default behaviour to avoid breaking the API: + "Clear the raw packet cache for the field and all its subfields"? + """ + def _clear_cache_ascending(pkt): # type: (Packet) -> None + pkt.raw_packet_cache = None + + if isinstance(pkt, Packet) and pkt.parent: + _clear_cache_ascending(pkt.parent) + if pkt.underlayer: + _clear_cache_ascending(pkt.underlayer) + + _clear_cache_ascending(self) def self_build(self): # type: () -> bytes """ Create the default layer regarding fields_desc dict - - :param field_pos_list: """ - if self.raw_packet_cache is not None and \ - self.raw_packet_cache_fields is not None: - for fname, fval in self.raw_packet_cache_fields.items(): - fld, val = self.getfield_and_val(fname) - if self._raw_packet_cache_field_value(fld, val) != fval: - self.raw_packet_cache = None - self.raw_packet_cache_fields = None - self.wirelen = None - break - if self.raw_packet_cache is not None: - return self.raw_packet_cache + if self.raw_packet_cache is not None: + return self.raw_packet_cache p = b"" for f in self.fields_desc: val = self.getfieldval(f.name) @@ -725,6 +861,17 @@ def self_build(self): except (AttributeError, IndexError): pass raise ex + + # Optimization: + # Once a packet has been built, save the result for the current layer in `raw_packet_cache`, + # except for *no-cache* packets. + if self.no_cache: + self.raw_packet_cache = None + else: + self.raw_packet_cache = p + # Once a packet has been built, and cache saved, flag as `explicit`. + self.explicit = 1 + return p def do_build_payload(self): @@ -734,6 +881,9 @@ def do_build_payload(self): :return: a string of payload layer """ + # Optimization: Flag the payload as explicit to avoid creating new instances for serialization. + self.payload.explicit = 1 + return self.payload.do_build() def do_build(self): @@ -765,6 +915,9 @@ def build(self): :return: string of the packet with the payload """ + # Flag as explicit to avoid creating new instances for serialization. + self.explicit = 1 + p = self.do_build() p += self.build_padding() p = self.build_done(p) @@ -1017,25 +1170,22 @@ def pre_dissect(self, s): def do_dissect(self, s): # type: (bytes) -> bytes _raw = s - self.raw_packet_cache_fields = {} for f in self.fields_desc: s, fval = f.getfield(self, s) # Skip unused ConditionalField if isinstance(f, ConditionalField) and fval is None: continue - # We need to track fields with mutable values to discard - # .raw_packet_cache when needed. - if (f.islist or f.holds_packets or f.ismutable) and fval is not None: - self.raw_packet_cache_fields[f.name] = \ - self._raw_packet_cache_field_value(f, fval, copy=True) self.fields[f.name] = fval # Nothing left to dissect if not s and (isinstance(f, MayEnd) or (fval is not None and isinstance(f, ConditionalField) and isinstance(f.fld, MayEnd))): break - self.raw_packet_cache = _raw[:-len(s)] if s else _raw - self.explicit = 1 + if self.no_cache: + self.raw_packet_cache = None + else: + self.raw_packet_cache = _raw[:-len(s)] if s else _raw + self.explicit = 1 return s def do_dissect_payload(self, s): @@ -1131,29 +1281,6 @@ def hide_defaults(self): del self.fields[k] self.payload.hide_defaults() - def clone_with(self, payload=None, **kargs): - # type: (Optional[Any], **Any) -> Any - pkt = self.__class__() - pkt.explicit = 1 - pkt.fields = kargs - pkt.default_fields = self.copy_fields_dict(self.default_fields) - pkt.overloaded_fields = self.overloaded_fields.copy() - pkt.time = self.time - pkt.underlayer = self.underlayer - pkt.parent = self.parent - pkt.post_transforms = self.post_transforms - pkt.raw_packet_cache = self.raw_packet_cache - pkt.raw_packet_cache_fields = self.copy_fields_dict( - self.raw_packet_cache_fields - ) - pkt.wirelen = self.wirelen - pkt.comment = self.comment - pkt.sniffed_on = self.sniffed_on - pkt.direction = self.direction - if payload is not None: - pkt.add_payload(payload) - return pkt - def __iter__(self): # type: () -> Iterator[Packet] """Iterates through all sub-packets generated by this Packet.""" From debbbdeaf87b6f65f009151ce4ae7a2968a1df69 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 7 Apr 2025 08:07:37 +0200 Subject: [PATCH 09/14] Avoid `add_payload()` clearing the cache when dissecting (#4706) - `clear_cache` parameter added to `add_payload()`. - `True` by default. Set to `False` when `add_payload()` called from `do_dissect_payload()`. --- scapy/packet.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index ac4ef8986c4..51a1b1dac94 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -379,12 +379,12 @@ def get_field(self, fld): """DEV: returns the field instance from the name of the field""" return self.fieldtype[fld] - def add_payload(self, payload): - # type: (Union[Packet, bytes]) -> None + def add_payload(self, payload, clear_cache=True): + # type: (Union[Packet, bytes], bool) -> None if payload is None: return elif not isinstance(self.payload, NoPayload): - self.payload.add_payload(payload) + self.payload.add_payload(payload, clear_cache=clear_cache) else: if isinstance(payload, Packet): self.payload = payload @@ -399,7 +399,8 @@ def add_payload(self, payload): raise TypeError("payload must be 'Packet', 'bytes', 'str', 'bytearray', or 'memoryview', not [%s]" % repr(payload)) # noqa: E501 # Invalidate cache when the packet has changed. - self.clear_cache() + if clear_cache: + self.clear_cache() def remove_payload(self): # type: () -> None @@ -1202,7 +1203,7 @@ def do_dissect_payload(self, s): ): # stop dissection here p = conf.raw_layer(s, _internal=1, _underlayer=self) - self.add_payload(p) + self.add_payload(p, clear_cache=False) return cls = self.guess_payload_class(s) try: @@ -1225,7 +1226,7 @@ def do_dissect_payload(self, s): if cls is not None: raise p = conf.raw_layer(s, _internal=1, _underlayer=self) - self.add_payload(p) + self.add_payload(p, clear_cache=False) def dissect(self, s): # type: (bytes) -> None From 35877da5b8f56940c4857a808d0db49bf8a5145d Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 7 Apr 2025 23:27:20 +0200 Subject: [PATCH 10/14] Restore default `clear_cache()` behaviour (#4707) - `upwards` (`False` by default) and `downwards` (`True` by default) parameters added to `clear_cache()`. - Set to `upwards=True` and `downwards=False` in `setfieldval()`, `delfieldval()`, `add_payload()` and `remove_payload()`. - Former implementation restored for `downwards=True`. --- scapy/packet.py | 54 ++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index 51a1b1dac94..a452fb20fbf 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -400,7 +400,7 @@ def add_payload(self, payload, clear_cache=True): # Invalidate cache when the packet has changed. if clear_cache: - self.clear_cache() + self.clear_cache(upwards=True, downwards=False) def remove_payload(self): # type: () -> None @@ -409,7 +409,7 @@ def remove_payload(self): self.overloaded_fields = {} # Invalidate cache when the packet has changed. - self.clear_cache() + self.clear_cache(upwards=True, downwards=False) def add_underlayer(self, underlayer): # type: (Packet) -> None @@ -655,7 +655,7 @@ def setfieldval(self, attr, val): any2i(self, val) self.explicit = 0 # Invalidate cache when the packet has changed. - self.clear_cache() + self.clear_cache(upwards=True, downwards=False) self.wirelen = None elif attr == "payload": self.remove_payload() @@ -679,7 +679,7 @@ def delfieldval(self, attr): del self.fields[attr] self.explicit = 0 # in case a default value must be explicit # Invalidate cache when the packet has changed. - self.clear_cache() + self.clear_cache(upwards=True, downwards=False) self.wirelen = None elif attr in self.default_fields: pass @@ -810,33 +810,33 @@ def copy_field_value(self, fieldname, value): # type: (str, Any) -> Any return self.get_field(fieldname).do_copy(value) - def clear_cache(self): - # type: () -> None + def clear_cache(self, upwards=False, downwards=True): + # type: (bool, bool) -> None """ - Ensure cache invalidation for all: - - - parent packet if any, - - underlayer if any. + Clear the raw packet cache for the current packet. - .. note:: - Contrary to base former implementation, don't invalidate cache for: + ``upwards`` and ``downwards`` indicate how this call should recurse in the packet tree. - - packet fields if any, - - payload if any. - - .. todo:: - Shall we restore a default behaviour to avoid breaking the API: - "Clear the raw packet cache for the field and all its subfields"? + :param upwards: Set to ``True`` to clear cache recursively over parent and underlayer packets. ``False`` by default. + :param downwards: Set to ``True`` (default) to clear cache recursively over subfields and payload. """ - def _clear_cache_ascending(pkt): # type: (Packet) -> None - pkt.raw_packet_cache = None - - if isinstance(pkt, Packet) and pkt.parent: - _clear_cache_ascending(pkt.parent) - if pkt.underlayer: - _clear_cache_ascending(pkt.underlayer) - - _clear_cache_ascending(self) + self.raw_packet_cache = None + + if upwards: + if self.parent: + self.parent.clear_cache(upwards=True, downwards=False) + if self.underlayer: + self.underlayer.clear_cache(upwards=True, downwards=False) + if downwards: + for fname, fval in self.fields.items(): + fld = self.get_field(fname) + if fld.holds_packets: + if isinstance(fval, Packet): + fval.clear_cache(upwards=False, downwards=True) + elif isinstance(fval, list): + for fsubval in fval: + fsubval.clear_cache(upwards=False, downwards=True) + self.payload.clear_cache(upwards=False, downwards=True) def self_build(self): # type: () -> bytes From d6e0cde8eca8c716c5dc1cda0b14e26541d3d909 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 18 Apr 2025 14:04:13 +0200 Subject: [PATCH 11/14] Revert `_PacketField.m2i()` changes (#4705) Useless now that `Packet.init_fields()` has a `for_dissect_only` parameter. --- scapy/fields.py | 33 +++++---------------------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/scapy/fields.py b/scapy/fields.py index e075faef7c8..06d73225589 100644 --- a/scapy/fields.py +++ b/scapy/fields.py @@ -1554,34 +1554,11 @@ def i2m(self, def m2i(self, pkt, m): # type: ignore # type: (Optional[Packet], bytes) -> Packet - # Do not import at module level on runtime ! (import loop) => local import - from scapy.packet import Packet - - # Build a default instance. - if isinstance(self.cls, type): - # Instantiation from packet class. - if TYPE_CHECKING: - assert issubclass(self.cls, Packet) - # Pre-set all default references with `None` values in order to avoid useless copies of default instances. - # They will be parsed after with the `dissect()` call below. - init_fields = { - field_name: None - for field_name in Packet.class_default_fields_ref.get(self.cls, {}) - } # type: Dict[str, None] - new_pkt = self.cls(**init_fields) # type: Packet - else: - # Instantiation from callback. - new_pkt = self.cls(b'') - - # we want to set parent wherever possible - # (only if `pkt` and `new_pkt` are both packets). - if isinstance(pkt, Packet) and isinstance(new_pkt, Packet): - new_pkt.parent = pkt - - # Eventually dissect the buffer to parse. - new_pkt.dissect(m) - - return new_pkt + try: + # we want to set parent wherever possible + return self.cls(m, _parent=pkt) # type: ignore + except TypeError: + return self.cls(m) class _PacketFieldSingle(_PacketField[K]): From ead2e516212c2f29fac6547ed88eea07a61d75f0 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 23 Apr 2025 19:01:28 +0200 Subject: [PATCH 12/14] Revert `clear_cache()` for payload modifications (#4706) In `Packet.add_payload()` and `remove_payload()`. --- scapy/packet.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index a452fb20fbf..fbd55618077 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -181,6 +181,7 @@ def __init__(self, # Clarify the meaning for this `explicit` flag. # Now that cache is cleared as soon as the packet is modified, possibly we could get rid of it? self.explicit = 0 + #: Caches bytes after `dissect()` or `build()` for the current layer only (not the payload, even if set). self.raw_packet_cache = None # type: Optional[bytes] self.wirelen = None # type: Optional[int] self.direction = None # type: Optional[int] @@ -379,12 +380,12 @@ def get_field(self, fld): """DEV: returns the field instance from the name of the field""" return self.fieldtype[fld] - def add_payload(self, payload, clear_cache=True): - # type: (Union[Packet, bytes], bool) -> None + def add_payload(self, payload): + # type: (Union[Packet, bytes]) -> None if payload is None: return elif not isinstance(self.payload, NoPayload): - self.payload.add_payload(payload, clear_cache=clear_cache) + self.payload.add_payload(payload) else: if isinstance(payload, Packet): self.payload = payload @@ -398,19 +399,12 @@ def add_payload(self, payload, clear_cache=True): else: raise TypeError("payload must be 'Packet', 'bytes', 'str', 'bytearray', or 'memoryview', not [%s]" % repr(payload)) # noqa: E501 - # Invalidate cache when the packet has changed. - if clear_cache: - self.clear_cache(upwards=True, downwards=False) - def remove_payload(self): # type: () -> None self.payload.remove_underlayer(self) self.payload = NoPayload() self.overloaded_fields = {} - # Invalidate cache when the packet has changed. - self.clear_cache(upwards=True, downwards=False) - def add_underlayer(self, underlayer): # type: (Packet) -> None self.underlayer = underlayer @@ -1203,7 +1197,7 @@ def do_dissect_payload(self, s): ): # stop dissection here p = conf.raw_layer(s, _internal=1, _underlayer=self) - self.add_payload(p, clear_cache=False) + self.add_payload(p) return cls = self.guess_payload_class(s) try: @@ -1226,7 +1220,7 @@ def do_dissect_payload(self, s): if cls is not None: raise p = conf.raw_layer(s, _internal=1, _underlayer=self) - self.add_payload(p, clear_cache=False) + self.add_payload(p) def dissect(self, s): # type: (bytes) -> None From e7465c8522391117ce78c2b00fd2722a2231b7ad Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 23 Apr 2025 19:07:55 +0200 Subject: [PATCH 13/14] Ensure cache set for good at the end of `Packet._fast_copy()` (#4705) --- scapy/packet.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index fbd55618077..5a0056838f5 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -543,9 +543,6 @@ def _fast_copy_fields_dict( # Both `copy()` and `clone_with()` copy the `parent` reference as is. clone.parent = self.parent - # Both `copy()` and `clone_with()` copy the `raw_packet_cache` attribute as is. - clone.raw_packet_cache = self.raw_packet_cache - # Both `copy()` and `clone_with()` copy the `wirelen` attribute as is. clone.wirelen = self.wirelen @@ -574,6 +571,10 @@ def _fast_copy_fields_dict( # Both `copy()` and `clone_with()` copy the `sniffed_on` attribute as is. clone.sniffed_on = self.sniffed_on + # Eventually set cache. + # Both `copy()` and `clone_with()` copy the `raw_packet_cache` attribute as is. + clone.raw_packet_cache = self.raw_packet_cache + return clone def _resolve_alias(self, attr): From 33aeae4d03a5fa0b0080ee45895b25eb359d6cb9 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 23 Apr 2025 19:12:12 +0200 Subject: [PATCH 14/14] Fix parent not set when building default packet fields (#4706, #4707) - `Packet._ensure_parent_of()` added. - Called in `do_init_cached_fields()`. --- scapy/packet.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scapy/packet.py b/scapy/packet.py index 5a0056838f5..6f1dc9d8d66 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -319,6 +319,7 @@ def do_init_cached_fields(self, for_dissect_only=False, init_fields=None): # Fix: Use `copy_field_value()` instead of just `value.copy()`, in order to duplicate list items as well in case of a list. self.fields[fname] = self.copy_field_value(fname, self.default_fields[fname]) + self._ensure_parent_of(self.fields[fname]) def prepare_cached_fields(self, flist): # type: (Sequence[AnyField]) -> None @@ -427,6 +428,15 @@ def remove_parent(self, other): point to the list owner packet.""" self.parent = None + def _ensure_parent_of(self, val): + # type: (Any) -> None + """Ensures a parent reference with self for val when applicable.""" + if isinstance(val, Packet): + val.parent = self + elif isinstance(val, list): + for item in val: # type: Any + self._ensure_parent_of(item) + def copy(self) -> Self: """Returns a deep copy of the instance.""" return self._fast_copy(for_clone_with=False)