Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix s1ap code generation (2nd pull reuqest) #234

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

brchiu
Copy link
Contributor

@brchiu brchiu commented Nov 2, 2017

Fix most S1AP decoding issue found till now. See discussion in #185.

After merged with #226, some S1AP sample messages can be decoded.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 71.426% when pulling 72fc0f1 on brchiu:fix_s1ap_code_generation_2 into 3f52df0 on vlm:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 71.459% when pulling 6b7bbcc on brchiu:fix_s1ap_code_generation_2 into 3f52df0 on vlm:master.

@brchiu brchiu force-pushed the fix_s1ap_code_generation_2 branch 2 times, most recently from d5cfad7 to 8663b74 Compare November 15, 2017 12:52
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 71.52% when pulling 8663b74 on brchiu:fix_s1ap_code_generation_2 into 07f8c3e on vlm:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 71.52% when pulling 92fa4bd on brchiu:fix_s1ap_code_generation_2 into 07f8c3e on vlm:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 71.369% when pulling d517381 on brchiu:fix_s1ap_code_generation_2 into 34df4e5 on vlm:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 71.249% when pulling 732a58d on brchiu:fix_s1ap_code_generation_2 into 4cc779f on vlm:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 71.358% when pulling b62d34f on brchiu:fix_s1ap_code_generation_2 into 4cc779f on vlm:master.

@brchiu brchiu force-pushed the fix_s1ap_code_generation_2 branch 3 times, most recently from 6950a07 to c307df9 Compare January 15, 2018 08:01
With this modification, asn1f_parameterization_fork() will fork
parameterization if rhs_pspecs and stored rhs_pspecs have different
constraints, e.g. expressions parameterized by different
VALUESET references. Associated information object tables will be
generated accordingly.
If one item's name already exists in this OPEN TYPE, simply do not
add it to expression open_type_choice.

It slightly modifies previous commit :
dcc822a.
Set the key.sequence to eclass's _type_unique_index and increment
it in each object parsing call, then set the result back to
_type_unique_index field. It can keep a class-wise counter for
eash object instantiated.

This new method not only solve name duplication when union two
information object sets, but also avoid name duplication within
multiple instances of this class.
Solve the problem that 'TYPE OCTECT STRING' fails to generate corresponding item.

ClassObj S1AP-PROTOCOL-IES ::= {
  { ID id-eNB-UE-S1AP-ID   CRITICALITY reject   TYPE ENB-UE-S1AP-ID  PRESENCE mandatory}|
  { ID id-S1-Message       CRITICALITY reject   TYPE OCTET STRING    PRESENCE mandatory}|
  ...
}
…y row

For example,

E-RABToBeSetupItemCtxtSUReqIEs S1AP-PROTOCOL-IES ::= {
    { ID id-E-RABToBeSetupItemCtxtSUReq CRITICALITY reject TYPE E-RABToBeSetupItemCtxtSUReq PRESENCE mandatory },
    ...
}

generate :

static const asn_ioc_cell_t asn_IOS_E_RABToBeSetupItemCtxtSUReqIEs_1_rows[] = {
};
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 71.333% when pulling 545295d on brchiu:fix_s1ap_code_generation_2 into 4cc779f on vlm:master.

@coveralls
Copy link

coveralls commented Jan 16, 2018

Coverage Status

Coverage increased (+0.3%) to 71.376% when pulling 8f68def on brchiu:fix_s1ap_code_generation_2 into 4cc779f on vlm:master.

With previous commit, the following data types are generated in ProtocolIE-Field.h
for S1AP's ASN.1 :

```
typedef enum ProtocolIE_Field_6564P0__value_PR {
    ProtocolIE_Field_6564P0__value_PR_NOTHING, /* No components present */
    ProtocolIE_Field_6564P0__value_PR_E_RABToBeSetupItemBearerSUReq
} ProtocolIE_Field_6564P0__value_PR;

typedef struct ProtocolIE_Field_6564P0 {
    ProtocolIE_ID_t      id;
    Criticality_t        criticality;
    struct ProtocolIE_Field_6564P0__value {
        ProtocolIE_Field_6564P0__value_PR        present;
        union ProtocolIE_Field_6564P0__value_u {
            E_RABToBeSetupItemBearerSUReq_t      E_RABToBeSetupItemBearerSUReq;
        } choice;

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

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

```

