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

Added intelligent AMF selection at gNB #731

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

Conversation

priyanshs
Copy link

  • Fixed the issue of gNB randomly routing registration requests to an arbitrary AMF, without considering the slice type requested by the UE and the slice types served by the AMF.

  • Extended the gNB to select AMF intelligently based on the type of slice requested by the UE.

Contribution by @prateekbhaisora

@priyanshs
Copy link
Author

@aligungr, please advise if we need to make further changes to this implementation. We tried connecting UEs with the same PLMN but on different slices, and it failed. The changes we made were to select AMF from the list.

Thanks

@priyanshs
Copy link
Author

@aligungr please have a look at the PR and let us know if we need to make further changes. It would be nice if we could merge with the master branch.
Thanks

@uchiha-Itachi-kanoha
Copy link

@priyanshs i have a query, does the UERANSIM supports IPsec ?

@@ -25,6 +25,11 @@ struct octet
{
}

inline uint8_t getValue() const
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this function.

You can directly cast to uint8_t in order to convert octet to uint8_t

uint8_t aaa = (uint8_t)bbb;

Copy link

@prateekbhaisora prateekbhaisora Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have used the getValue() getter method here as value is a private variable of struct octet and this appeared to me as the most appropriate way to access the value variable from outside, i.e., in the extractSliceInfoAndModifyPdu method. Please let me know, if there is a more recommended way.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

octet has a special converter for uint8_t. Please directly use this converter, and remove the getValue().

Example:

octet someOctet = ....
uint8_t someUint8 = (uint8_t)someOctet 

@@ -25,6 +25,11 @@ class OctetString
{
}

const std::vector<uint8_t>& getData() const
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data is already present. Please remove this method.

Copy link

@prateekbhaisora prateekbhaisora Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I used this getter() as m_data was a private member of the Octetstring class and for extractSliceInfoAndModifyPdu(), I needed the data from NAS PDU. Please let me know, if there is a more suitable alternative.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes m_data is private, however we have public data() function. Please use data() function instead.

@@ -73,7 +73,6 @@ static void RemoveCleartextIEs(nas::PlainMmMessage &msg, OctetString &&nasMsgCon
regReq.micoIndication = std::nullopt;
regReq.networkSlicingIndication = std::nullopt;
regReq.mmCapability = std::nullopt;
regReq.requestedNSSAI = std::nullopt;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain this remove?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to prevent the UE from removing Slice information, before sending the NAS PDU. It is in reference to the comment here.

for (auto &amf : m_amfCtx)
return amf.second; // return the first one
for (auto &amf : m_amfCtx) {
for (const auto &PlmnSupport : amf.second->plmnSupportList) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use lower-case for SingleSlice and SingleSlice. Because they are loop variables.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Will do that.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename const auto &PlmnSupport to const auto &plmnSupport

and

const auto &SingleSlice to const auto &singleSlice


And please replace const int32_t &requestedSliceType to just int32_t requestedSliceType


namespace nr::gnb
{

void NgapTask::handleInitialNasTransport(int ueId, const OctetString &nasPdu, int64_t rrcEstablishmentCause,
const std::optional<GutiMobileIdentity> &sTmsi)
int32_t extractSliceInfoAndModifyPdu(OctetString &nasPdu) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why the modification is needed for the original PDU

Copy link

@prateekbhaisora prateekbhaisora Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes sure. In the current implementation, the UE is removing the information elements (including slice information), before sending the NAS PDU to gNB [ RemoveCleartextIEs() ]. gNB then selects one AMF blindly and forwards the request to the selected AMF.

Now, to support intelligent AMF selection based on slice type, I have prevented removal of slice info from the PDU, when the UE sends it. Now, at the gNB end, the gNB uses this slice info., to select an appropriate AMF. Before forwarding the PDU to AMF, I had to remove the slice information from the PDU at gNB, else it leads to a "Unsupported Message Type received" error on AMF end.

@aligungr
Copy link
Owner

@priyanshs

Thanks for the pull request. I've completed the first review. Could you please look the comments above?

@priyanshs priyanshs requested a review from aligungr November 7, 2024 07:52
@aligungr
Copy link
Owner

Sorry for the late reply. I have some more comments. Please see. Thanks

for (auto &amf : m_amfCtx)
return amf.second; // return the first one
for (auto &amf : m_amfCtx) {
for (const auto &PlmnSupport : amf.second->plmnSupportList) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename const auto &PlmnSupport to const auto &plmnSupport

and

const auto &SingleSlice to const auto &singleSlice


And please replace const int32_t &requestedSliceType to just int32_t requestedSliceType

@@ -71,7 +71,7 @@ class NgapTask : public NtsTask
/* Utility functions */
void createAmfContext(const GnbAmfConfig &config);
NgapAmfContext *findAmfContext(int ctxId);
void createUeContext(int ueId);
void createUeContext(int ueId, const int32_t &requestedSliceType);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also make just int32_t requestedSliceType

@@ -118,7 +118,7 @@ class NgapTask : public NtsTask
void sendContextRelease(int ueId, NgapCause cause);

/* NAS Node Selection */
NgapAmfContext *selectAmf(int ueId);
NgapAmfContext *selectAmf(int ueId, const int32_t &requestedSliceType);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing please

@@ -25,6 +25,11 @@ class OctetString
{
}

const std::vector<uint8_t>& getData() const
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes m_data is private, however we have public data() function. Please use data() function instead.

@@ -25,6 +25,11 @@ struct octet
{
}

inline uint8_t getValue() const
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

octet has a special converter for uint8_t. Please directly use this converter, and remove the getValue().

Example:

octet someOctet = ....
uint8_t someUint8 = (uint8_t)someOctet 

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.

4 participants