[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

Hi,

Are your changes based on RFC 3847? If so, why is the required length 3?
The RFC states: (Note: Remaining fields are required when the RA bit is
set).
Also why is the required length including SYSID 9? The RFC states:(ID
Length octets).
Also it's unusual to include the declaration of isis_restart_flag_vals
inside the function.

Thanx,
Jaap

On Mon, 3 Apr 2006, Hannes Gredler wrote:

> hi ethereal developers,
>
> attached a patch that makes the IS-IS Restart CLV disector
> compliant to RFC 3847;
>
> asking for inclusion;
>
> /hannes
>
>
> Index: epan/dissectors/packet-isis-hello.c
> ===================================================================
> --- epan/dissectors/packet-isis-hello.c (revision 17281)
> +++ epan/dissectors/packet-isis-hello.c (working copy)
> @@ -336,29 +336,51 @@
>    *
>    */
>
> +#define ISIS_CLV_RESTART_FLAG_BITS 3
> +#define ISIS_CLV_RESTART_FLAG_MASK 0x07
> +#define ISIS_CLV_RESTART_MINLEN 3
> +#define ISIS_CLV_RESTART_SYSID_MINLEN 9
> +
>   static void
>   dissect_hello_restart_clv(tvbuff_t *tvb,
>                  proto_tree *tree, int offset, int id_length _U_, int length)
>   {
> -       int restart_options;
> +       guint8 restart_flags;
>
> -       if (length != 3) {
> +        static const value_string isis_restart_flag_vals[] = {
> +            { 0x1,  "Restart Request"},
> +            { 0x2,  "Restart Acknowledgement"},
> +            { 0x4,  "Suppress adjacency advertisement"},
> +            { 0, NULL }
> +        };
> +
> +       if (length != ISIS_CLV_RESTART_MINLEN &&
> +            length != ISIS_CLV_RESTART_SYSID_MINLEN) {
>              isis_dissect_unknown(tvb, tree, offset,
> -                                "malformed TLV (%d vs 3)",
> +                                "malformed TLV (%u vs 3/9)",
>                                   length, 3 );
>              return;
>          }
>
> -       restart_options = tvb_get_guint8(tvb, offset);
> +       restart_flags = tvb_get_guint8(tvb, offset);
>
> -       proto_tree_add_text ( tree, tvb, offset, 1,
> -                             "Restart Request bit %s, "
> -                             "Restart Acknowledgement bit %s",
> -                             ISIS_MASK_RESTART_RR(restart_options) ? "set" : "clear",
> -                             ISIS_MASK_RESTART_RA(restart_options) ? "set" : "clear");
> +        proto_tree_add_text ( tree, tvb, offset, 1, "Restart Flags: %s",
> +                              decode_enumerated_bitfield(restart_flags,
> +                                                         ISIS_CLV_RESTART_FLAG_MASK,
> +                                                         ISIS_CLV_RESTART_FLAG_BITS,
> +                                                         isis_restart_flag_vals, "%s" ));
> +
> +        /* remaining holdtime */
>          proto_tree_add_text ( tree, tvb, offset+1, 2,
>                                "Remaining holding time: %us",
>                                tvb_get_ntohs(tvb, offset+1) );
> +
> +        /* On LAN circuits there may be an optional sysid. */
> +        if (length == ISIS_CLV_RESTART_SYSID_MINLEN) {
> +            proto_tree_add_text ( tree, tvb, offset+3, 6,
> +                                  "System-ID: %s",
> +                                  print_system_id( tvb_get_ptr(tvb, offset+3, 6), 6 ) );
> +        }
>   }
>
>   /*
> Index: epan/dissectors/packet-isis-hello.h
> ===================================================================
> --- epan/dissectors/packet-isis-hello.h (revision 17281)
> +++ epan/dissectors/packet-isis-hello.h (working copy)
> @@ -40,13 +40,6 @@
>   #define ISIS_HELLO_TYPE_LEVEL_12       3
>
>   /*
> - * misc. bittest macros
> - */
> -
> -#define ISIS_MASK_RESTART_RR(x)            ((x)&0x1)
> -#define ISIS_MASK_RESTART_RA(x)            ((x)&0x2)
> -
> -/*
>    * Published API functions.  NOTE, this are "local" API functions and
>    * are only valid from with isis decodes.
>    */
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>