Wireshark-dev: Re: [Wireshark-dev] PIM dissector
From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Sat, 06 Apr 2013 16:17:22 +0200
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);
- Follow-Ups:
- Re: [Wireshark-dev] PIM dissector
- From: Guy Harris
- Re: [Wireshark-dev] PIM dissector
- References:
- [Wireshark-dev] PIM dissector
- From: mmann78
- [Wireshark-dev] PIM dissector
- Prev by Date: [Wireshark-dev] PIM dissector
- Next by Date: Re: [Wireshark-dev] PIM dissector
- Previous by thread: [Wireshark-dev] PIM dissector
- Next by thread: Re: [Wireshark-dev] PIM dissector
- Index(es):