Support of Aligned PER and Information Object Class

[AuthenticEshkinKot]

With this changes I can parse asn1 from 3GPP TS 36.413 and decode incoming S1 messages.
This patchset consists of two parts:

1. Patches from add support for parsing Information Object and Information Object Set. #99, which brings support of Information Object Class.
2. Patches from https://github.com/osmocom/asn1c/tree/aper-prefix, which brings support of Aligned PER. Unfortunately, I was unable to apply them "as is", so I made necessary fixes and that's why they are signed off by me. But I've saved original messages so it's easy to find corresponding commits.

[mouse07410]

How does this PR measure up to #125 ? Considering that #125 not only adds APER support, but also (unlike this one) passes the CI tests - wouldn't #125 be a better option?

[AuthenticEshkinKot]

@mouse07410
This PR adds support of Information Object Class

[mouse07410]

This PR adds support of Information Object Class

I did not look into #125 beyond "does it pass the tests". If it does not support Information Object Class, perhaps it would be better for you to add that support on top of #125? Because two competing PRs doing (mostly) the same thing isn't good, and my fork already merged #125 (and in addition - #129 so larger constraints are supported). If #125 lacks Info Object support, and you add that support to it - I'll merge your code. Otherwise, you'd have to wait for @vlm to decide which one of the two to merge, and in the end it might not be yours anyway.

[AuthenticEshkinKot]

OK, I'll see what I can do

[mouse07410]

Thank you! Please feel free to take a closer look at my fork, to see what enhancements are already there, so your amount of work could be less. ;-)

[mouse07410]

@AuthenticEshkinKot also please take a look at #99 - the main reason I didn't merge it is that it fails CI. I didn't look for the cause (yet). If it's simple enough to fix, and if it addresses your needs - perhaps you could propose a change, and I'd integrate #99 with your fix?

I just don't know what's the simplest/fastest/easiest way to add the capability you need - whether it's for you to create a subset of #115 that's compatible with #125 and #129, or to fix #99 , or...

[AuthenticEshkinKot]

@mouse07410
I've got two items of news - good one and bad one.
The good news is that I've merged #99 with #125 and it can successfully parse asn1 from 3GPP TS 36.413.
The bad news is that it can't decode S1 message correctly, while #115 can do it. Using asn1c debug mode, I've found some differences between #115 and #125, which lead to this situation. I've fixed some of them, but there are many left, so I think that it would be better, if you can look at them by yourself. I've prepared some test environment which can help you to understand the reason of problem.

1. Source asn1 - https://yadi.sk/d/dSl028jW3BqzsJ. These sources can be found in 3GPP TS 36.413.
   I parse them with 'asn1c -gen-PER -S ~/asn1c/skeletons/ ~/path/to/S1AP-Constants.asn ... the rest of files'
2. Example of decoder - https://yadi.sk/d/DdCxUGWr3Br4nr. It can be compiled by gcc with -DEMIT_ASN_DEBUG (for debug output).
3. Input data for decoder - https://yadi.sk/d/B99Uc1d23BqYoL. This is APER coded data. To see correct contents of it you should use asn1c from Support of Aligned PER and Information Object Class #115 and change all 'ANY_to_type' calls to 'ANY_to_type_aper' in main_test.c

Hope, this will help.

[AuthenticEshkinKot]

And one more thing, that I forgot.
To be able to parse asn1, you should merge #99 with #125. There will be no conflicts on merge, so it will be easy to do.

[mouse07410]

First, thank you for the work you're doing.

Merging #99 and #125 - my main concern is that #99 fails the CI tests. I'm trying to avoid introducing code that does not pass the CI. By the way, this was the main reason I initially merged #125 (that passes the CI) rather than #115 (that fails the CI).

Since having support for wider constraints in PER is desirable - perhaps I need to re-consider my approach and merge #115 instead... For that to happen, I need for #115 to (a) pass the CI tests, and (b) support wider constraints (what #129 accomplishes).

Also, we now know that #125 did not address some issues resulting in failure to properly support Information Object Class. It would also be good to know if there are ASN.1 constructs supported in #125 that #115 does not deal with. I.e., I am trying to figure - are these two PRs each cover things that the other one doesn't, or does one of them (#115) support everything that the other one (#125) does?

