From ec97aa46e05e3a741e3a4b3c702ea0ba18238f7c Mon Sep 17 00:00:00 2001 From: Nils Weiss Date: Thu, 13 Aug 2026 09:35:04 +0200 Subject: [PATCH] Improve performance of packet dissection and build Packet.__setattr__ has to resolve field names before it can fall back to a plain slot assignment, and Packet.__init__ goes through it 22 times for every layer that is dissected or built. It was the hottest function in both paths, at 18.8M calls per 21k dissected packets. Initialize the slots with object.__setattr__ instead. Their types move to class-level annotations, since that form leaves nowhere to put an inline type comment. PacketListField.getfield located the trailing Padding with "conf.padding_layer in p" followed by "p[conf.padding_layer]": two full recursive layer traversals per list element, ~2.7us for a two-layer element. Dissection always appends Padding as the last layer of the payload chain, so lastlayer() answers the same question in ~0.3us. This no longer descends into sub-packet fields, where a nested Padding was a false positive that truncated the list. copy_fields_dict() and getfield_and_val() went through copy_field_value() and get_field(), spending two Python frames per field just to reach self.fieldtype[name]. Index it directly; both helpers remain for external callers. Measured against master over 16 packet types, median of 6 interleaved rounds on a pinned core with a +/-0.6% noise floor: dissect +7.7% build (fresh packet) +10.2% dissect, PacketListField-heavy +13.9% (DNS/DHCP6/SCTP) rebuild (dissected packet, cached) +2.0% No API changes. 6600 tests pass, mypy reports no new errors, and flake8 reports two fewer warnings. The 8 remaining test failures need live OpenLDAP/SMB servers and fail on master too. AI-Assisted: yes (Claude Opus 5) Co-authored-by: Cursor --- scapy/fields.py | 14 +++++---- scapy/packet.py | 84 ++++++++++++++++++++++++++++++++----------------- 2 files changed, 63 insertions(+), 35 deletions(-) diff --git a/scapy/fields.py b/scapy/fields.py index 3e158d8520d..1770f813d7c 100644 --- a/scapy/fields.py +++ b/scapy/fields.py @@ -1590,9 +1590,11 @@ def getfield(self, # type: (...) -> Tuple[bytes, K] i = self.m2i(pkt, s) remain = b"" - if conf.padding_layer in i: - r = i[conf.padding_layer] - del r.underlayer.payload + # Dissection always appends Padding last, so checking the topmost + # layer avoids a full recursive haslayer()+getlayer() lookup. + r = i.lastlayer() + if isinstance(r, conf.padding_layer): + del r.underlayer.payload # type: ignore remain = r.load return remain, i # type: ignore @@ -1835,10 +1837,10 @@ def getfield(self, pkt, s): p = conf.raw_layer(load=remain) remain = b"" else: - if conf.padding_layer in p: - pad = p[conf.padding_layer] + pad = p.lastlayer() + if isinstance(pad, conf.padding_layer): remain = pad.load - del pad.underlayer.payload + del pad.underlayer.payload # type: ignore if self.next_cls_cb is not None: cls = self.next_cls_cb(pkt, lst, p, remain) if cls is not None: diff --git a/scapy/packet.py b/scapy/packet.py index 9440f51469d..3dd11cfe5e3 100644 --- a/scapy/packet.py +++ b/scapy/packet.py @@ -104,7 +104,29 @@ class Packet( "comments", "process_information" ] - name = None + # Types of the __slots__ above: __init__ sets them through + # object.__setattr__, which leaves nowhere to put an inline type comment. + time: Union[EDecimal, float] + sent_time: Union[EDecimal, float, None] + default_fields: Dict[str, Any] + fields: Dict[str, Any] + fieldtype: Dict[str, AnyField] + overloaded_fields: Dict[str, Any] + packetfields: List[AnyField] + original: bytes + explicit: int + raw_packet_cache: Optional[bytes] + raw_packet_cache_fields: Optional[Dict[str, Any]] + stop_dissection_after: Optional[Type['Packet']] + payload: 'Packet' + underlayer: Optional['Packet'] + parent: Optional['Packet'] + direction: Optional[int] + sniffed_on: Optional[_GlobInterfaceType] + wirelen: Optional[int] + comments: Optional[List[bytes]] + process_information: Optional[Dict[str, Any]] + name = None # type: Optional[str] fields_desc = [] # type: ClassVar[List[AnyField]] deprecated_fields = {} # type: Dict[str, Tuple[str, str]] overload_fields = {} # type: Dict[Type[Packet], Dict[str, Any]] @@ -155,33 +177,36 @@ def __init__(self, **fields # type: Any ): # type: (...) -> None - self.time = 0.0 if _internal else time.time() # type: Union[EDecimal, float] - self.sent_time = None # type: Union[EDecimal, float, None] - self.name = (self.__class__.__name__ - if self._name is None else - self._name) - self.default_fields = {} # type: Dict[str, Any] - self.overload_fields = self._overload_fields - self.overloaded_fields = {} # type: Dict[str, Any] - self.fields = {} # type: Dict[str, Any] - self.fieldtype = {} # type: Dict[str, AnyField] - self.packetfields = [] # type: List[AnyField] - self.payload = NoPayload() # type: Packet + # Every attribute set below is a __slots__ member, so bypass + # __setattr__: resolving field names there costs more than the + # assignment itself, and this runs for every dissected layer. + _set = object.__setattr__ + _set(self, "time", 0.0 if _internal else time.time()) + _set(self, "sent_time", None) + _set(self, "name", self.__class__.__name__ + if self._name is None else self._name) + _set(self, "default_fields", {}) + _set(self, "overload_fields", self._overload_fields) + _set(self, "overloaded_fields", {}) + _set(self, "fields", {}) + _set(self, "fieldtype", {}) + _set(self, "packetfields", []) + _set(self, "payload", NoPayload()) self.init_fields(bool(_pkt)) - self.underlayer = _underlayer - self.parent = _parent + _set(self, "underlayer", _underlayer) + _set(self, "parent", _parent) if isinstance(_pkt, bytearray): _pkt = bytes(_pkt) - self.original = _pkt - 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] - self.comments = None # type: Optional[List[bytes]] - self.process_information = None # type: Optional[Dict[str, Any]] - self.stop_dissection_after = stop_dissection_after + _set(self, "original", _pkt) + _set(self, "explicit", 0) + _set(self, "raw_packet_cache", None) + _set(self, "raw_packet_cache_fields", None) + _set(self, "wirelen", None) + _set(self, "direction", None) + _set(self, "sniffed_on", None) + _set(self, "comments", None) + _set(self, "process_information", None) + _set(self, "stop_dissection_after", stop_dissection_after) if _pkt: self.dissect(_pkt) if not _internal: @@ -536,11 +561,11 @@ def getfield_and_val(self, attr): if self.deprecated_fields and attr in self.deprecated_fields: attr = self._resolve_alias(attr) if attr in self.fields: - return self.get_field(attr), self.fields[attr] + return self.fieldtype[attr], self.fields[attr] if attr in self.overloaded_fields: - return self.get_field(attr), self.overloaded_fields[attr] + return self.fieldtype[attr], self.overloaded_fields[attr] if attr in self.default_fields: - return self.get_field(attr), self.default_fields[attr] + return self.fieldtype[attr], self.default_fields[attr] raise ValueError def __getattr__(self, attr): @@ -726,7 +751,8 @@ def copy_fields_dict(self, fields): # type: (_T) -> _T if fields is None: return None - return {fname: self.copy_field_value(fname, fval) + fieldtype = self.fieldtype + return {fname: fieldtype[fname].do_copy(fval) for fname, fval in fields.items()} def _raw_packet_cache_field_value(self, fld, val, copy=False):