Wireshark · Ethereal-dev: [Ethereal-dev] Re: New dissector for CIGI

[ronnie sahlberg <ronniesahlberg@xxxxxxxxx>]

See the modified dissector i have attached.

I do some very basic tests in dissect_cigi()   but know not enough
about this protocol to make the tests really good.
Can you fill in and expand on these tests to make sure the packet
really is cigi?


See also the SOF Timestamp flag in the cigi3 header.    i changed it
to FT_BOOLEAN and using a true_false string.

can you change the other booleans like this as well?




Does cigi use a reserved/registered set of ports?   i assume not.
If cigi does not use a standardized port then we can as a next step
make it into a heuristic dissector   but the test  "is trhis cigi or
not?"  needs to become as sharp as possible first.
That would get rid of the "register port range for cigi"   and let
ethereal heuristically find what is cigi and what is not   completely
automagically.




On 12/2/05, Harms, Kyle J <Kyle.J.Harms@xxxxxxxxxx> wrote:
> I have a few questions before I look at this some more.
>
> 1,
> The reason I did Booleans this way is because if I use the FT_BOOLEAN it
> only shows '. = IG Mode...' in the window as apposed to '.... ...1 = IG
> Mode...' which is very useful in debugging CIGI.  Is there a way to use
> FT_BOOLEAN and still see which bits it is actually using i.e. '....
> ...1' ?
>
> 2,
> README.developer does not mention anything about a new style dissector.
> Where might I look at an example of such a dissector?  I might suggest
> that the README.developer stub be updated to be a new style dissector.
>
> 3,
> I don't think there is really a clean way to do a 'range' check in a
> case/switch statement, other than default,  but I need that for else.
>
>         } else if ( packet_id >= CIGI2_PACKET_ID_USER_DEFINABLE_MIN &&
> packet_id <= CIGI2_PACKET_ID_USER_DEFINABLE_MAX ) {
>             hf_cigi2_packet = hf_cigi2_user_definable;
>             packet_length = packet_size;
>         } else {
>             hf_cigi2_packet = hf_cigi_unknown;
>             packet_length = packet_size;
>         }
>
> Is there such a way to make this clean as apposed to
> case 236:
> case 237:
> case 238:
> case 239:
> ...
> case 255:
>
> ?
>
> I will try and address these issues as soon as I can.
>
> Kyle
>
> -----Original Message-----
> From: ronnie sahlberg [mailto:ronniesahlberg@xxxxxxxxx]
> Sent: Friday, December 02, 2005 3:41 PM
> To: Ethereal development
> Subject: [Ethereal-dev] Re: New dissector for CIGI
>
> Hi,
>
>
> 1,
> In some places you specify booleans in a bit suboptimal way:
> Please see :
> the definition of cigi_valid_vals[]   and
> hf_cigi3_ig_control_timestamp_valid.
>
> These ones should instead be using a true_false_string   and the
> hf_field should be    TFS(&cigi_valid_tfs), and be of type FT_BOOLEAN.
>
>
> 2,
> we have many many dissetors now and we get more and more "collissions"
> when ehtereal takes the wrong dissector.
>
> Can you change dissect_cigi()   to being a new-style dissector (i.e.
> returning int) and using new_create_dissector_handle() to register the
> handle.
> The new signature for dissecvt_cigi() would then be static int.
>
> Then dissect_cigi would return 0 if it was not a cigi packet (and
> letting ethereal try something else) or x   for x number of bytes
> eaten by the dissector.
>
> in the beginning of dissect_cigi()  before you start manipulating
> col_info/col_protocol and before starting dissection  you should add
> some heuristics to verify (as far as this is possible) that this does
> indeed look like a cigi packet.  and return 0 if not.
>
>
> 3,
> you use very long chains of if()  {} else if {} when demultiplexing
> things like packet_id and calling the subdissectors.
> change this to a switch/case
>
>
>
> apart from that the dissector looks good
> can you please address the 3 issues above?
>
>
>
>
>
> On 12/2/05, Harms, Kyle J <Kyle.J.Harms@xxxxxxxxxx> wrote:
> > Hi,
> >
> > This patch is for a CIGI dissector (complete versions 2 and 3).  It
> has
> > been [fuzz] tested on GNU/Linux using the Ethereal 0.10.13 codebase.
> > However, the patch here is against the svn repository.
> >
> > More information about CIGI can be found at
> http://cigi.sourceforge.net/
> >
> > Kyle Harms
> >
> >
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>

Attachment: packet-cigi.c.gz
Description: GNU Zip compressed data