Re: Packet Diagram shows only raw bytes of a subtree instead of individual fields

[Gerald Combs <gerald () wireshark org>]

On 11/26/20 11:03 AM, John Thacker wrote:
> On Thu, Nov 26, 2020 at 1:19 PM Maynard, Christopher via Wireshark-dev <wireshark-dev () wireshark org 
> <mailto:wireshark-dev () wireshark org>> wrote:
>
>     Many protocols contain subtrees, such as a header with various fields that are part of the header, and it’s 
> convenient/logical to group those fields within the header subtree.  However, doing so results in a Packet Diagram 
> that only shows the raw bytes of the subtree rather than the individual fields contained within the subtree.
>      
>     So either I’m doing something wrong, in which case I welcome any suggestions for improving the display, or there 
> seems to be a current limitation to the way the Packet Diagram behaves with respect to subtrees.  Has anyone else 
> noticed this?
>     ...
>      
>     Is there a way to achieve this while still grouping the fields within a subtree?
>
>
> Not in a subtree currently. If you look around line 600 of ui/qt/packet-diagram.cpp, you'll see that it only groups 
> the top level fields in each protocol.

That's correct. I wanted to keep the initial implementation as simple, naive, and lazy as possible. 

> For the same reason, bitmask fields that are grouped together not in a subtree, using proto_tree_add_bitmask_list()
> (like packet-rtp.c#L2072 with octet1_fields), then they are displayed separately (in master, post commit
> https://gitlab.com/wireshark/wireshark/-/commit/7654bb260d08fdb7adeafce1877fa3c38f3465ae 
> <https://gitlab.com/wireshark/wireshark/-/commit/7654bb260d08fdb7adeafce1877fa3c38f3465ae> ), whereas
> for bitmask fields that are added with a subtree with proto_tree_add_bitmask() only the top level header
> item appears.
>
> You can see some images here: https://gitlab.com/wireshark/wireshark/-/merge_requests/959 
> <https://gitlab.com/wireshark/wireshark/-/merge_requests/959>
> There you can see bitmask fields that are displayed properly because there is no subtree.
>
> I agree it would be a nice enhancement to travel down into the children of items that have children, though I imagine
> you'd have to take care in some cases; e.g., dissect_e164_msisdn() from packet-e164.[ch] is used a lot in various 
> dissectors,
> and has a header that has the entire number, with child that only has the country code (but not a child for the non 
> country code digits).
> The simplest way to descend into the subtree for a E.164 number would thus only has an entry for the country code but 
> leave the
> other bits blank. Or you could have issues with dealing with overlaps.

Would it make sense to add second-level items only if they collectively fit the offset+size of the top-level item? In 
this case we'd skip the second-level country code child, but we'd add it if dissect_e164_msisdn() added a non-country 
code sibling field.

BTW, for 1- and possibly 2-bit fields we might want to take inspiration from 
https://twitter.com/vivekrj/status/1269649718059118601 and rotate the label 90 degrees.
___________________________________________________________________________
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-request () wireshark org?subject=unsubscribe