Wireshark · Wireshark-dev: Re: [Wireshark-dev] PIM dissector

[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

On 04/05/2013 05:04 PM, mmann78@xxxxxxxxxxxx wrote:
> I was looking at making the PIM dissector more filterable and noticed that it
> has been labelled a "tunnelling protocol" (per revision 827), so that subsequent
> layers (ie IPv4/IPv6) are branched from within the PIM dissector and not on the
> "main" tree.  Is this (still) standard practice?  Taking the sample capture from
> the wiki, it just looks "visually off" to have the IP and subsequent layers come
> off of the "PIM options tree".  Perhaps at least a new tree under the PIM
> dissector should be used instead of "options"?  I personally don't see anything
> wrong with just putting the IP and subsequent layers on the main tree (and that
> code has remained but been #if 0ed out since revision 827).
> 
> Michael
> 

Talking about a blast from the past...

It seems they were going back and forth about this, settling in r901. I've
attached a patch that allows you to play with either configuration.

As this part of the PIM message isn't intended for the PIM entities themselves,
but uses the PIM protocol to carry the message across, the carried payload (IPv4
or IPv6 in this case) will be put onto a section of the destination network.
Therefore this is an independent part of the PIM message. IMHO, it should go in
the main tree.
As an example of such a payload which should not be on the main tree look at
ICMP. There the header is part of the message and not destined other than the
receiving ICMP entity.

Thanks,
Jaap

Index: epan/dissectors/packet-pim.c
===================================================================
--- epan/dissectors/packet-pim.c	(revision 48757)
+++ epan/dissectors/packet-pim.c	(working copy)
@@ -36,6 +36,8 @@
 #define PIM_TYPE(x)     ((x) & 0x0f)
 #define PIM_VER(x)      (((x) & 0xf0) >> 4)
 
+#define MAINTREE
+
 enum pimv2_addrtype {
     pimv2_unicast, pimv2_group, pimv2_source
 };
@@ -198,7 +200,9 @@
          * this register will overwrite the PIM info in the columns.
          */
         pim_length = 8;
+#ifndef MAINTREE
         col_set_writable(pinfo->cinfo, FALSE);
+#endif
     } else {
         /*
          * Other message - checksum the entire packet.
@@ -304,14 +308,14 @@
                                     "Dummy header for an unknown protocol");
             break;
         case 4: /* IPv4 */
-#if 0
+#ifdef MAINTREE
             call_dissector(ip_handle, next_tvb, pinfo, tree);
 #else
             call_dissector(ip_handle, next_tvb, pinfo, pimopt_tree);
 #endif
             break;
         case 6: /* IPv6 */
-#if 0
+#ifdef MAINTREE
             call_dissector(ipv6_handle, next_tvb, pinfo, tree);
 #else
             call_dissector(ipv6_handle, next_tvb, pinfo, pimopt_tree);
@@ -680,7 +684,9 @@
              * this register will overwrite the PIM info in the columns.
              */
             pim_length = 8;
+#ifndef MAINTREE
             col_set_writable(pinfo->cinfo, FALSE);
+#endif            
         } else {
             /*
              * Other message - checksum the entire packet.
@@ -907,14 +913,14 @@
                                     "Dummy header for an unknown protocol");
             break;
         case 4: /* IPv4 */
-#if 0
+#ifdef MAINTREE
             call_dissector(ip_handle, next_tvb, pinfo, tree);
 #else
             call_dissector(ip_handle, next_tvb, pinfo, pimopt_tree);
 #endif
             break;
         case 6: /* IPv6 */
-#if 0
+#ifdef MAINTREE
             call_dissector(ipv6_handle, next_tvb, pinfo, tree);
 #else
             call_dissector(ipv6_handle, next_tvb, pinfo, pimopt_tree);