Skip to content

Conversation

@InertGas01
Copy link

No description provided.

@InertGas01
Copy link
Author

example code for the following blog.
free5gc/free5gc.github.io#203

Copy link
Member

@Alonza0314 Alonza0314 left a comment

Choose a reason for hiding this comment

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

There are a log of logrus.Error in test procedure. Do these should be ignore even if got in error? If they should not be ignore, maybe we need to substitute them with return error and t.Fatal in the main test function.

Comment on lines 274 to 279
pendingHOResponseChan := make(chan context.PendingHandoverResponse)
value, loaded := amfSelf.PendingHandovers.LoadOrStore(ue.Supi, pendingHOResponseChan)
if loaded {
logger.CommLog.Info("PendingHandoverResponse channel created by HandoverRequestAcknowledge handler.")
pendingHOResponseChan = value.(chan context.PendingHandoverResponse)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

	var pendingHOResponseChan chan context.PendingHandoverResponse
	value, loaded := amfSelf.PendingHandovers.LoadOrStore(ue.Supi, pendingHOResponseChan)
	if loaded {
		logger.CommLog.Info("PendingHandoverResponse channel created by HandoverRequestAcknowledge handler.")
		pendingHOResponseChan = value.(chan context.PendingHandoverResponse)
	} else {
        pendingHOResponseChan = make(chan context.PendingHandoverResponse)
    }

amfSelf := amfUe.ServingAMF()

// Create channel if not exist
pendingHOResponseChan := make(chan context.PendingHandoverResponse)
Copy link
Contributor

@ianchen0119 ianchen0119 Oct 27, 2025

Choose a reason for hiding this comment

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

	var pendingHOResponseChan chan context.PendingHandoverResponse
	value, loaded := amfSelf.PendingHandovers.LoadOrStore(ue.Supi, pendingHOResponseChan)
	if loaded {
		logger.CommLog.Info("PendingHandoverResponse channel created by HandoverRequestAcknowledge handler.")
		pendingHOResponseChan = value.(chan context.PendingHandoverResponse)
	} else {
        // handle the error
    }

Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, the pendingHOResponseChan should be crated already when receiving the HO Req Ack.
Please add error handling for the case of PendingHandovers not loaded.

Copy link
Author

@InertGas01 InertGas01 Oct 31, 2025

Choose a reason for hiding this comment

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

Yes. The pendingHOResponseChan should be created in CreateUEContextProcedure() right after the HO Req is sent. In this case, the loaded is set to true, and the pendingHOResponseChan is replaced by the loaded value. Otherwise, the pendingHOResponseChan created here is stored to the sync.Map and used afterwards. That's why I think the error handling for the if statement is not needed.

return true
})
if isEmpty {
logger.CommLog.Warnf("AmfRanPool is empty\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

use Warnln to instead

if sourceUe == nil {
// TODO: Send Namf_Communication_CreateUEContext Response to S-AMF
ran.Log.Error("handover between different Ue has not been implement yet")
var ueContextCreatedData models.UeContextCreatedData
Copy link
Contributor

Choose a reason for hiding this comment

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

add helper function to build the models.UeContextCreatedData from provided amfUeContext.

return
// TODO: Send to T-AMF
// Described in (23.502 4.9.1.3.2) step 3.Namf_Communication_CreateUEContext Request
/*
Copy link
Member

Choose a reason for hiding this comment

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

Why did this section remain in comment condition? If it doesn't need now, please remove it or leave the comment with why you make it as comment.

targetRanUe.Log = logger.NgapLog

// send Handover Request to target RAN
ngap_message.SendHandoverRequestWithAMFChange(
Copy link
Member

Choose a reason for hiding this comment

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

You called SendHandoverRequestWithAMFChange before the channel created. If the load section is quicker than you make the channel, it will result in panic or unexpected handover fail.

}
}
return
return amPolicyReqTriggers
Copy link
Member

Choose a reason for hiding this comment

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

As we have define in line 650, this function will return the amPolicyReqTriggers automatically without specifying it at return statement.


sourceUe := targetUe.SourceUe
if sourceUe == nil {
// TODO: Send Namf_Communication_CreateUEContext Response to S-AMF
Copy link
Member

Choose a reason for hiding this comment

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

After implementation, this TODO line can be removed.

} else {
logger.CommLog.Info("PendingHandoverResponse channel closed.")
}
case <-time.After(100 * time.Millisecond):
Copy link
Member

Choose a reason for hiding this comment

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

After the timeout case, I think we need some clear mechanism for removing created instances since the HO procedure is failed.

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