Wireshark-dev: Re: [Wireshark-dev] New dissector: PANA
From: "ronnie sahlberg" <[email protected]>
Date: Fri, 14 Jul 2006 09:24:33 +0000
checked in


can you upload the/a capture file to the SampleCaptures page of the
wireshark wiki so it gets pulled in to fuzztesting?


please also create a wiki page for this protocol.   it does not take much time.


i made some changes to the code and strengthened the heuristics.
is is very important that heuristic dissectors have heuristics as
sharp as possible since with so many protocols in wireshark
collissions happen.



it would be nice if the flags dissection was enhanced to put the name
of all ENABLED flags on the expansion line   as so many other
dissectors do  and so one does not need to open the expansion to see
if FOO is set or not.   This is a big useability enhancement and
should be implemented!
See packet-dcerpc-atsvc.c for an example how nice and userfriendly
bitmasks are dissected.



The header of the file is missing a comment that points to the
relevant RFC or draft for this protocol, as does the wikipage to be
created





On 7/14/06, Peter Racz <[email protected]> wrote:
Attached is the new version.

ronnie sahlberg wrote:
> much better,   some additional minor items though
>
> 1,   set COL_INFO and COL_PROTOCOL before you call the subdissector
helpers
> why?    in case the packets are short (small snaplen) then the
> subdissector helper will run out of data in the tvb
> throw an exception and the dissection will be aborted before the code
> reaches the point where COL_PROTOCOL is updated.
> the effect is that short or malformed packets will then not be marked as
> PANA in the protocol column.

I see. My problem was, however, that some of the PANA messages contain an
EAP message
as an AVP. In that case EAP overwrites the info and protocol columns
sometimes, which
I find not right (as EAP is just one parameter in the PANA message). Do you
have a
proposal how to solve this problem?

>
> 2, remove the fprintf's from the heuristics.
> when doing that, are there not a few missing return FALSE   in these
tests?

I removed the fprintf. However, I don't like not to give a hint in case of
an error.
The other thing with the "missing" return FALSE, I removed them as they are
not an
error. (According to IETF draft: "they MUST be set to zero, and ignored by
the
receiver.") These fields can be shown by Wireshark and do not cause any
error.

>
> do you have an example capture I can test with?

Capture file is attached.