Wireshark · Wireshark-dev: Re: [Wireshark-dev] Splitting packet_info struct for performance reasons?

[Guy Harris <guy@xxxxxxxxxxxx>]

On Mar 1, 2009, at 12:45 PM, Kaul wrote:

> I was astounded with the huge size of packet_info structure. I  
> believe in 99% of the cases, there is no need for many of the fields  
> within the structure. Wouldn't it make sense, for performance  
> reasons, to leave the most usable ones within it, and create a  
> pointer to an extra structure with the other, less commonly used  
> stuff. I'm pretty sure it'll save memory and probably improve  
> performance, but would require some changes and complicate a bit the  
> programming. Thoughts?

packet_info has become the grab bag of packet information in Wireshark  
(heck, that dates back to when it was still called Ethereal).

There are several issues here:

	1) we do most columns in the packet list by saving the binary  
information and formatting it only when we add the packet to the list  
(to avoid formatting strings in one dissector only to replace that  
string in a higher-layer dissector - that actually did save some CPU  
time);

	2) we use packet_info to handle "dissect as", so that the GUI can  
determine to which flow (at a given protocol layer) the packet belongs;

	3) we might also use this as a way to pass information between  
dissectors, e.g. addresses and ports.

The column handling needs some rethinking, given that:

	1) crap piles up in packet_info to implement this;

	2) in order to add support for a new column, code has to be changed  
in a number of places.

"Dissect as" could also use some rethinking, to generalize it.  In  
most forms of "decode as", the user wants to decode a particular flow,  
at a given protocol layer, as a particular protocol above that layer,  
e.g.:

	decode a particular TCP connection (pair of address/port endpoints)  
or UDP flow (pair of address/port endpoints, although perhaps with  
wildcards and/or adding extra packets, e.g. the first packet of a TFTP  
session);

	decode a particular DCE RPC session (session atop which DCE RPC runs,  
plus DCE RPC context ID);

and there are probably other potential examples (ATM virtual circuit,  
L2TPv3 session, etc.).  This might tie in with some changes to the  
conversation mechanism I suggested in

	http://www.wireshark.org/lists/wireshark-dev/200902/msg00315.html

Passing information between protocols could also use some rethinking -  
currently, we have pinfo->private_data, but in some cases dissectors  
can step on each other's toes; perhaps what we need is a stack of  
private information, with the items tagged by the protocol that put  
them there or the protocol for which they're intended.

> Semi-random examples:
> guint16 src_idx;              /* Source port index (Cisco MDS- 
> specific) */
>   guint16 dst_idx;              /* Dest port index (Cisco MDS- 
> specific) */
>   guint16 vsan;                 /* Fibre channel/Cisco MDS-specific */
>
> (how many of us need the above?)

Those are there for columns.

>   guint16 link_number;

I'm not sure what that's there for.  It stores an MTP2 link number,  
but the only code that sets it is in dissect_frame() and the only code  
that *uses* it is in dissect_frame(); that code sets it in the frame  
information part of the protocol tree - it arguably belongs instead in  
the MTP2 part instead - rather than in a column.  I guess it's a  
little more complicated if MTP2 is encapsulated in another protocol  
and that protocol provides the link number; that starts looking like  
passing information between dissectors.

>   guint8  annex_a_used;

Similar to link_number, although it is used in the MTP2 dissector -  
which should probably just dig it out of the pseudo-header or, again,  
if there's encapsulation in another protocol, from a passing-between- 
dissectors mechanism.

>   guint16 profinet_type;     /* the type of PROFINET packet (0: not  
> a PROFINET packet) */

Looks as if it's used to pass data between dissectors.

> (It it even 16bit aligned?!)

Yes.  By default, C compilers align structure members on the  
appropriate boundary, with padding (otherwise they either have to  
generate less-efficient code to access unaligned members or generate  
code that the processor might handle less efficiently); it's only with  
compiler-specific pragmas or attributes or... that you get "packed"  
structures with unaligned members.