Hi!
This patch applies on top of the wireshark/0001-abis_oml.patch.
It converts the C99 structure initialization which is not accepted by the wireshark developers (Harald told me that they need it to compile with non-gcc compilers which don't always support C99).
I have tested it here with four pcap files that Harald passed to me. It works fine.
BTW, *with* and *without* this patch, wireshark with dissector spots warnings like:
18:14:15 Warn Dissector bug, protocol A-bis OML, in packet 13: packet-gsm_abis_oml.c:940: failed assertion "DISSECTOR_ASSERT_NOT_REACHED"
It seems that some TLVs that appear in the body of packets are unknown. I'll debug which are the complaining tags and get back you.
Hi,
It seems that some TLVs that appear in the body of packets are unknown. I'll debug which are the complaining tags and get back you.
Mmm, OML has a bunch of 'vendor-specific' extensions so it would not surprise me if that's what you're hitting.
Cheers,
Sylvain
Hi pablo,
On Thu, Mar 03, 2011 at 07:01:49PM +0100, Pablo Neira Ayuso wrote:
It converts the C99 structure initialization which is not accepted by the wireshark developers (Harald told me that they need it to compile with non-gcc compilers which don't always support C99).
It's a very elegant solution to do the initialization at startup time, I like that (and didn't think of it any earlier...).
Let's hope the wireshark guys think similar!
I have tested it here with four pcap files that Harald passed to me. It works fine.
BTW, *with* and *without* this patch, wireshark with dissector spots warnings like:
18:14:15 Warn Dissector bug, protocol A-bis OML, in packet 13: packet-gsm_abis_oml.c:940: failed assertion "DISSECTOR_ASSERT_NOT_REACHED"
It seems that some TLVs that appear in the body of packets are unknown. I'll debug which are the complaining tags and get back you.
Did you enable the 'use ip.access nanoBTS' flag in the protocol preferences for the OML dissector? If you don't it will try to use the Siemens definitions on an ip.access protocol trace (which will break).
I will apply your patch right now...
Regards, Harald.
On 03/03/11 23:00, Harald Welte wrote:
On Thu, Mar 03, 2011 at 07:01:49PM +0100, Pablo Neira Ayuso wrote:
BTW, *with* and *without* this patch, wireshark with dissector spots warnings like:
18:14:15 Warn Dissector bug, protocol A-bis OML, in packet 13: packet-gsm_abis_oml.c:940: failed assertion "DISSECTOR_ASSERT_NOT_REACHED"
It seems that some TLVs that appear in the body of packets are unknown. I'll debug which are the complaining tags and get back you.
Did you enable the 'use ip.access nanoBTS' flag in the protocol preferences for the OML dissector? If you don't it will try to use the Siemens definitions on an ip.access protocol trace (which will break).
Yes, the nanoBTS mode is enabled.
I've been debugging this a bit. The problem seems to be related with "Activate Software" and "Activate Software ACK" messages. They contain an attribute whose tag is 0x42 which the dissector does not know how to interpret.
Any clue on this?
Hi Pablo,
I've now submitted the 0001-abis_oml.patch (after removing some dead code) to wireshark, see https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5784 for details (I suggest you add yourself to Cc).
On Sun, Mar 06, 2011 at 02:16:43PM +0100, Pablo Neira Ayuso wrote:
18:14:15 Warn Dissector bug, protocol A-bis OML, in packet 13: packet-gsm_abis_oml.c:940: failed assertion "DISSECTOR_ASSERT_NOT_REACHED"
It seems that some TLVs that appear in the body of packets are unknown. I'll debug which are the complaining tags and get back you.
Did you enable the 'use ip.access nanoBTS' flag in the protocol preferences for the OML dissector? If you don't it will try to use the Siemens definitions on an ip.access protocol trace (which will break).
Yes, the nanoBTS mode is enabled.
I've been debugging this a bit. The problem seems to be related with "Activate Software" and "Activate Software ACK" messages. They contain an attribute whose tag is 0x42 which the dissector does not know how to interpret.
Any clue on this?
I don't know yet, but I think it is not a blocker for submitting it. I really want to get it included. We can always fix those kind of minor bugs later.
I think the next should be 0004-rsl_ipaccess.c, but maybe make it a (default: on) preference whether or not to enable ip.access support. It has the same C99 array initializer problems.
On 24/03/11 20:17, Harald Welte wrote:
I think the next should be 0004-rsl_ipaccess.c, but maybe make it a (default: on) preference whether or not to enable ip.access support. It has the same C99 array initializer problems.
working on it. Expect one patch for this soon.
Pablo,
I just tried to commit it, but it is of course an incremental patch.
I think it's best to update the patch in git, i.e. apply it using quilt, then 'quilt refresh' the patch and commit the result to our git tree.
In short: I would like to see something that I can simply apply with 'git am' to our openbsc.git repository.
Thanks.
On 03/03/11 23:03, Harald Welte wrote:
Pablo,
I just tried to commit it, but it is of course an incremental patch.
I think it's best to update the patch in git, i.e. apply it using quilt, then 'quilt refresh' the patch and commit the result to our git tree.
In short: I would like to see something that I can simply apply with 'git am' to our openbsc.git repository.
Please, find it attached.
Hi Holger,
On 06/03/11 13:26, Holger Hans Peter Freyther wrote:
On 03/06/2011 01:21 PM, Pablo Neira Ayuso wrote:
Please, find it attached.
Are you using svn diff to create the diff in wireshark?
Yes, that's why those:
-diff --git a/epan/CMakeLists.txt b/epan/CMakeLists.txt -index a22406c..0b09335 100644 ---- a/epan/CMakeLists.txt -+++ b/epan/CMakeLists.txt -@@ -586,6 +586,7 @@ set(DISSECTOR_SRC +Index: wireshark/epan/CMakeLists.txt +=================================================================== +--- wireshark.orig/epan/CMakeLists.txt 2011-03-03 14:01:07.000000000 +0100 ++++ wireshark/epan/CMakeLists.txt 2011-03-03 14:01:20.000000000 +0100 +@@ -589,6 +589,7 @@ set(DISSECTOR_SRC
are showing up. If those disturb I can regenerate with git-svn (my initial checkout of wireshark was using svn directly, sorry).
On 03/06/2011 01:41 PM, Pablo Neira Ayuso wrote:
are showing up. If those disturb I can regenerate with git-svn (my initial checkout of wireshark was using svn directly, sorry).
I assume with your help we will get this merged soon, so don't bother for this patch. The reason to have the output of git format-patch is that one could use git am -3 in the future.
On 06/03/11 13:53, Holger Hans Peter Freyther wrote:
On 03/06/2011 01:41 PM, Pablo Neira Ayuso wrote:
are showing up. If those disturb I can regenerate with git-svn (my initial checkout of wireshark was using svn directly, sorry).
I assume with your help we will get this merged soon, so don't bother for this patch. The reason to have the output of git format-patch is that one could use git am -3 in the future.
Indeed, after all this I wanted to ask you if you're OK if I proceed with the submission to wireshark ;-).
Still, I think that I've got issue with a warning that spots a dissector bug.
BTW, the output of the patch is in git format, so I think that you can apply it with git am. It's the inner patch (which is contained inside this patch) that it's in svn format 8-).
On 03/06/2011 02:20 PM, Pablo Neira Ayuso wrote:
Still, I think that I've got issue with a warning that spots a dissector bug.
Yes, I think this was there before as well and on top of this I have never tried the randpkt to fuzz the dissector as mentioned in doc/README.developer.
On 06/03/11 14:45, Holger Hans Peter Freyther wrote:
On 03/06/2011 02:20 PM, Pablo Neira Ayuso wrote:
Still, I think that I've got issue with a warning that spots a dissector bug.
Yes, I think this was there before as well and on top of this I have never tried the randpkt to fuzz the dissector as mentioned in doc/README.developer.
I didn't know about this, I'll check it.
From a quick look at GSM 12.21, a trace I have, it looks like NM_ATT_SW_DESCR is not in the TLV table and this explains why we end up here. It might be a good opportunity to see what else we are missing.
I'll investigate this, thanks for the clue!
BTW, the output of the patch is in git format, so I think that you can apply it with git am. It's the inner patch (which is contained inside this patch) that it's in svn format 8-).
Yes, I am talking about git am -3 for the inner part as this (wireshark svn) is the place where having a three way merge is going to be the most useful thing to do. :)
Oh, I see, I didn't get it, sorry!
Thanks, applied.
I think you can try to submit the patch now to the wireshark bugzilla and take care of any comments that the wireshark developers may have.
Onci this patch is submitted, feel free to continue on the other patches (cleanup, C99-removal and submission), like: 0002-ericsson_rbs2409.patch 0003-lucent-hnb.patch 0004-rsl-ipaccess.patch
the other patches are still quite in a bit of flux, so we will probably delay them.
Regards, Harald
-
Harald Welte -
Holger Hans Peter Freyther -
Pablo Neira Ayuso -
Sylvain Munaut