>> Explicit how? Dissectors do not always have to be registered with a table 
>> such as tcp.port.

For instance like this

static_dissector_add("tcp", dissect_dcm_static, "DICOM over TCP", "dicom_tcp", 
proto_dcm, tcp_port_range);
static_dissector_add("udp", dissect_dcm_static, "DICOM over UDP", "dicom_udp", 
proto_dcm, udp_port_range);

proto_dcm for me is just the umbrella, that can be linked to different layers.

>> Are you sure? sip_tcp and sip_udp are different heuristics dissectors, I 
>> presume that they can be disabled via the GUI as well.

Based the debugging of my own dissector, I'm quite convinced. I don't want to 
bash SIP. I'm just using it to illustrate my thoughts. Let's look at this:

sip_handle = register_dissector("sip", dissect_sip, proto_sip);
sip_tcp_handle = register_dissector("sip.tcp", dissect_sip_tcp, proto_sip);
...
dissector_add_uint_range_with_preference("udp.port", DEFAULT_SIP_PORT_RANGE, 
sip_handle);
dissector_add_uint_range_with_preference("tcp.port", DEFAULT_SIP_PORT_RANGE, 
sip_tcp_handle);
...
heur_dissector_add("udp", dissect_sip_heur, "SIP over UDP", "sip_udp", 
proto_sip, HEURISTIC_ENABLE);
heur_dissector_add("tcp", dissect_sip_tcp_heur, "SIP over TCP", "sip_tcp", 
proto_sip, HEURISTIC_ENABLE);
…

This results in 

[ ] SIP
    [ ] sip_tcp
    [ ] sip_udp
    [ ] sip_sctp ...

- Disabling only the parent "SIP", disables BOTH STATIC ones 
'dissector_add_uint_range_with_preference' udp.port & tcp.port, but leaves the 
heuristic ones in place
- Disabling only sip_tcp/sip_udp disables the respective heur_dissector_add() 
only. The two static port remain in place.

>> So when would you like to enable all heuristics dissectors, for all 
>> protocols, but not enable the "static" dissector (as done by Enable/Disable 
>> All)?

More the other way around. I want to disable all heuristic ones but keep the 
static ones in place. 
Mindset wise, predictability means a lot to me. So from a performance 
perspective, it always going to behave equally slow/fast. if I have the static 
ones enables, but the heuristic ones not, I don't expect different behavior 
between two captures on the same equipment. Ports won't change too much, 
payload may quite a bit.

Regards
David

-----Original Message-----
From: Wireshark-dev <[email protected]> On Behalf Of Peter Wu
Sent: Friday, March 23, 2018 12:36
To: Developer support list for Wireshark <[email protected]>
Subject: Re: [Wireshark-dev] heur_dissector_add()

On Fri, Mar 23, 2018 at 09:07:58AM +0100, David Aggeler wrote:
> Hi Peter,
> 
> >> "DICOM on any TCP port" sounds more logical for a single protocol, 
> >> but there are actually protocols that run on other transports. One example 
> >> is SIP which has "SIP over SCTP", "SIP over TURN", "SIP over TCP" and "SIP 
> >> over UDO".
> 
> "SIP over TCP" / "SIP over UDP" are done the same ways as the DICOM is done. 
> So it's not all bindings I agree, but many.
> Therefore let's focus on IP based protocols for a moment. To me
> 
> protocolx_over_tcp_static             should be different to 
> protocolx_over_tcp_heuristic
> 
> A) In my understanding, the TCP static binding is done with the following 
> line. Here I cannot specify a name.
> This is borderline questionable, because it’s a UI preference function.
> 
>       dissector_add_uint_range_with_preference("tcp.port", 
> DICOM_DEFAULT_RANGE, dcm_handle);

Seems correct, the protocol name is taken from the handle which was returned by 
register_dissector or something like that.

> B) The TCP heuristic binding is done with
> 
>       heur_dissector_add()
> 
> 
> Is my understanding of A) correct? If yes, I'd prefer it more explicit. If 
> no, what am I missing on the static assignment? 

Explicit how? Dissectors do not always have to be registered with a table such 
as tcp.port. For example, the "data" dissector is looked up by dissectors using 
find_dissector("data") and then used directly.
That's likely why registration of dissectors is separate from linking them to 
something like tcp.port.

> With the currently implementation I cannot disable "SIP over TCP", but keep 
> "SIP over UDP" on.

Are you sure? sip_tcp and sip_udp are different heuristics dissectors, I 
presume that they can be disabled via the GUI as well.

> Form a tree perspective, the following would make sense. And toggling the 
> parent should toggle the children.
> 
> [ ] ProtocolX 
>       [ ] ProtocolX over TCP (static)
>       [ ] ProtocolX over TCP (heuristic)

I agree it would make more sense. For single protocols, there would just be a 
single item (which could be just heuristics or "static"). For protocols with 
multiple registrations, a tree would be visible.

Perhaps it could even append a comment to the protocol ("1 heuristics dissector 
hidden") in case a search query is active. Or maybe even display all subtree 
items since you are probably looking for a protocol rather than a specific 
instance.

> >> Do you think it is sufficient to append "(heuristic)" automatically in the 
> >> GUI after each description?
> 
> If my understanding of A) is correct, then yes, this comes pretty close to 
> the original idea in the 2015 discussion thread.

I guess you are referring to this "Enabling/disabling ANY heuristic dissector" 
discussion?
https://www.wireshark.org/lists/wireshark-dev/201507/msg00027.html

> It's not a single flag but better than nothing. Two buttons to 
> 'Enable/Disable all heuristic at once'  would pretty much be that part. Or 
> better 'Enable/Disable Visible'. Then with keyword, one can filter first and 
> disable / enable that subset.

So when would you like to enable all heuristics dissectors, for all protocols, 
but not enable the "static" dissector (as done by Enable/Disable All)? 
"Enable/Disable all visible" sounds reasonable, but perhaps we can do better. 
Do you have a use case in mind?

Kind regards,
Peter
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <[email protected]>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:[email protected]?subject=unsubscribe

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

Reply via email to