["Michael A. McCartney" <mccart@xxxxxxxxxxxxxxxxxx>]

Chris,

I spoke too soon.  I went back to see what was
implemented in the custom dissector (been a while) and
the data being dissected with the code, and found that
the proposed change would not work for me.

The packets I'm dissecting are actually non-Ethernet packets
inside Ethernet framing.  I took another look at the actual
packet-eth code and saw the comment there for the heuristic
dissector actually says that is what it is for.

Changing it to what we discussed would break what I have
and worst, probably would force me to hack packet-eth which
I'd rather not do.  Seems eth is a special case where both
the "framing" needs to be heuristic (as is now and used) as
well as the "payload" (not present like tcp/udp).   Is it
possible to do both, or is it either-or?

Right now, I like it as is without changes.  If/when there is
a need to implement payload as heuristic dissector, then this
needs to revisited how to do this, preferably both ways.

Thanks-Mike







Maynard, Chris wrote:
> Yes, but the source & destination MAC's are passed to sub-dissectors in
> the pinfo->src and pinfo->dst fields anyway, so the original poster
> could have gleaned them from there instead:
>
> packet-eth.c
> Line 205: SET_ADDRESS(&pinfo->src,      AT_ETHER, 6, src_addr);
> Line 209: SET_ADDRESS(&pinfo->dst,      AT_ETHER, 6, dst_addr);
>
> I was hoping to have the question I posed answered before the original
> poster implemented his dissector by using the current heuristic
> implementation, because if the Ethernet dissector is implemented
> incorrectly with regards to passing the entire packet to the registered
> heuristic sub-dissectors, correcting it will break his code.  If it's
> the right thing to do, I think it should be changed anyway, but it will
> be a hassle for him (and perhaps you?) if it is changed though.
>
> - Chris
>
> -----Original Message-----
> From: wireshark-dev-bounces@xxxxxxxxxxxxx
> [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Michael A.
> McCartney
> Sent: Monday, December 17, 2007 11:08 PM
> To: Developer support list for Wireshark
> Subject: Re: [Wireshark-dev] Register dissector to MAC address
>
> Chris,
>
> Orignal poster wanted a way to select the frames
> based on MAC signature - that was a way to do that.
>
> Yes, I agree with you that it passed the whole
> eth frame instead of just the payload like other
> dissectors do.  Just the way it works now in eth
> dissector. 
>
> It should be easy to change it to work like others
> and it would give you what you want, namely a
> eth payload without the Ethernet header. This is the
> right change to do to keep it consistent with the
> existing heuristic design I've seen thus far.
>
> Since the Ethernet encapsulation is only 12 bytes,
> it was sufficient to dup some code into the custom
> dissector to handle that 12 bytes. Yes, ugly I know,
> but does the job at the moment.
>
> My application needs to work off specific MAC signature,
> thus the if() test was fixed, small, and quick, and
> because of that, I was ok with checking every eth
> packet.
>
> As for WOL design you explained, it would be expensive
> to scan for that magic packet in the payload in every
> eth packet, so I totally agree with your proposal for
> a simple fixed etherype value.
>
> Thanks-Mike
>
> Maynard, Chris wrote:
>   
>> Mike,
>>
>> First, thanks for the response.  Like I said below though, I don't
>>     
> think
>   
>> any of the other dissectors work that way.  UDP, for example, makes a
>> call to tvb_new_subset() to pass the payload of the UDP packet to the
>> registered heuristic dissectors, so those dissectors only get the
>> payload without the UDP header.  I would have expected the same for
>> Ethernet heuristic dissection as well, but instead, the entire packet
>>     
> -
>   
>> Ethernet header and all - is passed to the registered heuristic
>> dissectors.
>>
>> For WOL at least, the actual Ethernet MAC is unimportant as far as
>> determining if it's a WOL packet or not.  Instead, only the
>>     
> MagicPacket
>   
>> matters, which theoretically could occur anywhere within the payload.
>> However, if the MagicPacket is found and the heuristic dissector
>> determines it's a WOL packet, then unfortunately I don't think the
>> protocols will be stacked properly.  For example, what we'd like to
>>     
> see
>   
>> is this:
>>
>> Frame <n> 
>> Ethernet II
>> Wake On LAN
>>
>> .... but unfortunately what we'll see is this instead:
>> Frame <n>
>> Wake On LAN
>>
>> That's why I abandoned the heuristic registration and simply
>>     
> registered
>   
>> for Ethertype 0x0842 instead.  Of course the downside to that is
>>     
> 0x0842
>   
>> is not really THE Ethertype for WOL packets.  As far as I can tell,
>> 0x0842 is simply the Ethertype chosen by the author of ether-wake.
>>     
> But
>   
>> who's to say that someone else won't implement the MagicPacket using a
>> different Ethertype one day.  If that happens, WOL packets won't be
>> dissected as such.
>>
>> Then there's the downside of changing the existing behavior - meaning
>> pretty much every packet will have to be scanned to determine if it
>> contains the MagicPacket or not since theoretically, the MagicPacket
>>     
> can
>   
>> occur within ANY packet (i.e., ANY Ethertype).  I suspect that by
>> scanning every packet, things could slow down quite a bit.  So, after
>> all is said and done, perhaps the WOL dissector is best left the way
>>     
> it
>   
>> is?  But that doesn't mean that the Ethernet dissector's heuristic
>> registration shouldn't be fixed nonetheless ... if indeed it's
>>     
> improper
>   
>> behavior.  I think it is but I have left the question open for the
>> author of that dissector or the core Wireshark developers to answer.
>>
>> - Chris
>>
>> -----Original Message-----
>> From: wireshark-dev-bounces@xxxxxxxxxxxxx
>> [mailto:wireshark-dev-bounces@xxxxxxxxxxxxx] On Behalf Of Michael A.
>> McCartney
>> Sent: Monday, December 17, 2007 9:14 PM
>> To: Developer support list for Wireshark
>> Subject: Re: [Wireshark-dev] Register dissector to MAC address
>>
>> Chris,
>>
>> I used to hack into packet-eth.c until I learned
>> a better way using heuristic dissector instead
>> and leave packet-eth.c alone.  Not sure why you
>> had difficulties but this is what I did and it
>> works fine.  And using the if(...), one can be
>> selective on MAC address.
>>
>> static gboolean
>> dissect_<name>_heur (tvbuff_t *tvb, packet_info *pinfo, proto_tree
>> *tree)
>> {
>>     /*
>>      * Is a <name or target> ethernet header?
>>      */
>>     if( tvb_get_guint8(tvb, 1) == 0x00 &&
>>         tvb_get_guint8(tvb, 2) == 0x00 &&
>>         tvb_get_guint8(tvb, 3) == 0x00 &&
>>           ... etc ...
>>         tvb_get_guint8(tvb,11) == 0x00 &&
>>         tvb_get_guint8(tvb,12) == 0x00 )
>>     {
>>         /* dissect <name> frame */
>>         dissect_<name>(tvb, pinfo, tree);
>>         return TRUE;
>>     }
>>     else
>>     {
>>         /* not a <name> ethernet packet header */
>>         return FALSE;
>>     }
>> }
>>
>> And in proto_reg_handoff_<name>
>> had this line...
>>
>>   heur_dissector_add("eth", dissect_<name>_heur, proto_<name>);
>>
>> Of course, doing this way, you need to dissect the
>> whole ethernet frame yourself including the MACs.
>>
>> Thanks-Mike
>>
>>
>> Maynard, Chris wrote:
>>   
>>     
>>> At first glance, packet-eth.c seems to have heuristic support, but it
>>>     
>>>       
>> doesn't appear to work, at least not how I expected it to.  For
>>     
> example,
>   
>> originally for the WOL dissector, I registered as I do for UDP,
>>     
> namely:
>   
>>   
>>     
>>>     heur_dissector_add("eth", dissect_wol, proto_wol);
>>>  
>>> But registering it that way didn't work for me, so it's been changed
>>>     
>>>       
>> to:
>>   
>>     
>>>     dissector_add("ethertype", ETHERTYPE_WOL, wol_handle);
>>>  
>>> I didn't dig too deeply into why it failed since I had a reasonable
>>>     
>>>       
>> alternative, but I suppose I should have.  It now seems to me to be a
>> bug in packet-eth.c, but I'm not entirely sure, based on the comments
>>     
> in
>   
>> the code.
>>   
>>     
>>>  
>>> First, compare the way a dissector like packet-udp.c tries the
>>>     
>>>       
>> heuristic dissectors, using the "next_tvb":
>>   
>>     
>>>     next_tvb = tvb_new_subset(tvb, offset, len, reported_len);
>>>     if (dissector_try_heuristic(heur_subdissector_list, next_tvb,
>>>     
>>>       
>> pinfo, tree))
>>   
>>     
>>> Now look at how packet-eth.c does it:
>>>     if (dissector_try_heuristic(heur_subdissector_list, tvb, pinfo,
>>>     
>>>       
>> parent_tree))
>>   
>>     
>>>         goto end_of_eth;
>>>
>>> Notice that there's no "next_tvb".  I assumed that this was
>>>     
>>>       
>> intentional when I looked at it before, but now I'm not so sure.  A
>>     
> bug?
>   
>> It now sure looks like it to me.  I couldn't find any other dissectors
>> that try to heuristically register to "eth" as I tried above.  Perhaps
>> because it doesn't work?  If it is a bug, then once that's corrected,
>> then that would be the better way to register both WOL and the
>>     
> original
>   
>> poster's dissector - heuristically.
>>   
>>     
>>>  
>>> - Chris
>>>
>>> ________________________________
>>>
>>> From: wireshark-dev-bounces@xxxxxxxxxxxxx on behalf of Stephen Fisher
>>> Sent: Mon 11/12/2007 12:50 PM
>>> To: Developer support list for Wireshark
>>> Subject: Re: [Wireshark-dev] Register dissector to MAC address
>>>
>>>
>>>
>>> On Mon, Nov 12, 2007 at 12:37:10PM -0500, Maynard, Chris wrote:
>>>
>>>   
>>>     
>>>       
>>>> Can anyone think of a reason NOT to add heuristic dissection support
>>>> to packet-eth.c?  Or does anyone have a better/alternate way to
>>>>         
> solve
>   
>>>> this?
>>>>     
>>>>       
>>>>         
>>> My first thought is that the original poster's dissector could be a
>>> heuristic that checks against the MAC address when deciding whether
>>>       
> to
>   
>>> acccept the packet or not.  Does this need changes to packet-eth.c?
>>>     
>>>       
>> I'm
>>   
>>     
>>> not sure, but could find out by researching the code a bit.
>>>
>>>
>>> Steve
>>>     
>>>       
>> [snip]
>>     
> [snip - again]
>
> -----------------------------------------
> This email may contain confidential and privileged material for the
> sole use of the intended recipient(s). Any review, use, retention,
> distribution or disclosure by others is strictly prohibited. If you
> are not the intended recipient (or authorized to receive for the
> recipient), please contact the sender by reply email and delete all
> copies of this message. Also, email is susceptible to data
> corruption, interception, tampering, unauthorized amendment and
> viruses. We only send and receive emails on the basis that we are
> not liable for any such corruption, interception, tampering,
> amendment or viruses or any consequence thereof.
> _______________________________________________
> Wireshark-dev mailing list
> Wireshark-dev@xxxxxxxxxxxxx
> http://www.wireshark.org/mailman/listinfo/wireshark-dev
>