Hi Jaap,

If I understand correctly I think the numbers are correct by design.

When viewing packet details the analysis is almost always on the protocol header. In this case that's what the size represents and that's what  I would expect.

I don't typically use Protocol Hierarchy statistics but I think counting total protocol size (header + payload) is a lot more interesting and useful. That matches my understanding of PDU size, which is presumably what I'm trying to look at with this statistic.

I think there is still a bug lurking with fixed-size headers vs variable size headers that needs to be fixed, to match the current behavior.


On 13/02/22 23:18, Jaap Keuter wrote:
This discussion was brought on by issue 17877 titled “Non-visible photo items 
cannot change length after construction”. In there it is correctly stated that 
calls to set proto_item length (either proto_item_set_len() or 
proto_item_set_end()) are not effective when proto_items are not visible. When 
looking at these functions it can be seen that these are part of the group of 
functions which are optimised for faking proto_items when not visible.

Without going into the faking details (see the macro 
TRY_TO_FAKE_THIS_ITEM_OR_FREE in epan/proto.c for that) it comes down to just 
short cutting the proto_item creation process by returning the tree intended to 
attach the newly to be constructed proto_item to. In effect these all return 
the root node of the dissection tree.

A special case exception is put in place for proto_items of type FT_PROTOCOL. 
EPAN can be setup so that proto_items of this type in fact are allowed to be 
created, even if they otherwise would have been faked. This feature is used for 
protocol level statistics. The protocol level statistics ’tally the numbers’ by 
running the dissection, with a hidden tree, but with the special case exception 
set. This results in a very compact dissection tree, consisting of the root 
node and the proto_items of type FT_PROTOCOL. The length of these items is then 
used to determine the numbers to add to the various protocol layers in the 
statistics tree.

This is where the ambiguity comes in. Some protocols claim just there share of 
the octets in the frame (discussing Ethernet packet dissection here, to keep it 
simple). Others create their proto_item until the end of the TVB handed to them 
(usually to the end of the frame), and adjust the length after dissection of 
their fields have taken place and the variable number of bytes in the protocol 
layer has been determined. However, the functions to set these lengths don’t 
work when faking items is in effect.

As a result these protocols take up way more of the frame in the statistics 
than they in fact do. Overall more the 100% of the frame is allotted to the 
protocols contained in them. The User's Guide goes into this fact with the 
explanation that these protocol 'contain’ their payload, so that is why the 
payload is added to the protocol. That is one interpretation, but not really 
consistent because fixed size dissectors, which create their proto_items of 
type FT_PROTOCOL with fixed size, do not exhibit this behaviour.

The simplest step to take would be to allow the functions proto_item_set_len() 
and proto_item_set_end() to operate on proto_items of type FT_PROTOCOL if the 
afore mentioned special case exception was in effect. However, since faking of 
other types of proto_items is still in effect, all these other proto_items are 
now using the proto_items of type FT_PROTOCOL as proto_item, rather than the 
root node. This means that code setting the length of a field, is now also no 
longer blocked, and in fact setting the length of the proto_item of type 
FT_PROTOCOL rather than his own (which is faked).

A simple experiment with the file (code_mac_tagged.pcap) attached to issue 
17877 makes this clear. Changing proto_items_set_len() to allow proto_items 
with type FT_PROTOCOL to set their length if the special case exception is set, 
shows a protocol hierarchy statistics page with all protocols matching their 
length in the dissection details pane. Except for COSE, the dissection details 
say 309 while the statistics say 246. This last length is the length set for 
the final part of the PDU, which ends up becoming the length of the protocol in 
the protocol hierarchy statistics.

The question now becomes how to proceed with this. Faking proto_items makes 
legitimate calls to set the length of proto_items of type FT_PROTOCOL 
indistinguishable from those calls meant for fields within those protocols. 
Another approach of faking proto_items by always returning the root node 
instead of the tree creates it’s own set of side effects. Now all 
’non-protocol’ proto_items are processed for statistics too, and exposing 
things like expert info in the statistics also. This defeats the purpose of 
faking proto_items in the first place.

A ‘quick and dirty’ solution is to run dissection with a visible tree. This 
gives the desired results, but at the cost of doing a lot more dissection work 
than strictly necessary, voiding the whole purpose of the special case 
exception in not faking proto_items with type FT_PROTOCOL.

I’m not seeing a simple way out of this. Do we want to modify the statistics in 
this way is the fist question to answer. They will be different than in 3.6, 
3.4 and earlier. This could be the right time to do it though. Then the 
question becomes how to achieve this without taking a significant performance 
hit by sidestepping the faking of proto_items.

Thanks,
Jaap

(not rereading this draft, it’s way too late for that. Sorry for any mistyping 
or confusing statements)



___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
              mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
            mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Reply via email to