It's difficult for developer to recognize to which ASN.1 type they are linked.
They all start with 'ProtocolIE_Field_'. It will be error-prone.

With this commit, more human comprehensible data types are generated :

```

typedef enum E_RABToBeSetupItemBearerSUReqIEs__value_PR {
    E_RABToBeSetupItemBearerSUReqIEs__value_PR_NOTHING, /* No components present */
    E_RABToBeSetupItemBearerSUReqIEs__value_PR_E_RABToBeSetupItemBearerSUReq
} E_RABToBeSetupItemBearerSUReqIEs__value_PR;

typedef struct E_RABToBeSetupItemBearerSUReqIEs {
    ProtocolIE_ID_t      id;
    Criticality_t        criticality;
    struct E_RABToBeSetupItemBearerSUReqIEs__value {
        E_RABToBeSetupItemBearerSUReqIEs__value_PR present;
        union E_RABToBeSetupItemBearerSUReqIEs__value_u {
            E_RABToBeSetupItemBearerSUReq_t               E_RABToBeSetupItemBearerSUReq;
        } choice;

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

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

```

Developers can understand they are generated from :

```

E-RABToBeSetupItemBearerSUReqIEs S1AP-PROTOCOL-IES ::= {
    { ID id-E-RABToBeSetupItemBearerSUReq CRITICALITY reject TYPE E-RABToBeSetupItemBearerSUReq PRESENCE mandatory },
    ...
}

```
Currently, there is no code generated for following ASN.1 excerpt.
Thus application is not aware of these values.

    maxnoofErrors           INTEGER ::= 256
    maxnoofBPLMNs           INTEGER ::= 6
    maxnoofPLMNsPerMME      INTEGER ::= 32
    maxnoofEPLMNs           INTEGER ::= 15
    ...

This commit generate asn_constant.h which has the following macro
defitions :

    #define maxnoofErrors (256)
    #define maxnoofBPLMNs (6)
    #define maxnoofPLMNsPerMME (32)
    #define maxnoofEPLMNs (15)
    #define maxnoofEPLMNsPlusOne (16)
    ...

It only handles ASN_BASIC_INTEGER type. Other built-in types shall
be added in the future.
Currently, there is no code generated for following ASN.1 excerpt.
Thus application is not aware of these values.

    ProtocolIE-ID ::= INTEGER (0..65535)

    id-MME-UE-S1AP-ID  ProtocolIE-ID ::= 0
    id-HandoverType    ProtocolIE-ID ::= 1
    id-Cause           ProtocolIE-ID ::= 2
    id-SourceID        ProtocolIE-ID ::= 3
      ...

    ProcedureCode ::= INTEGER (0..255)

    id-HandoverPreparation         ProcedureCode ::= 0
    id-HandoverResourceAllocation  ProcedureCode ::= 1
    id-HandoverNotification        ProcedureCode ::= 2
    id-PathSwitchRequest           ProcedureCode ::= 3
    ...

This commit adds corresponding macro definitions in
ProtocolIE-ID.h and ProcedureCode.h respectively.

    #define ProtocolIE_ID_id_MME_UE_S1AP_ID       ((ProtocolIE_ID_t)0)
    #define ProtocolIE_ID_id_HandoverType ((ProtocolIE_ID_t)1)
    #define ProtocolIE_ID_id_Cause        ((ProtocolIE_ID_t)2)
    #define ProtocolIE_ID_id_SourceID     ((ProtocolIE_ID_t)3)
    ...

    #define ProcedureCode_id_HandoverPreparation ((ProcedureCode_t)0)
    #define ProcedureCode_id_HandoverResourceAllocation    ((ProcedureCode_t)1)
    #define ProcedureCode_id_HandoverNotification ((ProcedureCode_t)2)
    #define ProcedureCode_id_PathSwitchRequest    ((ProcedureCode_t)3)
    ...

Only types of ASN_BASIC_INTEGER and ASN_BASIC_ENUMERATED referenced
by these constant variables are handled.

