search

openwsn-berkeley / dissectors

6TiSCH/IEEE802.15.4e Wireshark dissectors
Other
9 stars 7 forks source link

Fixing missing pan compression cases introduced in IEEE-802.15.4e in Table 2a #4

Closed mstaflex closed 9 years ago

mstaflex commented 9 years ago

As described in #3 I ran into a pan compression situation that was apparently not yet covered by the dissector.

In this commit I introduce a condition to catch the cases that were introduced by IEEE-802.15.4e and which are described in the IEEE-802.15.4e in Table 2a.

This should fix #3.

twatteyne commented 9 years ago

@mstaflex , thanks a million!!

@jmmunoz86 , do you want to test and merge this one?

mstaflex commented 9 years ago

I should mention that this fix is aligned with the PAN compression description in the STD document from 2012. I am following the discussion on the mailing list, but understand that the discussed obvious flaws and proposed fixes are not final yet..?

twatteyne commented 9 years ago

@mstaflex , +1, things are not very clear, as indicated in http://www.ietf.org/mail-archive/web/6tisch/current/msg03786.html. I suggest we stick to -2012 until this becomes clearer.

jmmunoz86 commented 9 years ago

yes, I'll check it now.

2015-07-01 16:46 GMT+02:00 Thomas Watteyne notifications@github.com:

@mstaflex https://github.com/mstaflex , thanks a million!!

@jmmunoz86 https://github.com/jmmunoz86 , do you want to test and merge this one?

— Reply to this email directly or view it on GitHub https://github.com/openwsn-berkeley/dissectors/pull/4#issuecomment-117703370 .

Jonathan Muñoz

twatteyne commented 9 years ago

Thanks!

twatteyne commented 9 years ago

@mstaflex , We have been discussing this issue quite a bit, especially in light of http://www.ietf.org/mail-archive/web/6tisch/current/msg03786.html.

There appears to be a glitch in the IEEE spec, and I'd rather we don't rely on work-in-progress at the IEEE.

What we have been doing in the OpenWSN team is to follow the very clear IEEE802.15.4-2011 meaning of the "PAN ID compression" field (5.2.1.1.5).

In our implementation, we always set both source and destination addresses (short of long format, it doesn't matter), AND set the "PAN ID compression" to 1 to indicate that only one PANID field is present in the IEEE802.15.4 header.

This is also what @jmmunoz86 is doing in these dissectors. My advice would be to do this until IEEE802.15.4-2015 is out and clearly states what we should do.

As a consequence, I don't know what to do with this PR.

mstaflex commented 9 years ago

@twatteyne ,

thank you for the clarification of the current situation.

The biggest problem with my PR is that your convention with using both addresses + PAN ID compression leads to a dissect that assumes no PAN IDs included at all (last line in the table).

This renders the changes useless for the time being (and especially the Plugfest).

In order to not pollute your PR-section I would suggest you reject this PR. Once the IEEE802.15.4-2015 is out the dissectors need additional care anyhow.

I will keep my changes in my fork and to be able to come back to them once the new version is out.

One last thing: It might be a good idea to put a hint in the dissector code itself to keep others from investing time to fix an apparent short-coming of the implementation. (just like I did :) )