Skip to content

SGW TAC Selection#100

Closed
TerryAlu wants to merge 2 commits intoopen5gs:masterfrom
TerryAlu:tac_select
Closed

SGW TAC Selection#100
TerryAlu wants to merge 2 commits intoopen5gs:masterfrom
TerryAlu:tac_select

Conversation

@TerryAlu
Copy link
Copy Markdown

@TerryAlu TerryAlu commented Aug 22, 2018

Instead of selecting SGW with round-robin manner, I make some changes to select an available SGW by TAC of eNodeB.

tac_selection

@acetcom
Copy link
Copy Markdown
Member

acetcom commented Aug 27, 2018

We will apply this fancy configuration to NextEPC. But, we are wondering how to define the configuration file.

Here is our proposal.

sgw_selection:  rr ( round-robin), tac(use predefined TAC) , dns( use DNS query based on 3gpp spec)

Now we are a bit busy with other things. Soon we will review this section and apply it.
Thank you very much for giving us this idea,

@TerryAlu
Copy link
Copy Markdown
Author

It sounds great to define the configuration file by sgw_selection, so I change the keyword from tac_selection to sgw_selection. Besides, I correct my indentations at previous commit.

@acetcom
Copy link
Copy Markdown
Member

acetcom commented Sep 14, 2018

Ultra late response!. So sorry!

IMHO, there is a little problem in your great work!. You added tac information in gtp_node_t structure. This approach is not allowed. mme_sgw_t structure with including gtp_node_t should be created in MME context.

If you are not changing it, let us modify your source code for creating mme_sgw_t structure.

Thanks a lot!

@TerryAlu
Copy link
Copy Markdown
Author

Creating mme_sgw_t in MME context seems better. Please help me to modify it. Thanks for the suggestions!

@acetcom
Copy link
Copy Markdown
Member

acetcom commented Sep 20, 2018

Here is my proposal for mme_sgw_t structure.

#include "gtp/gtp_node.h"

typedef struct _mme_sgw_t {
    gtp_node_t      gnode;

    c_uint16_t        tac[MAX_NUM_OF_TAI];
    c_uint8_t          num_of_tac;
} mme_sgw_t;

typedef struct _mme_context_t {
   mme_sgw_t     *sgw;
}

gtp_node_t has a lnode_t. So you can use linked-list function with mme_sgw_t structure.
And also, please try to reduce the (gtp_node_t *) type-casting to use function call parameter.

Thanks!

@TerryAlu
Copy link
Copy Markdown
Author

I have some questions to use linked-list function with mme_sgw_t.

After I create links between mme_sgw_t structures as the following figure, should I create a variable in mme_context_t to hold the reference to the first mme_sgw_t element? So I can traverse the link from the first every time.

mme_sgw_link

Besides, is it better to change the content of sgw_list from gtp_node_t to mme_sgw_t? If it is true, I need to define a function similar to gtp_add_node in mme_context.h in order to avoid modifying gtp_node.c. But this will cause some copy-and-paste programming.

@acetcom
Copy link
Copy Markdown
Member

acetcom commented Sep 20, 2018

Ok! Let me clarify my idea with using source code as follows.

