From 9db46b8363b80639022f118f6b2a67151f083c8b Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 21 Mar 2025 12:21:35 +0100 Subject: [PATCH 01/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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/35] 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) From 0976a60b1e67e2e430f82aee4635cfb575a8a809 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 27 Jun 2025 09:41:47 +0200 Subject: [PATCH 15/35] Fix Thales copyrights (#4705, #4706, #4707) --- scapy/packet.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scapy/packet.py b/scapy/packet.py index 6f1dc9d8d66..e805ef34544 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -2,6 +2,7 @@ # This file is part of Scapy # See https://scapy.net/ for more information # Copyright (C) Philippe Biondi +# Copyright (C) 2025 Thales """ Packet class From 7fed967df2ed6cbf87545f9fc65f2c1623c50c02 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 16 Jul 2025 16:59:44 +0200 Subject: [PATCH 16/35] Fix `post_build()` not being called anymore (#4705) ...due to `raw_packet_cache` being set in `self_build()`. --- scapy/packet.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index e805ef34544..dd4973c7c5e 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -902,12 +902,34 @@ def do_build(self): """ if not self.explicit: self = next(iter(self)) + + # First of all, save whether `post_build()` should be called after `self_build()`. + # For the memo, `self_build()` norammly sets the `raw_packet_cache` field. + do_post_build = (self.raw_packet_cache is None) + pkt = self.self_build() for t in self.post_transforms: pkt = t(pkt) pay = self.do_build_payload() - if self.raw_packet_cache is None: - return self.post_build(pkt, pay) + if do_post_build: + pkt_pay = self.post_build(pkt, pay) + + # If set, update `raw_packet_cache` after `post_build()` has been called. + if self.raw_packet_cache is not None: + # Note: + # The `post_build()` API does not let us know with certainty + # what part of the returned value actually corresponds to the current layer, + # and what part corresponds to the payload. + # + # Check the total size of current layer + payload has not changed. + # If lengths have not changed, take the first bytes to update the `raw_packet_cache` field. + # If lengths have changed, forget the `raw_packet_cache` optimization. + if len(pkt_pay) == len(pkt) + len(pay): + self.raw_packet_cache = pkt_pay[:len(self.raw_packet_cache)] + else: + self.raw_packet_cache = None + + return pkt_pay else: return pkt + pay From 625154c5c8f4e5662a5d37624dd2b83e0749b9a2 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 16 Jul 2025 17:29:10 +0200 Subject: [PATCH 17/35] Fix `NoPayload.clear_cache()` signature (#4707) Make it comply with `Packet.clear_cache()`. --- scapy/packet.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index dd4973c7c5e..76fe57688b2 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -1992,8 +1992,8 @@ def copy(self): # type: () -> NoPayload return self - def clear_cache(self): - # type: () -> None + def clear_cache(self, upwards=False, downwards=True): + # type: (bool, bool) -> None pass def __repr__(self): From 4583af86bde743c83a21c2c1bbae0953aaeff931 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 16 Jul 2025 20:10:29 +0200 Subject: [PATCH 18/35] Fix `setfieldval()` when the field is not set yet (#4705) --- scapy/packet.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index 76fe57688b2..6add8f62657 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -638,16 +638,22 @@ def setfieldval(self, attr, val): # 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 + try: + 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 + except AttributeError: + # Field name can't be found (yet?). + # Let the execution go on, especially on the payload. + # An `AttributeError` may eventually be raised in case of a `NoPayload`. + pass if self.deprecated_fields and attr in self.deprecated_fields: attr = self._resolve_alias(attr) From 7f0f095230109d3dc568714ade5dd181e3e7ca30 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 18 Jul 2025 09:02:06 +0200 Subject: [PATCH 19/35] Add `clear_cache()` calls in 'test/fields.uts' (#4705) --- test/fields.uts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/fields.uts b/test/fields.uts index b64300b8279..bd79df89c49 100644 --- a/test/fields.uts +++ b/test/fields.uts @@ -616,6 +616,7 @@ assert b[TestPkt:2].f1 == 100 assert Raw in b and b[Raw].load == b"123456" a.plist.append(TestPkt(f1=200)) +a.clear_cache() # List fields, if modified, do not automatically invalidate the cache. We currently have to do it explicitly. b = TestPLF2(raw(a)) b.show() assert b.len1 == 5 and b.len2 == 5 @@ -713,6 +714,7 @@ assert b[TestPkt:2].f1 == 100 assert Raw in b and b[Raw].load == b"123456" a.plist.append(TestPkt(f1=200)) +a.clear_cache() # List fields, if modified, do not automatically invalidate the cache. We currently have to do it explicitly. b = TestPLF2(raw(a)) b.show() assert b.len1 == 5 and b.len2 == 5 From d4d66559794d19bf0e7cadc3263cd9f730895a18 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 18 Jul 2025 16:15:36 +0200 Subject: [PATCH 20/35] Fix cache issues with packet list fields (#4705) - Add `list_field` class with `list_field_meta`. - `list_field.ensure_bound()` calls added anywhere `Packet.fields` is updated with a list field. --- scapy/packet.py | 124 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 118 insertions(+), 6 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index 6add8f62657..817e52da64e 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -203,16 +203,28 @@ def __init__(self, value = fields.pop(fname) except KeyError: continue - self.fields[fname] = value if isinstance(value, RawVal) else \ - self.get_field(fname).any2i(self, value) + if not isinstance(value, RawVal): + value = self.get_field(fname).any2i(self, value) + + # In case of a list, ensure we store a `list_field` instance, not a simple `list`. + if isinstance(value, list): + value = list_field.ensure_bound(self, value) + + self.fields[fname] = value # The remaining fields are unknown for fname in fields: if fname in self.deprecated_fields: # Resolve deprecated fields value = fields[fname] fname = self._resolve_alias(fname) - self.fields[fname] = value if isinstance(value, RawVal) else \ - self.get_field(fname).any2i(self, value) + if not isinstance(value, RawVal): + value = self.get_field(fname).any2i(self, value) + + # In case of a list, ensure we store a `list_field` instance, not a simple `list`. + if isinstance(value, list): + value = list_field.ensure_bound(self, value) + + self.fields[fname] = value continue raise AttributeError(fname) if isinstance(post_transform, list): @@ -320,6 +332,11 @@ 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]) + + # In case of a list, ensure we store a `list_field` instance, not a simple `list`. + if isinstance(self.fields[fname], list): + self.fields[fname] = list_field.ensure_bound(self, self.fields[fname]) + self._ensure_parent_of(self.fields[fname]) def prepare_cached_fields(self, flist): @@ -663,8 +680,14 @@ def setfieldval(self, attr, val): any2i = lambda x, y: y # type: Callable[..., Any] else: any2i = fld.any2i - self.fields[attr] = val if isinstance(val, RawVal) else \ - any2i(self, val) + if not isinstance(val, RawVal): + val = any2i(self, val) + + # In case of a list, ensure we store a `list_field` instance, not a simple `list`. + if isinstance(val, list): + val = list_field.ensure_bound(self, val) + + self.fields[attr] = val self.explicit = 0 # Invalidate cache when the packet has changed. self.clear_cache(upwards=True, downwards=False) @@ -1210,6 +1233,11 @@ def do_dissect(self, s): # Skip unused ConditionalField if isinstance(f, ConditionalField) and fval is None: continue + + # In case of a list, ensure we store a `list_field` instance, not a simple `list`. + if isinstance(fval, list): + fval = list_field.ensure_bound(self, fval) + self.fields[f.name] = fval # Nothing left to dissect if not s and (isinstance(f, MayEnd) or @@ -2133,6 +2161,90 @@ def route(self): return (None, None, None) +################# +# list fields # +################# + + +class list_field_meta(type): + """ + Wraps modifying methods for ``list`` base type. + + Inspired from https://stackoverflow.com/questions/8858525/track-changes-to-lists-and-dictionaries-in-python#8859168. + """ + def __new__( + mcs, + name, # type: str + bases, # Tuple[type, ...] + attrs, # type: Dict[str, Any] + ): # type: (...) -> type + # List names of `list` methods modifying the list. + for method_name in [ + "append", + "clear", + "extend", + "insert", + "pop", + "remove", + "reverse", # Memo: Reverse *IN PLACE*. + "sort", # Memo: Stable sort *IN PLACE*. + "__delitem__", + "__iadd__", + "__imul__", + "__setitem__", + ]: + # Wrap the method so that `Packet.clear_cache()` be automatically called. + attrs[method_name] = list_field_meta._wrap_method(getattr(list, method_name)) + return type.__new__(mcs, name, bases, attrs) + + @staticmethod + def _wrap_method(meth): # type: (Callable[[Any, ...], Any]) -> Callable[[Any, ...], Any] + def wrapped( + self, # type: list_field + *args, # type: Any + **kwargs, # type: Any + ): # type: (...) -> Any + # Automatically call `Packet.clear_cache()` when the `list_field` is modified. + self.pkt.clear_cache(upwards=True, downwards=False) + + # Call the wrapped method, and return its result. + return meth(self, *args, **kwargs) + return wrapped + + +class list_field(list, metaclass=list_field_meta): + """ + Overrides the base ``list`` type for list fields bound with packets. + + Ensures :meth:`Packet.clear_cache()` is called when the list is modified. + + Lower case for the class name in order to avoid confusions with classes like ``PacketListField``. + """ + def __init__( + self, + pkt, # type: Packet, + *args # type: Any + ): # type: (...) -> None + # Call the `list.__init__()` super constructor. + super().__init__(*args) + + #: Packet bound with this list field. + self.pkt = pkt + + @staticmethod + def ensure_bound( + pkt, # type: Packet + lst, # type: List[Any] + ): # type: (...) -> list_field + """ + Ensures a :class:`list_field` instance bound with ``pkt``. + """ + # If `lst` is a simple `list`, this method intends to create a new `list_field` instance. + # If `lst` is already a `list_field` instance, we never know where this instance comes from, and what it's being used for. + # Let's create a new `list_field` instance in any case. + return list_field(pkt, lst) + + #################### # packet classes # #################### From bf0fa0e988ed4670da3015ce70eacc52cab93657 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 18 Jul 2025 16:17:06 +0200 Subject: [PATCH 21/35] Revert "Add `clear_cache()` calls in 'test/fields.uts' (#4705)" This reverts commit 7f0f095230109d3dc568714ade5dd181e3e7ca30. --- test/fields.uts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/fields.uts b/test/fields.uts index bd79df89c49..b64300b8279 100644 --- a/test/fields.uts +++ b/test/fields.uts @@ -616,7 +616,6 @@ assert b[TestPkt:2].f1 == 100 assert Raw in b and b[Raw].load == b"123456" a.plist.append(TestPkt(f1=200)) -a.clear_cache() # List fields, if modified, do not automatically invalidate the cache. We currently have to do it explicitly. b = TestPLF2(raw(a)) b.show() assert b.len1 == 5 and b.len2 == 5 @@ -714,7 +713,6 @@ assert b[TestPkt:2].f1 == 100 assert Raw in b and b[Raw].load == b"123456" a.plist.append(TestPkt(f1=200)) -a.clear_cache() # List fields, if modified, do not automatically invalidate the cache. We currently have to do it explicitly. b = TestPLF2(raw(a)) b.show() assert b.len1 == 5 and b.len2 == 5 From e32dab12b423a722d2d50c7d0753c985b96289b3 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 18 Jul 2025 17:06:30 +0200 Subject: [PATCH 22/35] Avoid caching for packets returned by `fuzz()` (#4705) --- scapy/packet.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scapy/packet.py b/scapy/packet.py index 817e52da64e..85b55edb46c 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -2939,4 +2939,9 @@ def fuzz(p, # type: _P new_default_fields[name] = rnd q.default_fields.update(new_default_fields) q = q.payload + + # Avoid caching for `p`. + p.no_cache = True + p.clear_cache() + return p From 23b1628e90c932981f8dd4913d4912b51759b547 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Fri, 18 Jul 2025 18:01:32 +0200 Subject: [PATCH 23/35] Adjust `MAX_TIME_PER_OP` in 'perf_packet_fields.uts' (#4705) Based on github ci experimentation. --- test/perf_packet_fields.uts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/perf_packet_fields.uts b/test/perf_packet_fields.uts index bb2e54b57cd..a10b6459132 100644 --- a/test/perf_packet_fields.uts +++ b/test/perf_packet_fields.uts @@ -16,7 +16,8 @@ NUMBER_OF_F_PER_I = 100 # - 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. +# - About 1.9s measured with optimizations on github ci windows-latest / Python 3.13 with NUMBER_OF_I_PER_M=100, NUMBER_OF_F_PER_I=100. +MAX_TIME_PER_OP = (2.0 / (100.0 * 100.0)) * 1.10 # 10% margin. class TestPacket(Packet): """Base class for `M`, `I` and `F`.""" From 5a1f68f1e85fcab33bad6b6d3d5c963507455723 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 21 Jul 2025 11:05:25 +0200 Subject: [PATCH 24/35] Avoid `fuzz()` messing up `default_fields` (#4705) --- scapy/packet.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scapy/packet.py b/scapy/packet.py index 85b55edb46c..340c61d34b9 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -2937,7 +2937,12 @@ def fuzz(p, # type: _P rnd = fld._find_fld_pkt(q).randval() if rnd is not None: new_default_fields[name] = rnd - q.default_fields.update(new_default_fields) + # Avoid messing up the original packet `default_fields` dictionary which is unique for all instances of the given layer. + # Build a new dictionary, and update it with `new_default_fields`. + q.default_fields = { + **q.default_fields, + **new_default_fields, + } q = q.payload # Avoid caching for `p`. From 471bf407050a2bbe0bd85901ac829ccd4e17f531 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 21 Jul 2025 12:44:50 +0200 Subject: [PATCH 25/35] Make `FlagValue` bound to a `Packet` (#4705) - `FlagValue.pkt` attribute added. - `list_field.ensure_bound()` generalized as `Packet._ensure_bound_field_value()` and completed for `FlagValue`s. - `Packet.clear_cache()` automatically in `FlagValue.__setattr__()`. --- scapy/fields.py | 12 ++++++++- scapy/packet.py | 67 ++++++++++++++++++++++--------------------------- 2 files changed, 41 insertions(+), 38 deletions(-) diff --git a/scapy/fields.py b/scapy/fields.py index 06d73225589..69ff08486ff 100644 --- a/scapy/fields.py +++ b/scapy/fields.py @@ -2973,7 +2973,7 @@ def __next__(self): class FlagValue(object): - __slots__ = ["value", "names", "multi"] + __slots__ = ["pkt", "value", "names", "multi"] def _fixvalue(self, value): # type: (Any) -> int @@ -2990,6 +2990,12 @@ def _fixvalue(self, value): def __init__(self, value, names): # type: (Union[List[str], int, str], Union[List[str], str]) -> None + + #: Packet bound with this flag value. + #: + #: Ensures ``Packet.clear_cache()`` is called when the flags are modified. + self.pkt = None # type: Optional[Packet] + self.multi = isinstance(names, list) self.names = names self.value = self._fixvalue(value) @@ -3125,6 +3131,10 @@ def __setattr__(self, attr, value): self.value |= (2 ** self.names.index(attr)) else: self.value &= ~(2 ** self.names.index(attr)) + + # Automatically call `Packet.clear_cache()` when the flags are modified. + if self.pkt is not None: + self.pkt.clear_cache(upwards=True, downwards=True) else: return super(FlagValue, self).__setattr__(attr, value) diff --git a/scapy/packet.py b/scapy/packet.py index 340c61d34b9..3036471aaed 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -205,11 +205,7 @@ def __init__(self, continue if not isinstance(value, RawVal): value = self.get_field(fname).any2i(self, value) - - # In case of a list, ensure we store a `list_field` instance, not a simple `list`. - if isinstance(value, list): - value = list_field.ensure_bound(self, value) - + value = self._ensure_bound_field_value(value) self.fields[fname] = value # The remaining fields are unknown for fname in fields: @@ -219,11 +215,7 @@ def __init__(self, fname = self._resolve_alias(fname) if not isinstance(value, RawVal): value = self.get_field(fname).any2i(self, value) - - # In case of a list, ensure we store a `list_field` instance, not a simple `list`. - if isinstance(value, list): - value = list_field.ensure_bound(self, value) - + value = self._ensure_bound_field_value(value) self.fields[fname] = value continue raise AttributeError(fname) @@ -332,10 +324,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]) - - # In case of a list, ensure we store a `list_field` instance, not a simple `list`. - if isinstance(self.fields[fname], list): - self.fields[fname] = list_field.ensure_bound(self, self.fields[fname]) + self.fields[fname] = self._ensure_bound_field_value(self.fields[fname]) self._ensure_parent_of(self.fields[fname]) @@ -682,11 +671,7 @@ def setfieldval(self, attr, val): any2i = fld.any2i if not isinstance(val, RawVal): val = any2i(self, val) - - # In case of a list, ensure we store a `list_field` instance, not a simple `list`. - if isinstance(val, list): - val = list_field.ensure_bound(self, val) - + val = self._ensure_bound_field_value(val) self.fields[attr] = val self.explicit = 0 # Invalidate cache when the packet has changed. @@ -708,6 +693,31 @@ def __setattr__(self, attr, val): pass return object.__setattr__(self, attr, val) + def _ensure_bound_field_value( + self, + value, # type: Any + ): # type: (...) -> Any + """ + Ensures a field instance bound with ``self`` when applicable. + """ + if isinstance(value, list): + # If `value` is a simple `list`, create a new `list_field` instance. + # If `value` is already a `list_field` instance, we never know where this instance comes from, and what it's being used for. + # Let's create a new `list_field` instance in any case. + return list_field(self, value) + + if isinstance(value, FlagValue): + # We never know where the `FlagValue` instance comes from, and what it's being used for. + # Let's create a new instance. + value = value.copy() + value.pkt = self + return value + + # Non-specific bound field. + # Maybe a packet? rely on parentship in that case. + # Return `value` as is. + return value + def delfieldval(self, attr): # type: (str) -> None if attr in self.fields: @@ -1233,11 +1243,7 @@ def do_dissect(self, s): # Skip unused ConditionalField if isinstance(f, ConditionalField) and fval is None: continue - - # In case of a list, ensure we store a `list_field` instance, not a simple `list`. - if isinstance(fval, list): - fval = list_field.ensure_bound(self, fval) - + fval = self._ensure_bound_field_value(fval) self.fields[f.name] = fval # Nothing left to dissect if not s and (isinstance(f, MayEnd) or @@ -2231,19 +2237,6 @@ def __init__( #: Packet bound with this list field. self.pkt = pkt - @staticmethod - def ensure_bound( - pkt, # type: Packet - lst, # type: List[Any] - ): # type: (...) -> list_field - """ - Ensures a :class:`list_field` instance bound with ``pkt``. - """ - # If `lst` is a simple `list`, this method intends to create a new `list_field` instance. - # If `lst` is already a `list_field` instance, we never know where this instance comes from, and what it's being used for. - # Let's create a new `list_field` instance in any case. - return list_field(pkt, lst) - #################### # packet classes # From 9415f58ac58ef14fb50e88846bd6a57b0e48aace Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 21 Jul 2025 12:55:56 +0200 Subject: [PATCH 26/35] Make `Packet._ensure_bound_field_value()` recursive on lists (#4705) --- scapy/packet.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scapy/packet.py b/scapy/packet.py index 3036471aaed..b242155ebbf 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -704,7 +704,11 @@ def _ensure_bound_field_value( # If `value` is a simple `list`, create a new `list_field` instance. # If `value` is already a `list_field` instance, we never know where this instance comes from, and what it's being used for. # Let's create a new `list_field` instance in any case. - return list_field(self, value) + return list_field( + self, + # Recurse on list items. + [self._ensure_bound_field_value(x) for x in value], + ) if isinstance(value, FlagValue): # We never know where the `FlagValue` instance comes from, and what it's being used for. From 2fec7034eec6df877a52f4deb61f0cd4c8575c35 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Mon, 21 Jul 2025 13:09:05 +0200 Subject: [PATCH 27/35] Rework `list_field` as `ListValue` in 'fields.py' (#4705) More consistent with `FlagValue`. --- scapy/fields.py | 64 +++++++++++++++++++++++++++++++++++++++ scapy/packet.py | 80 ++++--------------------------------------------- 2 files changed, 69 insertions(+), 75 deletions(-) diff --git a/scapy/fields.py b/scapy/fields.py index 69ff08486ff..230ab5e6b12 100644 --- a/scapy/fields.py +++ b/scapy/fields.py @@ -1619,6 +1619,70 @@ def getfield(self, return s[len_pkt:], i +class _ListValueMeta(type): + """ + Wraps modifying methods for ``list`` base type. + + Inspired from https://stackoverflow.com/questions/8858525/track-changes-to-lists-and-dictionaries-in-python#8859168. + """ + def __new__( + mcs, + name, # type: str + bases, # Tuple[type, ...] + attrs, # type: Dict[str, Any] + ): # type: (...) -> type + # List names of `list` methods modifying the list. + for method_name in [ + "append", + "clear", + "extend", + "insert", + "pop", + "remove", + "reverse", # Memo: Reverse *IN PLACE*. + "sort", # Memo: Stable sort *IN PLACE*. + "__delitem__", + "__iadd__", + "__imul__", + "__setitem__", + ]: + # Wrap the method so that `Packet.clear_cache()` be automatically called. + attrs[method_name] = _ListValueMeta._wrap_method(getattr(list, method_name)) + return type.__new__(mcs, name, bases, attrs) + + @staticmethod + def _wrap_method(meth): # type: (Callable[[Any, ...], Any]) -> Callable[[Any, ...], Any] + def wrapped( + self, # type: ListValue + *args, # type: Any + **kwargs, # type: Any + ): # type: (...) -> Any + # Automatically call `Packet.clear_cache()` when the `ListValue` is modified. + self.pkt.clear_cache(upwards=True, downwards=False) + + # Call the wrapped method, and return its result. + return meth(self, *args, **kwargs) + return wrapped + + +class ListValue(list, metaclass=_ListValueMeta): + """ + Overrides the base ``list`` type for list fields bound with packets. + + Ensures ``Packet.clear_cache()`` is called when the list is modified. + """ + def __init__( + self, + pkt, # type: Packet, + *args # type: Any + ): # type: (...) -> None + # Call the `list.__init__()` super constructor. + super().__init__(*args) + + #: Packet bound with this list field. + self.pkt = pkt + + class PacketListField(_PacketField[List[BasePacket]]): """PacketListField represents a list containing a series of Packet instances that might occur right in the middle of another Packet field. diff --git a/scapy/packet.py b/scapy/packet.py index b242155ebbf..213f2af0ece 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -33,6 +33,7 @@ Field, FlagsField, FlagValue, + ListValue, MayEnd, MultiEnumField, MultipleTypeField, @@ -701,10 +702,10 @@ def _ensure_bound_field_value( Ensures a field instance bound with ``self`` when applicable. """ if isinstance(value, list): - # If `value` is a simple `list`, create a new `list_field` instance. - # If `value` is already a `list_field` instance, we never know where this instance comes from, and what it's being used for. - # Let's create a new `list_field` instance in any case. - return list_field( + # If `value` is a simple `list`, create a new `ListValue` instance. + # If `value` is already a `ListValue` instance, we never know where this instance comes from, and what it's being used for. + # Let's create a new `ListValue` instance in any case. + return ListValue( self, # Recurse on list items. [self._ensure_bound_field_value(x) for x in value], @@ -2171,77 +2172,6 @@ def route(self): return (None, None, None) -################# -# list fields # -################# - - -class list_field_meta(type): - """ - Wraps modifying methods for ``list`` base type. - - Inspired from https://stackoverflow.com/questions/8858525/track-changes-to-lists-and-dictionaries-in-python#8859168. - """ - def __new__( - mcs, - name, # type: str - bases, # Tuple[type, ...] - attrs, # type: Dict[str, Any] - ): # type: (...) -> type - # List names of `list` methods modifying the list. - for method_name in [ - "append", - "clear", - "extend", - "insert", - "pop", - "remove", - "reverse", # Memo: Reverse *IN PLACE*. - "sort", # Memo: Stable sort *IN PLACE*. - "__delitem__", - "__iadd__", - "__imul__", - "__setitem__", - ]: - # Wrap the method so that `Packet.clear_cache()` be automatically called. - attrs[method_name] = list_field_meta._wrap_method(getattr(list, method_name)) - return type.__new__(mcs, name, bases, attrs) - - @staticmethod - def _wrap_method(meth): # type: (Callable[[Any, ...], Any]) -> Callable[[Any, ...], Any] - def wrapped( - self, # type: list_field - *args, # type: Any - **kwargs, # type: Any - ): # type: (...) -> Any - # Automatically call `Packet.clear_cache()` when the `list_field` is modified. - self.pkt.clear_cache(upwards=True, downwards=False) - - # Call the wrapped method, and return its result. - return meth(self, *args, **kwargs) - return wrapped - - -class list_field(list, metaclass=list_field_meta): - """ - Overrides the base ``list`` type for list fields bound with packets. - - Ensures :meth:`Packet.clear_cache()` is called when the list is modified. - - Lower case for the class name in order to avoid confusions with classes like ``PacketListField``. - """ - def __init__( - self, - pkt, # type: Packet, - *args # type: Any - ): # type: (...) -> None - # Call the `list.__init__()` super constructor. - super().__init__(*args) - - #: Packet bound with this list field. - self.pkt = pkt - - #################### # packet classes # #################### From 9c26a329a4ebf2672cb841e07c32cf2d0ca9efeb Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Tue, 22 Jul 2025 11:35:26 +0200 Subject: [PATCH 28/35] Fix `Packet.setfieldval()` when `attr` not already in `self.fields` (#4705) --- scapy/packet.py | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/scapy/packet.py b/scapy/packet.py index 213f2af0ece..6422c87259f 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -643,24 +643,26 @@ def setfieldval(self, attr, val): # type: (str, Any) -> None # Optimization: - # Try to avoid replacing a value by the same value, + # If `attr` is already in `self.fields`, + # try to avoid replacing a value by the same value, # and avoid recursive cache invalidation by the way. - try: - 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 - except AttributeError: - # Field name can't be found (yet?). - # Let the execution go on, especially on the payload. - # An `AttributeError` may eventually be raised in case of a `NoPayload`. - pass + if attr in self.fields: + try: + 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 + except AttributeError: + # Field name can't be found (yet?). + # Let the execution go on, especially on the payload. + # An `AttributeError` may eventually be raised in case of a `NoPayload`. + pass if self.deprecated_fields and attr in self.deprecated_fields: attr = self._resolve_alias(attr) From 000e16bc299880129cb05e1bcd07b3d25bc5a66a Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 23 Jul 2025 16:45:06 +0200 Subject: [PATCH 29/35] Copy the `no_cache` flag in `Packet._fast_copy()` (#4705) --- scapy/packet.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scapy/packet.py b/scapy/packet.py index 6422c87259f..1e22e112f0f 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -528,6 +528,10 @@ def _fast_copy_fields_dict( clone.fields = _fast_copy_fields_dict(self.fields) clone.explicit = self.explicit + # Copy the `no_cache` flag if set on the source packet, and not already set on the clone. + if self.no_cache and (not clone.no_cache): + clone.no_cache = True + # Both `copy()` and `clone_with()` set the `default_fields` attribute with the `copy_fields_dict()` method. # # Memo: From 46f12db76b8a91358f3f65e89f561aeb54848262 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 23 Jul 2025 16:46:02 +0200 Subject: [PATCH 30/35] Fix unit test (#4705) 'regression.uts' / "Sending and receiving an TCP syn with flooding methods" --- test/regression.uts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/regression.uts b/test/regression.uts index bddaf06315d..d3b56c2bc55 100644 --- a/test/regression.uts +++ b/test/regression.uts @@ -1802,6 +1802,7 @@ from functools import partial def _test_flood(ip, flood_function, add_ether=False): with no_debug_dissector(): p = IP(dst=ip)/TCP(sport=RandShort(), dport=80, flags="S") + p[TCP].no_cache = True # Disable caching for the TCP layer, since `sport` is set with a random field. if add_ether: p = Ether()/p p.show2() From d64e5748ebf78266fa9f4b1bdc0931e3ea42c737 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Wed, 23 Jul 2025 17:43:12 +0200 Subject: [PATCH 31/35] Avoid caching with `BTLE` layer (#4705) --- scapy/layers/bluetooth4LE.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scapy/layers/bluetooth4LE.py b/scapy/layers/bluetooth4LE.py index 0611d3edd26..37286f5ed22 100644 --- a/scapy/layers/bluetooth4LE.py +++ b/scapy/layers/bluetooth4LE.py @@ -214,6 +214,9 @@ class BTLE(Packet): X3BytesField("crc", None) ] + # Avoid caching with this layer for data reordering to work properly. + no_cache = True + @staticmethod def compute_crc(pdu, init=0x555555): def swapbits(a): From 8b3eb9d903a35904f3544fc8723beb544ebfc995 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Thu, 24 Jul 2025 05:45:55 +0200 Subject: [PATCH 32/35] Hide `ValueError`s in `Packet.setfieldval()` optimization (#4705) --- scapy/packet.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scapy/packet.py b/scapy/packet.py index 1e22e112f0f..5ab9187b837 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -667,6 +667,10 @@ def setfieldval(self, attr, val): # Let the execution go on, especially on the payload. # An `AttributeError` may eventually be raised in case of a `NoPayload`. pass + except ValueError: + # Value comparisons may fail (may occur with `FlagValue`s among others). + # Forget the optimization in such case. + pass if self.deprecated_fields and attr in self.deprecated_fields: attr = self._resolve_alias(attr) From cb146b8dc0baec982083a99f1e6ed88a490fc730 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Thu, 18 Sep 2025 00:54:54 +0200 Subject: [PATCH 33/35] Fix `_NTLMPayloadField` and `StrFieldUtf16.any2i()` (#4705) With `bytes` for input. Otherwise, `any2i()` calls in `Packet.__init__()` doubles UTF16 encoding. This problem has been revealed with the usage of the new `_init_fields()` return value for the initialization of the clone in `Packet._fast_copy()`. --- scapy/fields.py | 2 ++ scapy/layers/ntlm.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/scapy/fields.py b/scapy/fields.py index 230ab5e6b12..16f34debae6 100644 --- a/scapy/fields.py +++ b/scapy/fields.py @@ -1478,6 +1478,8 @@ class StrField(_StrField[bytes]): class StrFieldUtf16(StrField): def any2i(self, pkt, x): # type: (Optional[Packet], Optional[str]) -> bytes + if isinstance(x, bytes): + return x if isinstance(x, str): return self.h2i(pkt, x) return super(StrFieldUtf16, self).any2i(pkt, x) diff --git a/scapy/layers/ntlm.py b/scapy/layers/ntlm.py index efabef3cc8d..283befd9c0e 100644 --- a/scapy/layers/ntlm.py +++ b/scapy/layers/ntlm.py @@ -152,6 +152,10 @@ def h2i(self, pkt, x): # type: (Optional[Packet], bytes) -> List[Tuple[str, str]] return self._on_payload(pkt, x, "h2i") + def any2i(self, pkt, x): + # type: (Optional[Packet], bytes) -> List[Tuple[str, str]] + return self._on_payload(pkt, x, "any2i") + def i2repr(self, pkt, x): # type: (Optional[Packet], bytes) -> str return repr(self._on_payload(pkt, x, "i2repr")) From 793fd2965ab89f27e261833e6ca16f3683ee1994 Mon Sep 17 00:00:00 2001 From: Alexis ROYER Date: Thu, 18 Sep 2025 18:33:21 +0200 Subject: [PATCH 34/35] Handle `VolatileValue` fields (#4705) In `Packet.self_build()`. Defensive code added in `DHCPOptionsField.i2repr()` and `i2m()`. --- scapy/layers/dhcp.py | 14 ++++++++++++++ scapy/packet.py | 9 +++++++++ 2 files changed, 23 insertions(+) diff --git a/scapy/layers/dhcp.py b/scapy/layers/dhcp.py index aeedf8e35c2..59f02bf235f 100644 --- a/scapy/layers/dhcp.py +++ b/scapy/layers/dhcp.py @@ -52,6 +52,7 @@ RandInt, RandNum, RandNumExpo, + VolatileValue, ) from scapy.arch import get_if_hwaddr @@ -401,6 +402,15 @@ class DHCPOptionsField(StrField): islist = 1 def i2repr(self, pkt, x): + # `x` may happen to be a `RandDHCPOptions` instance. + # Example: + # ```python + # pkt = fuzz(DHCP()) + # pkt.show() + # ``` + if isinstance(x, VolatileValue): + return repr(x) + s = [] for v in x: if isinstance(v, tuple) and len(v) >= 2: @@ -471,6 +481,10 @@ def m2i(self, pkt, x): return opt def i2m(self, pkt, x): + # Ensure the `RandDHCPOptions` instance has been resolved before (see `Packet.self_build()`). + # + assert not isinstance(x, VolatileValue), f"Bad value {x!r}, should have been resolved before" + if isinstance(x, str): return x s = b"" diff --git a/scapy/packet.py b/scapy/packet.py index 5ab9187b837..2ec5e5c34fe 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -908,6 +908,15 @@ def self_build(self): p = b"" for f in self.fields_desc: val = self.getfieldval(f.name) + + if isinstance(val, VolatileValue): + # Resolve the volatile value. + val = val._fix() + + # Disable caching for the current packet. + self.no_cache = True + self.raw_packet_cache = None + if isinstance(val, RawVal): p += bytes(val) else: From 2c638a214e85c51e46d7e9e4d7654e8aeb286692 Mon Sep 17 00:00:00 2001 From: Alexis ROYER <202673196+alxroyer-thales@users.noreply.github.com> Date: Tue, 30 Sep 2025 18:09:39 +0200 Subject: [PATCH 35/35] Fix caching management for `_NDRPacket` (#4705) --- scapy/layers/dcerpc.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/scapy/layers/dcerpc.py b/scapy/layers/dcerpc.py index 346de69bab2..1e7b5c26b87 100644 --- a/scapy/layers/dcerpc.py +++ b/scapy/layers/dcerpc.py @@ -1292,6 +1292,9 @@ def _e(ndrendian): class _NDRPacket(Packet): __slots__ = ["ndr64", "ndrendian", "deferred_pointers", "request_packet"] + # Can't trust the cache due to deferred pointers. + no_cache = True + def __init__(self, *args, **kwargs): self.ndr64 = kwargs.pop("ndr64", False) self.ndrendian = kwargs.pop("ndrendian", "little") @@ -1313,12 +1316,6 @@ def do_dissect(self, s): ) return super(_NDRPacket, self).do_dissect(s) - def post_dissect(self, s): - if self.deferred_pointers: - # Can't trust the cache if there were deferred pointers - self.raw_packet_cache = None - return s - def do_build(self): _up = self.parent or self.underlayer for f in self.fields.values():