[mouse07410]

Update

Here are some details regarding why I'm not merging #99 until it is fixed:

$ make check
. . . . .
Checking ../tests/58-param-OK.asn1 against ../tests/58-param-OK.asn1.-EF
WARNING: Parameterized type ub-name expected for ub-name at line 19 in ../tests/58-param-OK.asn1
WARNING: Parameterized type ub-name expected for ub-name at line 19 in ../tests/58-param-OK.asn1
. . . . .
Checking ../tests/97-type-identifier-SW.asn1 against ../tests/97-type-identifier-SW.asn1.-EF
WARNING: ASN.1 expression "TYPE-IDENTIFIER" at line 17 of module ASN1C-UsefulInformationObjectClasses
clashes with expression "TYPE-IDENTIFIER" at line 23 of module ModuleTypeIdentifier1 (../tests/97-type-identifier-SW.asn1).
Rename or remove either instance to resolve the conflict in ../skeletons/standard-modules/ASN1C-UsefulInformationObjectClasses.asn1
. . . . .
Checking ../tests/99-class-sample-OK.asn1 against ../tests/99-class-sample-OK.asn1.-EFprint-class-matrix
--- .tmp.check-parsing.59808.old	2017-01-30 12:21:23.000000000 -0500
+++ .tmp.check-parsing.59808.new	2017-01-30 12:21:23.000000000 -0500
@@ -13,12 +13,16 @@
     &Type	 OPTIONAL
 } WITH SYNTAX { [TYPE &Type] [WITH CODE &code] IDENTIFIED BY &id }
 
--- Class matrix has 4 entries:
+-- Class matrix has 8 entries:
 --    [             &id][           &code][           &Type]
 -- [1] request-whatever        <no entry>        <no entry> 
 -- [2]   response-stuff                 1        <no entry> 
 -- [3]       request-id                 2        SampleType 
 -- [4]     request-salt        <no entry>              Salt 
+-- [5] request-whatever        <no entry>        <no entry> 
+-- [6]   response-stuff                 1        <no entry> 
+-- [7]       request-id                 2        SampleType 
+-- [8]     request-salt        <no entry>              Salt 
 
 
 SampleClassObjectSet SAMPLE-CLASS ::= {{ IDENTIFIED BY request-whatever } | { WITH CODE 1 IDENTIFIED BY response-stuff } | { TYPE SampleType WITH CODE 2 IDENTIFIED BY request-id } | { TYPE Salt IDENTIFIED BY request-salt }}
Error while processing ../tests/99-class-sample-OK.asn1.-EFprint-class-matrix (from ../tests/99-class-sample-OK.asn1)
FAIL check-parsing.sh (exit status: 1)

Take a good look at the last error cluster - it looks like that code generates two copies of the same thing!

Until and unless all of the above are fixed - that code is going nowhere. (As I said, it was the reason I did not consider #115 for inclusion, and chose to settle on #125 as it passed all the CI checks.)

[AuthenticEshkinKot]

@mouse07410
Thank you for detailed answer!
I already have version that passes CI tests, but it's code is not good enough, so some time is needed to finish it. Then I'll take a look on #129.

[mouse07410]

It works correctly for #115.

Can you please check also my #99 or master branch?

Could you make a PR to my repo?

Certainly (at least will do my best) - if you post the URL of your repo here.

[AuthenticEshkinKot]

Your master and #99 got same problem, as @ikarso's version - they fail to decode my example of APER.
My repo (just a fork) - https://github.com/AuthenticEshkinKot/asn1c.

[mouse07410]

First of all - thank you for trying #99 and master for me.

Your master and #99 got same problem, as @ikarso's version - they fail to decode my example of APER.

This is sad, and strange. I've tried both of these branches with the data in https://github.com/ikarso/3GPPTS36.413_AuthenticEshkinKot, and it all parsed/decoded beautifully.

What data were you using that failed to decode? What test-program did you use? I followed what @ikarso wrote in the README of his repo - and all worked well.

Can you please add the data that #99 fails to decode to your fork (with some description, hopefully :)? So I can pull it and try myself?

My repo (just a fork) - https://github.com/AuthenticEshkinKot/asn1c.

PR has been created. There are two conflicts.

One - in skeletons/OCTET_STRING.c caused by the change in the naming (#89, I believe). I think it's both safe and OK to choose the #115's version over your current master.

The other one is in libasn1compiler/asn1c_misc.c. Here I'm not sure which one is more correct. Can you take a look and comment? Depending on the answer, I may change my master as well.

[AuthenticEshkinKot]

@mouse07410
Finally, I've found that @ikarso's main_test.c differs from mine and has its own special ANY_to_type_aper. So, I've tested your master with it and everything is OK, it works with my APER.
When I wrote about PR to my repo I meant only one commit mouse07410@ec0ade4, not the whole branch. Sorry, it is my fault - I didn't consider this. So, I would like to reject the whole PR and get the commit.
Anyway, many thanks for your effort and patience! :)

[mouse07410]

I've tested your master with it and everything is OK, it works with my APER.

Perfect! Thanks!!

When I wrote about PR to my repo I meant only one commit mouse07410/asn1c@ec0ade4

Ha! :-)

So, I would like to reject the whole PR and get the commit.

Sure, whatever's the most convenient way.

Anyway, many thanks for your effort and patience! :)

My pleasure - and thank you for fixing #115, and helping with the rest of this thing. Without your help Info Object support wouldn't have been merged.

[ghost]

Hi, does one of you know if the APER support will be integrated into the master branch?

At the OpenairInterface project we need to use S1AP release 14 and we need the APER encoding/decoding functionality. Our current fork does not compile the version 14 of S1AP, probably because of the information object class, which you implemented after our fork.

It would actually be good for us to switch back to the main (this one here) asn1c instead of using our own version.

From what I understand, the only problem for us to do so is the support of APER (but I didn't check deeper) which is not present.

I can provide some help if you need.

Thanks and regards.

[AuthenticEshkinKot]

Hi, @cedric-roux
My fork can compile S1AP release 14. Feel free to try it.

[brchiu]

@cedric-roux , you can also try @vlm's master repository and adding #226, #234 and #238.

By the way, I am an individual member of openairinterface5g and not affiliated to any organization, can I still contribute to it ?

[ghost]

@AuthenticEshkinKot @brchiu thanks. To be honest I would prefer to see APER put into this repository. I don't want to juggle with various repositories and patch sets. I will get lost. :)

But for some particular research projects we have at OpenairInterface, that may make it. Thanks.

[soroshsabz]

ITNOA

Hi, @vlm did you any plane to merge this PR?

[badhrinathpa]

Hi, I used this 115 Pull request code, But still I am facing issue like this :
badhri85@node:~/asn1c/asn1c$ ./asn1c/asn1c -EF -P -gen-PER asnfiles/*.asn
ASN.1 grammar parse error near line 13 (token "BEGIN"): syntax error, unexpected TOK_BEGIN, expecting TOK_DEFINITIONS
Cannot parse "asnfiles/S1AP-PDU-Contents.asn"

[AuthenticEshkinKot]

Hi, @badhrinathpa !
Maybe some semicolon or comma is missed.

[badhrinathpa]

Hi @AuthenticEshkinKot
Updated the ASN file. But facing the following issue now :
FATAL: Terminal value for ProtocolIE-ContainerList->maxnoofE-RABs not found in asnfiles/specasn/S1AP-Containers.asn

[AuthenticEshkinKot]

@badhrinathpa
Add "maxnoofE-RABs" to "FROM S1AP-Constants;" section in S1AP-Containers.asn

[badhrinathpa]

@AuthenticEshkinKot Thanks for the help. That solved the max ERAB problem. But there were other issues too. I needed the APER encode/decode. So, Used the code from master branch of @mouse07410 and used the command :
asn1c -fcompound-names -fno-include-deps -gen-PER -findirect-choice ../*.asn
The files got generated now.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ AuthenticEshkinKot
✅ KrzysztofW
❌ zhanglei002
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[mouse07410]

@AuthenticEshkinKot I see you've made updates and applied some fixes to this PR - could you please check it against my fork https://github.com/mouse07410/asn1c.git (which merged the original #115), branch vlm_master - and rebase those fixes that aren't there yet?

Thanks!