diff --git a/lib/gtp/gtp_node.h b/lib/gtp/gtp_node.h
index b24c3b6e..3b868e79 100644
--- a/lib/gtp/gtp_node.h
+++ b/lib/gtp/gtp_node.h
@@ -14,7 +14,7 @@ extern "C" {
     do { \
         d_assert((__cTX), break, "Null param"); \
         d_assert((__gNODE), break, "Null param"); \
-        (__cTX)->gnode = __gNODE; \
+        (__cTX)->gnode = (gtp_node_t *)__gNODE; \
     } while(0)

 /**
diff --git a/src/mme/mme_context.h b/src/mme/mme_context.h
index 852c082c..c976e6b4 100644
--- a/src/mme/mme_context.h
+++ b/src/mme/mme_context.h
@@ -16,6 +16,7 @@
 #include "s1ap/s1ap_message.h"
 #include "nas/nas_message.h"
 #include "fd/s6a/s6a_message.h"
+#include "gtp/gtp_node.h"

 /* S1AP */
 #include "S1AP_Cause.h"
@@ -34,6 +35,8 @@ extern "C" {

 #define MAX_NUM_OF_BPLMN            6

+typedef struct _mme_sgw_t mme_sgw_t;
+
 typedef struct _enb_ue_t enb_ue_t;
 typedef struct _mme_ue_t mme_ue_t;

@@ -72,7 +75,7 @@ typedef struct _mme_context_t {
     c_sockaddr_t    *gtpc_addr6;    /* MME GTPC IPv6 Address */

     list_t          sgw_list;       /* SGW GTPC Client List */
-    gtp_node_t      *sgw;           /* Iterator for SGW round-robin */
+    mme_sgw_t       *sgw;           /* Iterator for SGW round-robin */

     list_t          pgw_list;       /* PGW GTPC Client List */
     c_sockaddr_t    *pgw_addr;      /* First IPv4 Address Selected */

 #define MAX_NUM_OF_BPLMN            6

+typedef struct _mme_sgw_t mme_sgw_t;
+
 typedef struct _enb_ue_t enb_ue_t;
 typedef struct _mme_ue_t mme_ue_t;

@@ -72,7 +75,7 @@ typedef struct _mme_context_t {
     c_sockaddr_t    *gtpc_addr6;    /* MME GTPC IPv6 Address */

     list_t          sgw_list;       /* SGW GTPC Client List */
-    gtp_node_t      *sgw;           /* Iterator for SGW round-robin */
+    mme_sgw_t       *sgw;           /* Iterator for SGW round-robin */

     list_t          pgw_list;       /* PGW GTPC Client List */
     c_sockaddr_t    *pgw_addr;      /* First IPv4 Address Selected */
@@ -140,6 +143,13 @@ typedef struct _mme_context_t {

 } mme_context_t;

+typedef struct _mme_sgw_t {
+    gtp_node_t      gnode;
+
+    c_uint16_t      tac[MAX_NUM_OF_TAI];
+    c_uint8_t       num_of_tac;
+} mme_sgw_t;
+
 typedef struct _mme_enb_t {
     index_t         index;  /* An index of this node */
     fsm_t           sm;     /* A state machine */

What do you think about it?
Feel free to raise any issue.

Thanks a lot!

@TerryAlu
Copy link
Copy Markdown
Author

I have structures similar to yours in mme_context.h. But, I have a trouble allocating memory to mme_sgw_t structure and making mme_sgw_t linked list. Because I want to use gtp_add_node function, which only works with gtp_node_t.

@acetcom
Copy link
Copy Markdown
Member

acetcom commented Sep 22, 2018

There is a problem the way I proposed. I'm so sorry if you took a lot of time to figure this out.
We think we should fix a lot of source code to support tac_select function.

typedef struct _mme_sgw_t {
    lnode_t node ;

    c_uint16_t      tac[MAX_NUM_OF_TAI];
    c_uint8_t       num_of_tac;
    gtp_node_t     *gnode;
} mme_sgw_t;

We would like to place lnode_t node that manages the list in mme_sgw_t and change the gnode to pointer.
Then, gnode add/remove interface should be changed as below.

rv = gtp_create_node(&sgw->gnode,
                                context_self()->parameter.no_ipv4,
                                context_self()->parameter.no_ipv6,
                                context_self()->parameter.prefer_ipv4);
rv = gtp_delete_node(sgw->gnode);

Yes. In doing so, we will not use the gtp_node_t used by SGW/PGW directly, but we will have to construct the following structure.

  • On SGW
typedef struct _sgw_mme_t {
    lnode_t node ;

    gtp_node_t     *gnode;
} sgw_mme_t;

typedef struct _sgw_pgw_t {
    lnode_t node ;

    gtp_node_t     *gnode;
} sgw_pgw_t;
  • On PGW
typedef struct _pgw_sgw_t {
    lnode_t node ;

    gtp_node_t     *gnode;
} pgw_sgw_t;

Then we need to implement add/remove function to manage the list of these structures in MME/SGW/PGW context.

What do you think about our second proposal? You can fix this whole part. If you do not have time to do that, we will start this re-factoring.

@TerryAlu
Copy link
Copy Markdown
Author

I think the second proposal is practical! But I don't have time to make those changes currently. You can refactor it when you are available.

acetcom added a commit that referenced this pull request Sep 23, 2018
@acetcom
Copy link
Copy Markdown
Member

acetcom commented Sep 23, 2018

To solve this problem, I created some of common gtp_node_t, namely mme_sgw_t and mme_pgw_t. I think it would be more efficient to keep all the rest. It means that I didn't created sgw_mme_t, sgw_pgw_t, and pgw_sgw_t.

And then, I modified the configuration file that you proposed.

#
# <SGW Selection Mode>
#
# o Round-Robin
#   (If you does not set the `selection_mode`, default mode is Round-Robin)
#
#   selection_mode: rr
#   gtpc:
#     addr: 127.0.0.2
#     addr: 127.0.2.2
#     addr: 127.0.4.2
#
# o SGW selection by eNodeB TAC
#
#   selection_mode: tac
#   gtpc:
#     - addr: 127.0.0.2
#       tac: 26000
#     - addr: 127.0.2.2
#       tac: [25000, 27000, 28000]
#

Thanks to you, we have also solved the problems of our structure and we can add one good feature for SGW selection.

I'm really appreciated for your work!

acetcom added a commit that referenced this pull request Sep 23, 2018
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 18, 2020

CLA assistant check
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.
1 out of 2 committers have signed the CLA.

✅ TerryAlu
❌ CaiYunZhan


CaiYunZhan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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