Other built-in types shall be added in the future.
For RANAP specification :

  RANAP-ELEMENTARY-PROCEDURE ::= CLASS {
    &InitiatingMessage,
    &SuccessfulOutcome          OPTIONAL,
    &UnsuccessfulOutcome        OPTIONAL,
    &Outcome                    OPTIONAL,
    &procedureCode              ProcedureCode  UNIQUE,
    &criticality                Criticality    DEFAULT ignore
  } WITH SYNTAX {
    INITIATING MESSAGE          &InitiatingMessage
    [SUCCESSFUL OUTCOME         &SuccessfulOutcome]
    [UNSUCCESSFUL OUTCOME       &UnsuccessfulOutcome]
    [OUTCOME                    &Outcome]
    PROCEDURE CODE              &procedureCode
    [CRITICALITY                &criticality]
  }

Not all instances of elementary procedure have all their
optional members defined.

For examples :

  iu-Release RANAP-ELEMENTARY-PROCEDURE ::= {
    INITIATING MESSAGE  Iu-ReleaseCommand
    SUCCESSFUL OUTCOME  Iu-ReleaseComplete
    PROCEDURE CODE      id-Iu-Release
    CRITICALITY         reject
  }

  rAB-Assignment RANAP-ELEMENTARY-PROCEDURE ::= {
    INITIATING MESSAGE	RAB-AssignmentRequest
    OUTCOME             RAB-AssignmentResponse
    PROCEDURE CODE      id-RAB-Assignment
    CRITICALITY         reject
  }

So there are empty cells generated in Outcome.c ... etc.

  static const asn_ioc_cell_t asn_IOS_RANAP_ELEMENTARY_PROCEDURES_1_rows[] = {
    { "&InitiatingMessage", aioc__type, &asn_DEF_Iu_ReleaseCommand },
    { "&SuccessfulOutcome", aioc__type, &asn_DEF_Iu_ReleaseComplete },
    { "&UnsuccessfulOutcome",  },
    { "&Outcome",  },
    { "&procedureCode", aioc__value, &asn_DEF_ProcedureCode, &asn_VAL_1_id_Iu_Release },
    { "&criticality", aioc__value, &asn_DEF_Criticality, &asn_VAL_1_reject },

    { "&InitiatingMessage", aioc__type, &asn_DEF_RAB_AssignmentRequest },
    { "&SuccessfulOutcome",  },
    { "&UnsuccessfulOutcome",  },
    { "&Outcome", aioc__type, &asn_DEF_RAB_AssignmentResponse },
    { "&procedureCode", aioc__value, &asn_DEF_ProcedureCode, &asn_VAL_49_id_RAB_Assignment },
    { "&criticality", aioc__value, &asn_DEF_Criticality, &asn_VAL_49_reject }
  }

However, in type_selector() functions, these undefined cell are still
be counted into presence_index value. This results in wrong behavior for
decoder.

This commit is to fix this problem.
@brchiu brchiu force-pushed the fix_s1ap_code_generation_2 branch 7 times, most recently from 00f130c to cea02d7 Compare February 9, 2018 07:16
@mouse07410
Copy link

mouse07410 commented Feb 13, 2018

This PR has a (small) conflict with #226 in file tests/tests-asn1c-compiler/50-constraint-OK.asn1.-Pgen-PER.

@brchiu
Copy link
Contributor Author

brchiu commented Feb 13, 2018

@mouse07410, you can refer to my local branch https://github.com/brchiu/asn1c/tree/s1ap to see how to resolve it.

This conflict comes from my commits : brchiu@e436f61 and brchiu@8d0f685 which generate some constant definition not available in current implementation.

I think my tweak is quite ugly and perhaps requires to be rewritten in a better way.

mouse07410 added a commit to mouse07410/asn1c that referenced this pull request Feb 13, 2018
This commit addresses compatibility of PR vlm#234 with vlm#226 (addition of APER support)
When generating code for multiple ASN.1 syntaxes that have clashing
names, we need to add a prefix in order to prevent clashes in the global
C symbol namespace.  Using the ASN1C_PREFIX environment variable and
this patch serves as a work-around to that.  All non-basic type names
as well as references to that type and source code + header file names
will be pre-fixed accordingly.

This is a re-implementation of this feature due to osmocom's
implementation version can not work on newest asn1c repository.
@brchiu brchiu force-pushed the fix_s1ap_code_generation_2 branch from cea02d7 to 8f68def Compare March 4, 2018 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants