vlm / asn1c

The ASN.1 Compiler
http://lionet.info/asn1c/
BSD 2-Clause "Simplified" License
1.03k stars 551 forks source link

Failed to parse S1AP and RANAP asn.1 #185

Open brchiu opened 7 years ago

brchiu commented 7 years ago

I'd like to report another problem when processing S1AP, RANAP and NBAP asn1 files with newest repository acdca4184712a92b9318f3d0fbf9d70e581361aa.

They are OK to be parsed and C code generated in 94f0b645d401f75b5b1aa8e5440dc2df0f916517

FATAL: Information Object Set S1AP-ELEMENTARY-PROCEDURES contains no objects at line 247
FATAL: Cannot compile "InitiatingMessage" (20:1) at line 224
FATAL: Cannot compile "InitiatingMessage" (20:1) at line 224
Makefile:1140: recipe for target 'regenerate-from-asn1-source' failed

FATAL: Information Object Set RANAP-ELEMENTARY-PROCEDURES contains no objects at line 233
FATAL: Cannot compile "InitiatingMessage" (20:1) at line 204
FATAL: Cannot compile "InitiatingMessage" (20:1) at line 204
Makefile:1312: recipe for target 'regenerate-from-asn1-source' failed
make: *** [regenerate-from-asn1-source] Error 70

If you need these asn.1 files, please let me know.

Thanks for your great effort !

brchiu commented 7 years ago

By the way, though S1AP and RANAP can be parsed and generate corresponding C code for S1AP and RANAP with commit 94f0b64, due to APER not support in this repository, S1AP and RANAP message can not be encoded or decoded yet.

We still need to merge APER implementation, for example #125 or #115, in the future.

brchiu commented 6 years ago

Part of problems during parsing S1AP's ASN.1 was fixed in fix_s1ap branch of my fork.

But there still be unsolved ones. It looks like current asn1c can not handle following ASN.1. A minimum excerpt is in this gist.

PROTOCOL-IES ::= CLASS {
    &id     ProtocolIE-ID   UNIQUE,
    &criticality    Criticality,
    &Value,
    &presence   Presence
}
WITH SYNTAX {
    ID              &id
    CRITICALITY     &criticality
    TYPE            &Value
    PRESENCE        &presence
}

ProtocolIE-Container {PROTOCOL-IES : IEsSetParam} ::= 
    SEQUENCE (SIZE (0..number1)) OF
    ProtocolIE-Field {{IEsSetParam}}

ProtocolIE-SingleContainer {PROTOCOL-IES : IEsSetParam} ::= 
    ProtocolIE-Field {{IEsSetParam}}

ProtocolIE-Field {PROTOCOL-IES : IEsSetParam} ::= SEQUENCE {
    id      PROTOCOL-IES.&id        ({IEsSetParam}),
    criticality PROTOCOL-IES.&criticality   ({IEsSetParam}{@id}),
    value       PROTOCOL-IES.&Value     ({IEsSetParam}{@id})
}
laf0rge commented 6 years ago

@brchiu You state "though S1AP and RANAP can be parsed and generate corresponding C code for S1AP and RANAP with commit 94f0b64". I can confirm this, both for RANAP and S1AP. I then did a git bisect and found that ea6635bdae9667bcf6111a25d896c556c946c11a is the offending commit that breaks parsing of the S1AP/RANAP (and likely many other 3GPP) ASN1 syntax.

Unfortunately that's a very large commit and the commit log doesn't provide a lot of explanation :/

brchiu commented 6 years ago

hi, @laf0rge,

I am quite interested in several open source projects related to telecom, e.g. your Osmocom and openairinterface... etc.

@vlm add many good and useful features in subsequent commits. For example, the ability to parse into the content of PDU instead of leaving it as ANY type there in 94f0b64. I do appreciate his great effort. I am trying to fix it in my fork on top of current master. I have some progress, very slow, but not complete.

Moreover, APER is required to decode S1AP, RANAP .... etc message. I did some merge from @mouse07410's repository. It also in the midway of work. If anyone who is familiar with APER encoding rule, perhaps he/she can take it over or review then enhance it.

laf0rge commented 6 years ago

@brchiu APER is not the difficult part, I think that's rather easy to solve. It doesn't require too much knowledge about asn1c internals. APER encoding rules are very easy, and not all that different from UPER, so the existing code can be used as template. Also, there's the hacks by Eurecom/OAI and the code by @mouse07410 so lots of point to start. The proper approach to clean this up and complete is is by means of a test suite againts which the code can be validated. There's an industry-proven UPER implementation in Erlang, and I have access to proprietary asn1 compilers, so generating trusted test data from known implementations is also an option that way. I'm happy to contribute on that side in the months to come.

However, the fact that asn1c fails to process the S1AP/RANAP syntax and has FATAL aborts is sad, particularly as it appears to be a regression. I strongly suggest you get your "minimal" sample for reproducing the bug submitted as a test case into upstream asn1c to avoid any such regressions from re-appearing. Fixing that looks like it requires quite a lot of knowledge about asn1c internals, and that's where I have no clue, sorry.

brchiu commented 6 years ago

@laf0rge,

I must say, it is a limitation but NOT an regression.

@vlm's current implementation focus on handling J2735 and 1609 specs which does not heavily use parameterized type as 3GPP specs do. So newly added features can not work on multiple instances of parameterized type .... etc. That is the current situation.

I don't have time to deal with merging of APER code simultaneously. Whether it is difficult part or not is irrelevant to me. If anyone expect S1AP .... etc to work in future asn1c, he/she had better to put effort on it.

Wish you can contribute to it.

laf0rge commented 6 years ago

On Mon, Oct 09, 2017 at 11:17:44AM +0000, Bi-Ruei, Chiu wrote:

I must say, it is a limitation but NOT an regression.

I think it depends on your point of view: If something used to work in earlier versions but now no longer works, a strong case can be made to call it a regression. But I don't think we need to argue about terminology here.

@vlm's current implementation focus on handling J2735 and 1609 specs which does not heavily use parameterized type as 3GPP specs do. So newly added features can not work on multiple instances of parameterized type .... etc. That is the current situation.

I fully understand that and I understand there is a commercial requirement/contract for @vlm to put his focus on those areas. That's natural and I'm perfectly happy that he has time to work on asn1c at all, particularly as it seems now with funding for some features. Yet, of course it is sad to see that on the way some other promising new capabilities (such as being able to compile S1AP/RANAP/HNBAP/RUA/NBAP/...) get broken again.

I don't have time to deal with merging of APER code simultaneously. Whether it is difficult part or not is irrelevant to me. If anyone expect S1AP .... etc to work in future asn1c, he/she had better to put effort on it.

I'm not saying that I'm "expecting" anyone else to do anything, or that I am entitled to any kind of feature or functionality without either working on it or paying for it being worked on.

I've just stated that I think APER is an area where I and/or the @osmocom community and/or the @sysmocom team can help, as unlike as the issue #185 it is not a complex task involving intricate knowledge on asn1c internals.

Wish you can contribute to it.

Unfortunately I'm overworked as-is with other open source cellular related work. So not right away, but within the weeks + months to come, I clearly have an interest to contribute in that area.

--

brchiu commented 6 years ago

Status Update :

With commit 3bbbd041550371e11e12d4fc56b7d204fdd8905d, ASN.1 of S1AP, RANAP, X2AP, M3AP, LPPa, PCAP, XwAP can be parsed by asn1c and C files generated can be compiled.

However, there still be problem associated with ASN.1 of NBAP due to the following syntax it uses. Specifically, asn1c is not able to handle { procedureCode id-cellSetup, ddMode fdd }.

cellSetupFDD NBAP-ELEMENTARY-PROCEDURE ::= {
    INITIATING MESSAGE      CellSetupRequestFDD
    SUCCESSFUL OUTCOME      CellSetupResponse
    UNSUCCESSFUL OUTCOME    CellSetupFailure
    MESSAGE DISCRIMINATOR   common
    PROCEDURE ID            { procedureCode id-cellSetup, ddMode fdd } 
    CRITICALITY             reject
}

Moreover, because of lacking APER codec functions for several types, e.g. OPENTYPE, in #226, I am not able to test the built s1ap-dump using S1AP messages at hand. And APER codec functions are not verified yet, it is just merged back from @mouse07410's repository.

Wish people who need this feature can jointly help to improve it.

velichkov commented 6 years ago

Hi @brchiu,

I added OpenType APER support (copy-pasted :blush: uper functions) in my repo 2be7f4b73a8f54987a5eeecaf95cc1b690b9467a I could open a PR to your repo or you could just cherry-pick this commit in your branch to include it in #226

With it I'm able the APER encode and then decode the following S1AP message (in XER format)

<S1AP-PDU>
    <initiatingMessage>
        <procedureCode>11</procedureCode>
        <criticality><ignore/></criticality>
        <value>
            <DownlinkNASTransport>
                <protocolIEs>
                </protocolIEs>
            </DownlinkNASTransport>
        </value>
    </initiatingMessage>
</S1AP-PDU>
brchiu commented 6 years ago

@velichkov , Great !

However, a previous example decoded by @mouse07410 repository shows s1ap-dump -iaper -oxer sample-S1AP-InitialUEMessage.aper result in the following output.

<S1AP-PDU>
    <initiatingMessage>
        <procedureCode>12</procedureCode>
        <criticality><ignore/></criticality>
        <value>
            00 00 05 00 08 00 02 00 33 00 1A 00 2C 2B 07 41 
            72 0B F6 32 F5 49 80 00 01 00 00 00 01 04 E0 60 
            C0 C0 00 05 02 01 D0 11 D1 52 32 F5 49 00 3D 5C 
            20 00 90 11 03 57 18 86 F1 00 43 00 06 00 32 F5 
            49 00 3D 00 64 40 08 00 32 F5 49 00 00 10 00 00 
            86 40 01 30
        </value>
    </initiatingMessage>
</S1AP-PDU>

Looks like asn1c does not traverse down to each protocolIEs field. There still be some work to do.

velichkov commented 6 years ago

However, a previous example decoded by @mouse07410 repository shows s1ap-dump -iaper -oxer sample-S1AP-InitialUEMessage.aper result in the following output.

I'm testing with the generated converter-example and with this sample-S1AP-InitialUEMessage.aper it fails with

Decoding member "value" in ProtocolIE-Field (constr_SEQUENCE.c:1597)
Type selector is not defined for Open Type ProtocolIE-Field->value->value (OPEN_TYPE.c:419)
Failed to decode element ProtocolIE-Field (OPEN_TYPE.c:420)

so it does traverse down but the IOC table in ProtocolIE-Field.c is empty and I can't figure out why.

Maybe here we need to call the process callback similar to _asn1f_foreach_unparsed_union

brchiu commented 6 years ago

It seems only IOC tables of InitiatingMessage, SuccessfulOutcome and UnsuccessfulOutcome are generated. And others are empty.

Taking aforementioned InitialUEMessage as an example, no corresponding data structure generated for the following relation.

InitialUEMessage ::= SEQUENCE {
    protocolIEs     ProtocolIE-Container   {{InitialUEMessage-IEs}},
    ...
}

InitialUEMessage-IEs S1AP-PROTOCOL-IES ::= {
    { ID id-eNB-UE-S1AP-ID  CRITICALITY reject TYPE ENB-UE-S1AP-ID  PRESENCE mandatory}|
    { ID id-NAS-PDU         CRITICALITY reject TYPE NAS-PDU         PRESENCE mandatory}|
.....
    { ID id-Coverage-Level  CRITICALITY ignore TYPE Coverage-Level  PRESENCE optional},
    ...
}

There still be some work to do.

brchiu commented 6 years ago

Part of IOC generation problems is solved in my local branch.

However, due to complex relation between so many generated types, there is cyclic inclusion of header file occurs and some generated C files from S1AP ASN.1 fail to be compiled. Specifically, generated ProtocolIE-Field.h includes a lot of header files of other types because many information object sets are instantiated by it.

ProtocolIE-Field {S1AP-PROTOCOL-IES : IEsSetParam} ::= SEQUENCE {
    id            S1AP-PROTOCOL-IES.&id            ({IEsSetParam}),
    criticality   S1AP-PROTOCOL-IES.&criticality   ({IEsSetParam}{@id}),
    value         S1AP-PROTOCOL-IES.&Value         ({IEsSetParam}{@id})
}

I think it might need a overhaul of file structure to solve this problem. For example, break up ProtocolIE-Field.h to small ones, .... etc.

velichkov commented 6 years ago

Hi @brchiu,

Part of IOC generation problems is solved in my local branch.

You have made a great progress!

However, due to complex relation between so many generated types, there is cyclic inclusion of header file occurs and some generated C files from S1AP ASN.1 fail to be compiled. I think it might need a overhaul of file structure to solve this problem. For example, break up ProtocolIE-Field.h to small ones, .... etc.

An alternative solution according to the documentation could be

   -findirect-choice
      When  generating code for a CHOICE type, compile the CHOICE mem-
      bers as indirect pointers instead of declaring them inline. Con-
      sider  using this option together with -fno-include-deps to pre-
      vent circular references.

I just tried with both options and the cyclic inclusion is removed at least for S1AP but the compilation fails with

cc -DASN_DISABLE_OER_SUPPORT  -DPDU=S1AP_PDU -DASN_EMIT_DEBUG=1 -I. -g3 -O0 -o ProtocolIE-Field.o -c ProtocolIE-Field.c
ProtocolIE-Field.c:15995:1: error: redefinition of ‘memb_id_constraint_0’
 memb_id_constraint_0(const asn_TYPE_descriptor_t *td, const void *sptr,
 ^~~~~~~~~~~~~~~~~~~~
ProtocolIE-Field.c:15977:1: note: previous definition of ‘memb_id_constraint_0’ was here
 memb_id_constraint_0(const asn_TYPE_descriptor_t *td, const void *sptr,
 ^~~~~~~~~~~~~~~~~~~~
ProtocolIE-Field.c:16001:1: error: redefinition of ‘memb_criticality_constraint_0’
 memb_criticality_constraint_0(const asn_TYPE_descriptor_t *td, const void *sptr,
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ProtocolIE-Field.c:15983:1: note: previous definition of ‘memb_criticality_constraint_0’ was here
 memb_criticality_constraint_0(const asn_TYPE_descriptor_t *td, const void *sptr,
brchiu commented 6 years ago

hi, @velichkov,

Many thanks for your suggestion. :+1: I use my newest fix_s1ap_code_generation_2, with the command line option -fcompound-names -fno-include-deps -gen-PER -findirect-choice, the generated C files from S1AP ASN.1 get compiled !!

Below is the first several lines of my Makefile for s1ap for your reference

ASN_PROGRAM = s1ap-dump
CFLAGS += -DASN_CONVERTER_TITLE="S1AP decoder" -DHAVE_CONFIG_H -DJUNKTEST -D_DEFAULT_SOURCE
begin: S1AP-PDU.c maybe-wip-pause all

-include Makefile.am.example

S1AP-PDU.c: ../sample.makefile.regen ../s1ap-14.4.0.asn1
    make regen-makefile
    @touch S1AP-PDU.c
    make

regen-makefile:
    TITLE="S1AP decoder" \
    ASN_CMDOPTS="-fcompound-names -fno-include-deps -gen-PER -findirect-choice" \
    ASN_MODULES="../s1ap-14.4.0.asn1" \
    ASN_PDU=S1AP-PDU \
    ASN_PROGRAM=s1ap-dump \
    ../sample.makefile.regen

However, after merged with #226, s1ap-dump still fail to decode the sample-S1AP-InitialContextSetup.aper under mouse07410's examples/sample.source.S1AP directory, which I created from the content of certain wireshark capture file. Error message is here

Based on the following excerpt,

Decoding member "uEaggregateMaximumBitRateDL" in UEAggregateMaximumBitrate (constr_SEQUENCE.c:1597)
Integer with range 34 bits (INTEGER.c:849)
Can encode 34 (5 bytes) in 3 bits (INTEGER.c:863)
  [PER got  3<=72 bits => span 11 +1[3..72]:05 (69) => 0x0] (asn_bit_data.c:139)
Got length 1 (INTEGER.c:870)
Aligning 5 bits (per_support.c:301)
  [PER got  5<=69 bits => span 16 +1[8..72]:05 (64) => 0x5] (asn_bit_data.c:139)
Failed to decode element BitRate (INTEGER.c:872)
Failed decode uEaggregateMaximumBitRateDL in UEAggregateMaximumBitrate (constr_SEQUENCE.c:1607)

Then checking s1ap-14.4.0.asn, BitRate indeed needs 34 bits.

BitRate ::= INTEGER (0..10000000000) 

And in asn_bit_data.c, it is incapable of getting more than 31 bits.

    /*
     * Extract specified number of bits.
     */
    if(off <= 8)
        accum = nbits ? (buf[0]) >> (8 - off) : 0;
    else if(off <= 16)
        accum = ((buf[0] << 8) + buf[1]) >> (16 - off);
    else if(off <= 24)
        accum = ((buf[0] << 16) + (buf[1] << 8) + buf[2]) >> (24 - off);
    else if(off <= 31)
        accum = (((uint32_t)buf[0] << 24) + (buf[1] << 16)
            + (buf[2] << 8) + (buf[3])) >> (32 - off);
    else if(nbits <= 31) {
        asn_bit_data_t tpd = *pd;
        /* Here are we with our 31-bits limit plus 1..7 bits offset. */
        asn_get_undo(&tpd, nbits);
        /* The number of available bits in the stream allow
         * for the following operations to take place without
         * invoking the ->refill() function */
        accum  = asn_get_few_bits(&tpd, nbits - 24) << 24;
        accum |= asn_get_few_bits(&tpd, 24);
    } else {
        asn_get_undo(pd, nbits);
        return -1;
    }

Can any one have a solution to this ? Looks like we are close to the goal.

velichkov commented 6 years ago

I use my newest branch head : commit 6ab95f54, with the command line option -fcompound-names -fno-include-deps -gen-PER -findirect-choice, the generated C files from S1AP ASN.1 get compiled !!

:tada: Yeah, that's really a great news. Many thanks for your great efforts !!! I just extracted again the asn1 from the 36413-e40.doc and I'm able to compile it as well.

still fail to decode the sample-S1AP-InitialContextSetup.aper under mouse07410's examples/sample.source.S1AP directory, which I created from the content of certain wireshark capture file

Could you upload the pcap file if you still have it?

Can any one have a solution to this ?

I will try to fix it this weekend.

brchiu commented 6 years ago

I try to decode other S1AP sample messages available, i.e. sample-S1AP-InitialUEMessage.aper, sample-S1AP-InitialUEMessage2.aper and sample-S1AP-InitiatingMessage.aper, they all can be decoded.

:tada: :tada: :tada:

Below is an example :

$ ./s1ap-dump -per-nopad -iaper sample-S1AP-InitialUEMessage.aper
<S1AP-PDU>
    <initiatingMessage>
        <procedureCode>12</procedureCode>
        <criticality><ignore/></criticality>
        <value>
            <InitialUEMessage>
                <protocolIEs>
                    <ProtocolIE-Field>
                        <id>8</id>
                        <criticality><reject/></criticality>
                        <value>
                            <ENB-UE-S1AP-ID>51</ENB-UE-S1AP-ID>
                        </value>
                    </ProtocolIE-Field>
                    <ProtocolIE-Field>
                        <id>26</id>
                        <criticality><reject/></criticality>
                        <value>
                            <NAS-PDU>
                                07 41 72 0B F6 32 F5 49 80 00 01 00 00 00 01 04 
                                E0 60 C0 C0 00 05 02 01 D0 11 D1 52 32 F5 49 00 
                                3D 5C 20 00 90 11 03 57 18 86 F1
                            </NAS-PDU>
                        </value>
                    </ProtocolIE-Field>
                    <ProtocolIE-Field>
                        <id>67</id>
                        <criticality><reject/></criticality>
                        <value>
                            <TAI>
                                <pLMNidentity>32 F5 49</pLMNidentity>
                                <tAC>00 3D</tAC>
                            </TAI>
                        </value>
                    </ProtocolIE-Field>
                    <ProtocolIE-Field>
                        <id>100</id>
                        <criticality><ignore/></criticality>
                        <value>
                            <EUTRAN-CGI>
                                <pLMNidentity>32 F5 49</pLMNidentity>
                                <cell-ID>
                                    0000000000000000000100000000
                                </cell-ID>
                            </EUTRAN-CGI>
                        </value>
                    </ProtocolIE-Field>
                    <ProtocolIE-Field>
                        <id>134</id>
                        <criticality><ignore/></criticality>
                        <value>
                            <RRC-Establishment-Cause><mo-Signalling/></RRC-Establishment-Cause>
                        </value>
                    </ProtocolIE-Field>
                </protocolIEs>
            </InitialUEMessage>
        </value>
    </initiatingMessage>
</S1AP-PDU>

As for the wireshark captures, I don't remember where I downloaded them, perhaps from Wireshark's wiki but they do not exist any more. We should have them for validating the decoding result. :cry:

As for these binary *.aper files, you can have it at @mouse07410's repository.

UPDATE: I have sent pull request #234 and the binary files are there.

@laf0rge, we have some progress for S1AP message processing. Could you kindly provide some S1AP and other 3GPP protocol messages for basic testing ? Or tell us where we can access them ?

brchiu commented 6 years ago

One memory leak in s1ap-dump might need be solved. It looks like a general problem in converter-examples.c.

==27543== Memcheck, a memory error detector
==27543== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==27543== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==27543== Command: ./s1ap-dump -per-nopad -iaper sample-S1AP-InitialUEMessage.aper
==27543== Parent PID: 2039
==27543== 
==27543== 
==27543== HEAP SUMMARY:
==27543==     in use at exit: 8,192 bytes in 1 blocks
==27543==   total heap usage: 40 allocs, 39 frees, 15,557 bytes allocated
==27543== 
==27543== 8,192 bytes in 1 blocks are still reachable in loss record 1 of 1
==27543==    at 0x4C2FA3F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27543==    by 0x4C31D84: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27543==    by 0x19E847: data_decode_from_file (in /media/brchiu/DATA/DocCollect/SoftwareTech/asn1c/asn1c_br3/examples/sample.source.S1AP/s1ap-dump)
==27543==    by 0x19D7B9: main (in /media/brchiu/DATA/DocCollect/SoftwareTech/asn1c/asn1c_br3/examples/sample.source.S1AP/s1ap-dump)
==27543== 
==27543== LEAK SUMMARY:
==27543==    definitely lost: 0 bytes in 0 blocks
==27543==    indirectly lost: 0 bytes in 0 blocks
==27543==      possibly lost: 0 bytes in 0 blocks
==27543==    still reachable: 8,192 bytes in 1 blocks
==27543==         suppressed: 0 bytes in 0 blocks
==27543== 
==27543== For counts of detected and suppressed errors, rerun with: -v
==27543== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
velichkov commented 6 years ago

Hi @brchiu,

About the failure to decode sample-S1AP-InitialContextSetup.aper if you comment out lines 1557 - 1572 in const_SEQUENCE.c, then it will be decoded successfully.

https://github.com/vlm/asn1c/blob/ddbabf409349f7767e9b27ee5a247514eed64b18/skeletons/constr_SEQUENCE.c#L1557-L1572

This code was added in mouse07410/asn1c#29 in commits 569eeaff4 and 12850abc0

brchiu commented 6 years ago

Hi, @velichkov,

Many thanks for your advice. It works !

mouse07410 commented 6 years ago

I believe those lines were added to address a problem, https://github.com/mouse07410/asn1c/issues/30 to be specific.

Does it apply to the current master here? If so, should anything be done about it?

velichkov commented 6 years ago

Hi @mouse07410,

I believe those lines were added to address a problem, mouse07410#30 to be specific.

Yes, I know.

Does it apply to the current master here?

Yes, it does.

If so, should anything be done about it?

Yes, someone needs to implement a proper fix. The master in your fork is currently broken as well and cannot decode this S1AP parameter and other SEQUENCEs as well (see mouse07410/asn1c#31). It seems you shouldn't have accepted this fix as it is neither correct nor complete, there is no equivalent code in SEQUENCE_encode_aper so the encode/decode is not symmetrical.

My priority right now is to help @brchiu in his efforts to improve IOC support.

mouse07410 commented 6 years ago

My priority right now is to help @brchiu in his efforts to improve IOC support.

OK, makes sense. Thanks!

brchiu commented 6 years ago

@velichkov , I added one commit to #234. It uses the information object set, which instantiating a parameterized type, at least for ProtocolIE-Field as an example, as part of generated name, please refer to https://github.com/vlm/asn1c/pull/234/commits/6b7bbccb07d8ecf8a9688c3cc15fbd43038287f7.

My next dream about this issue is to remove the need of -findirect-choice, which can makes

/* Forward declarations */
struct ImmediateMDT;
struct LoggedMDT;
struct MDTMode_Extension;

/* MDTMode */
typedef struct MDTMode {
    MDTMode_PR present;
    union MDTMode_u {
        struct ImmediateMDT *immediateMDT;
        struct LoggedMDT    *loggedMDT;
        /*
         * This type is extensible,
         * possible extensions are below.
         */
        struct MDTMode_Extension    *mDTMode_Extension;
    } choice;

    /* Context for parsing across buffer boundaries */
    asn_struct_ctx_t _asn_ctx;
} MDTMode_t;

back to

/* MDTMode */
typedef struct MDTMode {
    MDTMode_PR present;
    union MDTMode_u {
        ImmediateMDT_t   immediateMDT;
        LoggedMDT_t  loggedMDT;
        /*
         * This type is extensible,
         * possible extensions are below.
         */
        MDTMode_Extension_t  mDTMode_Extension;
    } choice;

    /* Context for parsing across buffer boundaries */
    asn_struct_ctx_t _asn_ctx;
} MDTMode_t;

which is more friendly for developers.

I am still thinking how to achieve it with minimum modification.

laf0rge commented 6 years ago

@brchiu sorry for ultra late response. You can find some RUA/HNBAP/RANAP pcap files at http://git.osmocom.org/osmo-iuh/tree/pcap We can also generally create more, given that we have a working open source Iuh and IuCS / IuPS implementation in OsmoHNBGW, OsmoMSC and OsmoSGSN, as well as related small cell hardware in the Osmocom project.

acetcom commented 6 years ago

One of my S1-Reset Packets cannot be decoded. My work is as follows.

Please let me know if I misunderstand something, and my apologies for that case.

brchiu commented 6 years ago

@acetcom, sorry, after analysed with gdb, it is out of my capability to figure out what's wrong.

It seems that APER decoding might not work properly in this case, however, I am not able to debug it.

Waiting for people with better knowledge of APER to fix it.

Following is the stackframe for people who is willing to dig into it.

aper_open_type_get_simple (ctx=0x7fffffffd9e8, td=0x5555558ffc00 <asn_DEF_ResetType>, constraints=0x0, 
    sptr=0x7fffffffd208, pd=0x7fffffffd660) at per_opentype.c:449
449     if(rv.code == RC_OK) {
(gdb) n
451         padding = spd.nbits - spd.nboff;
(gdb) n
452                 if ((padding < 8 ||
(gdb) n
454         (spd.nboff == 0 && spd.nbits == 8 && spd.buffer == buf)) &&
(gdb) n
452                 if ((padding < 8 ||
(gdb) n
460         FREEMEM(buf);
(gdb) n
461         if(padding >= 8) {
(gdb) n
462             ASN_DEBUG("Too large padding %d in open type", (int)padding);
(gdb) p padding 
$64 = 112
(gdb) n
463             ASN__DECODE_FAILED;
(gdb) bt
#0  aper_open_type_get_simple (ctx=0x7fffffffd9e8, td=0x5555558ffc00 <asn_DEF_ResetType>, constraints=0x0, 
    sptr=0x7fffffffd208, pd=0x7fffffffd660) at per_opentype.c:463
#1  0x0000555555681fec in aper_open_type_get (ctx=0x7fffffffd9e8, td=0x5555558ffc00 <asn_DEF_ResetType>, constraints=0x0, 
    sptr=0x7fffffffd208, pd=0x7fffffffd660) at per_opentype.c:514
#2  0x000055555564783e in OPEN_TYPE_aper_get (opt_codec_ctx=0x7fffffffd9e8, td=0x555555941c60 <asn_DEF_ResetIEs>, 
    sptr=0x55555597e6a0, elm=0x555555941bf0 <asn_MBR_ResetIEs_221+176>, pd=0x7fffffffd660) at OPEN_TYPE.c:453
#3  0x00005555556603f4 in SEQUENCE_decode_aper (opt_codec_ctx=0x7fffffffd9e8, td=0x555555941c60 <asn_DEF_ResetIEs>, 
    constraints=0x0, sptr=0x7fffffffd440, pd=0x7fffffffd660) at constr_SEQUENCE.c:1602
#4  0x0000555555668679 in SET_OF_decode_aper (opt_codec_ctx=0x7fffffffd9e8, 
    td=0x555555927d00 <asn_DEF_ProtocolIE_Container_6559P37>, constraints=0x0, sptr=0x7fffffffd530, pd=0x7fffffffd660)
    at constr_SET_OF.c:1161
#5  0x0000555555660448 in SEQUENCE_decode_aper (opt_codec_ctx=0x7fffffffd9e8, td=0x5555558ffa00 <asn_DEF_Reset>, 
    constraints=0x0, sptr=0x7fffffffd748, pd=0x7fffffffd660) at constr_SEQUENCE.c:1604
#6  0x0000555555681b64 in aper_open_type_get_simple (ctx=0x7fffffffd9e8, td=0x5555558ffa00 <asn_DEF_Reset>, 
    constraints=0x0, sptr=0x7fffffffd748, pd=0x7fffffffda00) at per_opentype.c:446
#7  0x0000555555681fec in aper_open_type_get (ctx=0x7fffffffd9e8, td=0x5555558ffa00 <asn_DEF_Reset>, constraints=0x0, 
    sptr=0x7fffffffd748, pd=0x7fffffffda00) at per_opentype.c:514
#8  0x000055555564783e in OPEN_TYPE_aper_get (opt_codec_ctx=0x7fffffffd9e8, 
    td=0x5555558f8c40 <asn_DEF_InitiatingMessage>, sptr=0x55555597e4e0, 
    elm=0x5555558f8bd0 <asn_MBR_InitiatingMessage_1+176>, pd=0x7fffffffda00) at OPEN_TYPE.c:453
#9  0x00005555556603f4 in SEQUENCE_decode_aper (opt_codec_ctx=0x7fffffffd9e8, 
    td=0x5555558f8c40 <asn_DEF_InitiatingMessage>, constraints=0x0, sptr=0x55555597e4b8, pd=0x7fffffffda00)
    at constr_SEQUENCE.c:1602
#10 0x00005555556587bf in CHOICE_decode_aper (opt_codec_ctx=0x7fffffffd9e8, td=0x5555558f7340 <asn_DEF_S1AP_PDU>, 
    constraints=0x0, sptr=0x7fffffffdaa8, pd=0x7fffffffda00) at constr_CHOICE.c:1083
#11 0x000055555567ecbf in aper_decode (opt_codec_ctx=0x7fffffffd9e8, td=0x5555558f7340 <asn_DEF_S1AP_PDU>, 
    sptr=0x7fffffffdaa8, buffer=0x55555597b490, size=41, skip_bits=0, unused_bits=0) at per_decoder.c:171
#12 0x00005555555f5758 in data_decode_from_file (isyntax=ATS_ALIGNED_BASIC_PER, 
    pduType=0x5555558f7340 <asn_DEF_S1AP_PDU>, file=0x55555597b260, 
    name=0x7fffffffe18d "./s1reset_three_part_of_s1_interface.bin", suggested_bufsize=8192, on_first_pdu=1)
    at converter-example.c:852
#13 0x00005555555f44d8 in main (ac=1, av=0x7fffffffdd80) at converter-example.c:474
(gdb) 
acetcom commented 6 years ago

@brchiu Thank you for your effort.

I will leave a little story with this.

About a year ago, I have started to learn asn1c for NextEPC. With the help of #115 and asn1tostruct.py (OAI), I was able to create an S1AP encoder/decoder.

However, I realized that implementing the S1-Reset feature does not work right now. Of course, I could workaround it, but I wanted to solve it fundamentally. So, I re-visit asn1c again and found out the great things you did.

I am a little worried about how to modify NextEPC encoder module, but I will try to apply it to the NextEPC.

For reference, testepc.pcapng

Thanks again!

velichkov commented 6 years ago

Hi @acetcom, @brchiu,

To resolve the problem with s1reset_three_part_of_s1_interface.bin apply 25da4e3d90e827f70c31b3a4833fe31ef56401ce

When decoding a SEQUENCE OF with less then 256 the alignment bits were not read and because of this the decoded length was wrong

<S1AP-PDU>
    <initiatingMessage>
        <procedureCode>14</procedureCode>
        <criticality><reject/></criticality>
        <value>
            <Reset>
                <protocolIEs>
                    <ResetIEs>
                        <id>2</id>
                        <criticality><ignore/></criticality>
                        <value>
                            <Cause>
                                <radioNetwork><release-due-to-eutran-generated-reason/></radioNetwork>
                            </Cause>
                        </value>
                    </ResetIEs>
                    <ResetIEs>
                        <id>92</id>
                        <criticality><reject/></criticality>
                        <value>
                            <ResetType>
                                <partOfS1-Interface>
                                    <ProtocolIE-SingleContainer>
                                        <id>91</id>
                                        <criticality><reject/></criticality>
                                        <value>
                                            <UE-associatedLogicalS1-ConnectionItem>
                                                <mME-UE-S1AP-ID>100</mME-UE-S1AP-ID>
                                                <eNB-UE-S1AP-ID>4</eNB-UE-S1AP-ID>
                                            </UE-associatedLogicalS1-ConnectionItem>
                                        </value>
                                    </ProtocolIE-SingleContainer>
                                    <ProtocolIE-SingleContainer>
                                        <id>91</id>
                                        <criticality><reject/></criticality>
                                        <value>
                                            <UE-associatedLogicalS1-ConnectionItem>
                                                <mME-UE-S1AP-ID>99</mME-UE-S1AP-ID>
                                                <eNB-UE-S1AP-ID>3</eNB-UE-S1AP-ID>
                                            </UE-associatedLogicalS1-ConnectionItem>
                                        </value>
                                    </ProtocolIE-SingleContainer>
                                    <ProtocolIE-SingleContainer>
                                        <id>91</id>
                                        <criticality><reject/></criticality>
                                        <value>
                                            <UE-associatedLogicalS1-ConnectionItem>
                                                <mME-UE-S1AP-ID>44</mME-UE-S1AP-ID>
                                            </UE-associatedLogicalS1-ConnectionItem>
                                        </value>
                                    </ProtocolIE-SingleContainer>
                                </partOfS1-Interface>
                            </ResetType>
                        </value>
                    </ResetIEs>
                </protocolIEs>
            </Reset>
        </value>
    </initiatingMessage>
</S1AP-PDU>
acetcom commented 6 years ago

@velichkov I've tested your updated code. All S1 Reset is properly working. But, S1 Setup Request is failed. I've attached the sample pcap below.

s1setup_request.pcapng.tar.gz s1setup_request.bin.tar.gz

If you update any code, I will test it with the above testepc.pcapng.

Thanks!

velichkov commented 6 years ago

Hi @acetcom,

But, S1 Setup Request is failed.

The previous fix was definitely not correct, my second attempt is 3f5ef6d (you need to revert or drop the previous patch and apply this on top of a1537b84624a1626d6c806e6791765d837d190ef).

And if possible could you send me all messages from the testepc.pcapng extracted as a bin files.

acetcom commented 6 years ago

Hi @velichkov,

The first thing you did was reverted. And I applied your second attempt. I tried a few things and it seems to work well.

For your reference, I generated all message using NextEPC test framework. bin.tar.gz

Thanks!

brchiu commented 6 years ago

@velichkov , if your 3f5ef6d commit really works, please advice how I incorporate your commit into #226 OR you will submit a separate pull request AFTER #226 accepted. Both ways are ok for me.

Sorry, I am not that familiar with git operation for this scenario.

By the way, I slightly change this #185 that adding ASN1C_PREFIX string to members of enumerated type. For example, Cause_PR_radioNetwork of RANAP_Cause_PR now becomes RANAP_Cause_PR_radioNetwork, which is better for multiple protocols co-existence scenario.

mouse07410 commented 6 years ago

@velichkov May I ask to make it a separate PR for after #226 is merged?

And/or could you make it a PR against the vlm_master branch in my fork? That branch tracks the current vlm master, with some useful PRs (like #226) applied. I'd really appreciate if you could do it.

velichkov commented 6 years ago

Hi @brchiu,

please advice how I incorporate your commit into #226

I'm OK to incorporate this into your PR, an easy way to do it is to cherry-pick from my repository

git remote add velichkov https://github.com/velichkov/asn1c.git
git fetch velichkov
git checkout merge_mouse07410_aper_implementation
git cherry-pick 3f5ef6d
git push

@mouse07410,

@velichkov May I ask to make it a separate PR for after #226 is merged?

Ok. If @brchiu decides to not cherry-pick this change I will open a new PR once #226 gets merged.

And/or could you make it a PR against the vlm_master branch in my fork?

I will. In the future feel free to check-pick changes directly from my repo as described above.

acetcom commented 6 years ago

Hi, @velichkov

NextEPC am moving new asn1c API for all S1AP encoders/decoders. You can find the example modification in 2c1d70de3c320cdf2f03d21acbc66c4d5dd7e2ed

Unfortunately, there are some errors. See the below.

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-14.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-23.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-26.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-28.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-42.aper
<S1AP-PDU>
    <initiatingMessage>
        <procedureCode>9</procedureCode>
        <criticality><reject/></criticality>
        <value>
            <InitialContextSetupRequest>
                <protocolIEs>
...
../nextepc/sample-S1AP-PDU-42.aper: Decode failed past byte 49: Unexpected end of input

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-46.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-47.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-52.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-52.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-56.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-77.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-77.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-135.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-138.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-140.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-142.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-144.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-145.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-146.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-164.aper
Segmentation fault: 11

$ ./s1ap-dump -iaper -per-nopad ../nextepc/sample-S1AP-PDU-183.aper
Segmentation fault: 11
acetcom commented 6 years ago

Hi, @velichkov

From your s1ap branch, I've found the packet is incorrectly encoded. Let me re-check it.

Sorry to bother you

Thanks!

acetcom commented 6 years ago

Hi, @velichkov and @brchiu

I've compiled NextEPC with 32bit machine. There is an overflow error as below.

S1AP_BitRate.c:49:35: error: overflow in implicit constant conversion [-Werror=overflow]
  { APC_CONSTRAINED,  34, -1,  0,  10000000000 } /* (0..10000000000) */,
                                   ^

So, I've fixed like the followings.

diff --git a/lib/s1ap/asn1c/per_support.h b/lib/s1ap/asn1c/per_support.h
index 23079c9..15366e9 100644
--- a/lib/s1ap/asn1c/per_support.h
+++ b/lib/s1ap/asn1c/per_support.h
@@ -25,7 +25,11 @@ typedef struct asn_per_constraint_s {
        int  range_bits;                /* Full number of bits in the range */
        int  effective_bits;            /* Effective bits */
        long lower_bound;               /* "lb" value */
+#if 0 /* modified by acetcom */
        long upper_bound;               /* "ub" value */
+#else
+       c_int64_t upper_bound;         /* "ub" value */
+#endif
 } asn_per_constraint_t;
 typedef struct asn_per_constraints_s {
        asn_per_constraint_t value;

c_int64_t is NextEPC type. I don't know how to fix it in the asn1c library.

Thanks!

acetcom commented 6 years ago

Hi, @velichkov and @brchiu

As you know, I'm porting all NextEPC encoding/decoding using new API. Basically, 64bit machine is properly worked for most test case. However, encoding is not working in 32bit machine even though decoding is worked well.

The MME_UE_S1AP_ID/ENB_UE_S1AP_ID is not properly encoded. So, the encoding array cannot be decoded in the asn1c library.

Here is my sequence.

converter-example.tar.gz

000b4080 94000003 00000006 00010000 00010008 00060001 00000001 001a0079 78efefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef


- Expected Result (In 64bit machine, encoding is properly worked.)

"000b4080 8c000003 00000002 00010008 00020001 001a0079 78efefef efefefef" "efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef" "efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef" "efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef" "efefefef efefefef efefefef efefefef ef";


For your reference, the followings are updated code.

diff --git a/skeletons/converter-example.c b/skeletons/converter-example.c index 2340610..babe018 100644 --- a/skeletons/converter-example.c +++ b/skeletons/converter-example.c @@ -193,6 +193,140 @@ ats_by_name(const char name, const asn_TYPE_descriptor_t td, return NULL; }

+#include "S1AP-PDU.h" +#include "InitiatingMessage.h" +#include "DownlinkNASTransport.h" +#include "ProtocolIE-Field.h" + +int s1ap_encode_pdu(char pkbuf, int len, S1AP_PDU_t *message) +{

velichkov commented 6 years ago

Hi @acetcom,

I just pushed 3 commits in my fork - two APER fixes (fec0acc058491b60eb00cd223de246b168b69f92, f5dc5cc6358a153cd0e0da503563c595d2a2edd4) and one XER (8d06d92df6dca1b9a6dcb0aedef0f91e053823d0)

$ file ./s1ap-dump 
./s1ap-dump: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux 2.6.32, BuildID[sha1]=58db588dc25d71cb014f8c63f3f4577123e710fb, with debug_info, not stripped
$ ./s1ap-dump -t
000b4080 8c000003 00000002 00010008 00020001 001a0079 78efefef efefefef 
efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef 
efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef 
efefefef efefefef efefefef efefefef efefefef efefefef efefefef efefefef 
efefefef efefefef efefefef efefefef ef

The problems were also reproducible with make check as I've already added the PDUs you provided in my fork.

acetcom commented 6 years ago

Hi, @velichkov

Great! NextEPC 32bit version is also successfully ported with this.

For your reference, I've only changed the following code based on your s1ap branch.

acetcom@sejin123:~/Documents/git/asn1c.velichkov/skeletons$ git diff
diff --git a/skeletons/per_support.h b/skeletons/per_support.h
index 23079c9..1378030 100644
--- a/skeletons/per_support.h
+++ b/skeletons/per_support.h
@@ -25,7 +25,11 @@ typedef struct asn_per_constraint_s {
        int  range_bits;                /* Full number of bits in the range */
        int  effective_bits;            /* Effective bits */
        long lower_bound;               /* "lb" value */
+#if 0 /* modified by acetcom */
        long upper_bound;               /* "ub" value */
+#else
+       int64_t upper_bound;            /* "ub" value */
+#endif
 } asn_per_constraint_t;
 typedef struct asn_per_constraints_s {
        asn_per_constraint_t value;

The attached pcap was created by new S1AP API. s1ap-32bit.tar.gz

And also, you can find the example code in NextEPC repository.

git clone https://github.com/acetcom/nextepc
git checkout s1ap

The new NextEPC will be released after testing real eNodeB. 1~2 week might be needed.

Thank you very much!!!

Best Regards, Sukchan

mouse07410 commented 6 years ago

@velichkov what branch of your fork should I cherry-pick those three commits from (assuming it matters?)? Or maybe you can make a PR to my vlm_master branch (which will replace master fairly soon)?

velichkov commented 6 years ago

Hi @mouse07410,

@velichkov what branch of your fork should I cherry-pick those three commits from

The branch name is s1ap and is based on the brchiu's s1ap branch.

(assuming it matters?)?

Depends if you need 32bit support or not.

Or maybe you can make a PR to my vlm_master branch (which will replace master fairly soon)?

I prefer if you could merge my branch directly or review and cherry-pick some of the commits that you find useful because the whole procedure of creating additional branches, rebasing, opening PRs is tedious and time consuming.

mouse07410 commented 6 years ago

The branch name is s1ap and is based on the brchiu's s1ap branch.

Thanks!

(assuming it matters?)?

Depends if you need 32bit support or not.

Actually, what I meant to ask was - do I need to know the branch name in order to cherry-pick a commit from your repo. ;-)

It appears that I don't - cherry-picking worked OK without specifying the branch.

I prefer if you could merge my branch directly or review and cherry-pick some of the commits that you find useful...

Fair enough, at least for those more straightforward "mergers".

Could you tell me what s1ap branch is, and how it is related to your master and to vlm/master? Is this branch an enhancement that should be integrated into the main master (in your and/or my forks, if not in the upstream), or is it a modification that is necessary to deal with S1AP ASN.1, but supporting it is likely to break something else?

velichkov commented 6 years ago

Hi @mouse07410,

Could you tell me what s1ap branch is, and how it is related to your master and to vlm/master

My master is pretty out of date because I don't use it. My s1ap branch is based on @brchiu's s1ap where he has merged the IOC and APER branches needed to compile the S1AP.

Is this branch an enhancement that should be integrated into the main master (in your and/or my forks, if not in the upstream), or is it a modification that is necessary to deal with S1AP ASN.1, but supporting it is likely to break something else?

My changes are not specific to the S1AP protocol and they need to be integrated in the mainline. I saw you've already cherry-picked the three commits mentioned in the previous posts and most probably you will want a23f13febfe6d276439e36771148ae069e51254f and ca9847ddcefe0abed8bbfa10253c8d1b55a6c942 as well. The first fixes a segmentation fault while freeing OPEN_TYPE structs and with the second the -per-nopad option is no longer needed when decoding APER PDUs.

mouse07410 commented 6 years ago

My master is pretty out of date because I don't use it

Ah, OK. Good to know.

My changes are not specific to the S1AP protocol and they need to be integrated in the mainline

OK, good. I'm merging your s1ap branch (as it includes @brchiu's IOC and APER) into my vlm_master that will replace the current master (which by now became outdated, as it does not support OER). I assume that merge includes the two commits you recommended.

Thanks!

mouse07410 commented 6 years ago

@velichkov OK, merged. My vlm_master now has your s1ap. I will let you and @brchiu know when I make it the "main" master of my fork.

acetcom commented 6 years ago

Hi, @velichkov and @mouse07410

I've tested mouse07410/vlm_master branch with NextEPC test framework. It is also correctly working. But, I'd just like to know the following difference between velichkov/s1ap and mouse07410/vlm_master. Currently, NextEPC is applied with velichkov/s1ap.

acetcom@sejin123:~/Documents/git/nextepc/lib/s1ap/asn1c$ git diff INTEGER.c
diff --git a/lib/s1ap/asn1c/INTEGER.c b/lib/s1ap/asn1c/INTEGER.c
index 39bae07..11ed138 100644
--- a/lib/s1ap/asn1c/INTEGER.c
+++ b/lib/s1ap/asn1c/INTEGER.c
@@ -507,7 +507,7 @@ INTEGER__xer_body_decode(const asn_TYPE_descriptor_t *td, void *sptr,
                /* The last symbol encountered was a digit. */
         switch(asn_strtoimax_lim(dec_value_start, &dec_value_end, &dec_value)) {
         case ASN_STRTOX_OK:
-            if(specs && specs->field_unsigned && dec_value <= ULONG_MAX) {
+            if(specs && specs->field_unsigned && (uintmax_t) dec_value <= ULONG_MAX) {
                 break;
             } else if(dec_value >= LONG_MIN && dec_value <= LONG_MAX) {
                 break;
@@ -777,12 +777,16 @@ INTEGER_encode_uper(const asn_TYPE_descriptor_t *td,
                /* #11.5.6 -> #11.3 */
                ASN_DEBUG("Encoding integer %ld (%lu) with range %d bits",
                        value, value - ct->lower_bound, ct->range_bits);
-        if(per_long_range_rebase(value, ct->lower_bound, ct->upper_bound, &v)) {
-            ASN__ENCODE_FAILED;
-        }
+       if(specs && specs->field_unsigned) {
+               v = (unsigned long)value - (unsigned long)ct->lower_bound;
+       } else {
+               if(per_long_range_rebase(value, ct->lower_bound, ct->upper_bound, &v)) {
+                       ASN__ENCODE_FAILED;
+               }
+       }
         if(uper_put_constrained_whole_number_u(po, v, ct->range_bits))
-            ASN__ENCODE_FAILED;
-               ASN__ENCODED_OK(er);
+               ASN__ENCODE_FAILED;
+       ASN__ENCODED_OK(er);
        }

        if(ct && ct->lower_bound) {
mouse07410 commented 6 years ago

Ha, you noticed. ;-)

The difference you see contains:

And also it looks like indentation could be tightened up a little in that file. ;-)

acetcom commented 6 years ago

For reference, mouse07410/vlm_master is also correctly working in 32bit machine.