fix: remove potential panic in SupportDnnLists #482

patriciareinoso · 2025-07-16T15:58:50Z

Description

If SupportDnnLists is not set in the configuration file, there is a potential panic in the code.

If the UE's subscription obtained from the UDM does not contain the default DNN, the AMF willl try to get it from the ue.SmfSelectionData. If not found, the AMF will get the DNN from SupportDnnLists. If SupportDnnLists is empty, it will fallback to "internet" as done in producer/callback.go

Signed-off-by: Patricia Reinoso <[email protected]>

Copilot

Pull Request Overview

This PR adds a safe fallback for DNN selection to prevent a panic when SupportDnnLists is empty and removes an unused helper method.

Adds a check to fall back to the first configured DNN or "internet" when no default DNN is found
Logs a warning when falling back from UDM subscription defaults
Removes the now-unused InSupportDnnList method

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
gmm/handler.go	Added fallback logic for DNN selection and warning logging
context/context.go	Removed unused `InSupportDnnList` helper method

Comments suppressed due to low confidence (2)

gmm/handler.go:240

It would be helpful to add unit tests for the fallback branch when SupportDnnLists is empty (leading to the "internet" default) and when SmfSelectionData provides a DNN, to ensure this logic behaves as expected.

					if dnn == "" {

context/context.go:449

Ensure no remaining references to InSupportDnnList exist elsewhere; removing this method might cause compile errors if it was used by any consumers.

	context.AmfRanPool.Delete(gnbId)

gmm/handler.go

gab-arrobo · 2025-07-16T23:43:55Z

@patriciareinoso, please rebase your PR and review Copilot's comments/suggestions

Signed-off-by: Patricia Reinoso <[email protected]>

patriciareinoso · 2025-07-17T08:50:33Z

I am not changing the core behavior of the code so did not expect the E2E tests to fail
I see different results in 2 test runs
in one we can see

2025-07-17T07:34:36.819Z	WARN	gmm/handler.go:247	Subscription context obtained from UDM does not contain the default DNN, using internet	{"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:14266337", "suci": "suci-0-208-93-0-0-0-0100007491", "supi": "SUPI:imsi-208930100007491"}
2025-07-17T07:34:36.819Z	INFO	consumer/sm_context.go:74	Select SMF [snssai: {Sst:1 Sd:010203}, dnn: internet]	{"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:14266337", "suci": "suci-0-208-93-0-0-0-0100007491", "supi": "SUPI:imsi-208930100007491"}
2025-07-17T07:34:36.819Z	INFO	nrfcache/nrfcache.go:361	{6544b711-aefa-490a-83aa-55a72431ec28 NSSF REGISTERED 60 0xc00060d5f0 <nil> [] []   [nssf] [] <nil> [] [] <nil> 0 0 0  <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> map[] <nil> false 0xc00060d608 false false []}	{"component": "LIB", "category": "Nrfcache"}
2025-07-17T07:34:36.820Z	INFO	nrfcache/match_filters.go:113	smf match found, nfInstance Id ef7b609a-2ec6-41d3-b588-4901e568a17f	{"component": "LIB", "category": "Nrfcache"}
2025-07-17T07:34:36.820Z	INFO	nrfcache/nrfcache.go:361	{ef7b609a-2ec6-41d3-b588-4901e568a17f SMF REGISTERED 60 0xc00060c150 0xc00060c168 [] []   [10.42.0.16] [] 0xc00060c198 [] [] <nil> 0 0 0  <nil> <nil> <nil> <nil> 0xc0000a4b40 <nil> <nil> <nil> <nil> <nil> map[] <nil> false 0xc00060c1f8 false false []}	{"component": "LIB", "category": "Nrfcache"}
2025-07-17T07:34:36.874Z	INFO	gmm/handler.go:271	create smContext[pduSessionID: 10] Success	{"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:14266337", "suci": "suci-0-208-93-0-0-0-0100007491", "supi": "SUPI:imsi-208930100007491"}

and in the other

2025-07-17T08:01:21.378Z	ERROR	gmm/handler.go:707	AM Policy Control Create Error[server no response]	{"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:6556641", "suci": "suci-0-208-93-0-0-0-0100007491", "supi": "SUPI:imsi-208930100007491"}

@gab-arrobo any hint?

gab-arrobo

+1

gab-arrobo · 2025-07-17T16:28:01Z

gmm/handler.go

+							dnn = defaultDnn
+						}
+					}
+					ue.GmmLog.Warnf("Subscription context obtained from UDM does not contain the default DNN, using %s", dnn)


Yes, the warning is in the right place. However, the message is confusing because "the default DNN" message/text implies that we are referring to defaultDNN (only else statement). I think it should be something like this:

Suggested change

ue.GmmLog.Warnf("Subscription context obtained from UDM does not contain the default DNN, using %s", dnn)

ue.GmmLog.Warnf("Subscription context obtained from UDM does not contain the DNN, using %s", dnn)

gab-arrobo · 2025-07-17T16:30:49Z

I am not changing the core behavior of the code so did not expect the E2E tests to fail I see different results in 2 test runs in one we can see

2025-07-17T07:34:36.819Z	WARN	gmm/handler.go:247	Subscription context obtained from UDM does not contain the default DNN, using internet	{"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:14266337", "suci": "suci-0-208-93-0-0-0-0100007491", "supi": "SUPI:imsi-208930100007491"}
2025-07-17T07:34:36.819Z	INFO	consumer/sm_context.go:74	Select SMF [snssai: {Sst:1 Sd:010203}, dnn: internet]	{"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:14266337", "suci": "suci-0-208-93-0-0-0-0100007491", "supi": "SUPI:imsi-208930100007491"}
2025-07-17T07:34:36.819Z	INFO	nrfcache/nrfcache.go:361	{6544b711-aefa-490a-83aa-55a72431ec28 NSSF REGISTERED 60 0xc00060d5f0 <nil> [] []   [nssf] [] <nil> [] [] <nil> 0 0 0  <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> map[] <nil> false 0xc00060d608 false false []}	{"component": "LIB", "category": "Nrfcache"}
2025-07-17T07:34:36.820Z	INFO	nrfcache/match_filters.go:113	smf match found, nfInstance Id ef7b609a-2ec6-41d3-b588-4901e568a17f	{"component": "LIB", "category": "Nrfcache"}
2025-07-17T07:34:36.820Z	INFO	nrfcache/nrfcache.go:361	{ef7b609a-2ec6-41d3-b588-4901e568a17f SMF REGISTERED 60 0xc00060c150 0xc00060c168 [] []   [10.42.0.16] [] 0xc00060c198 [] [] <nil> 0 0 0  <nil> <nil> <nil> <nil> 0xc0000a4b40 <nil> <nil> <nil> <nil> <nil> map[] <nil> false 0xc00060c1f8 false false []}	{"component": "LIB", "category": "Nrfcache"}
2025-07-17T07:34:36.874Z	INFO	gmm/handler.go:271	create smContext[pduSessionID: 10] Success	{"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:14266337", "suci": "suci-0-208-93-0-0-0-0100007491", "supi": "SUPI:imsi-208930100007491"}

and in the other

2025-07-17T08:01:21.378Z	ERROR	gmm/handler.go:707	AM Policy Control Create Error[server no response]	{"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:6556641", "suci": "suci-0-208-93-0-0-0-0100007491", "supi": "SUPI:imsi-208930100007491"}

@gab-arrobo any hint?

I am unable to replicate the AM Policy Control Create Error[server no response] message/log. In all my tests, I only see the Subscription context obtained from UDM does not contain the default DNN, using internet message/log

Signed-off-by: Patricia Reinoso <[email protected]>

thakurajayL · 2025-07-18T13:51:40Z

gmm/handler.go

@@ -45,6 +45,7 @@ const (
 	DNN_CONGESTION            = "DNN_CONGESTION"
 	PRIORITIZED_SERVICES_ONLY = "PRIORITIZED_SERVICES_ONLY"
 	OUT_OF_LADN_SERVICE_AREA  = "OUT_OF_LADN_SERVICE_AREA"
+	defaultDnn                = "internet"


I am not sure if we should have any hardcoding of default DNN anywhere in the code. Its risky and will lead to issues which would be hard to debug. Could you check if in the webui we add default DNN? We can always make some assumptions that we should always get default DNN.

In the webui we do not add a default DNN.
In canonical's solution, we do not set the SupportDnnLists at all, and we do not go trough this branch of the code. I do not know why the e2e tests goes trough that branch of code.

I do see the same thing in my local setup (code goes through the else statement and ends up running ue.GmmLog.Warnf("Subscription context obtained from UDM does not contain the DNN, using %s", dnn)
FYI, this is the content for ulNasTransport and as you can see there, DNN is nil. What is the value of ulNasTransport in your setup/test?

2025-07-18T15:44:44.379Z WARN gmm/handler.go:214 GA: ulNasTransport is: &{ExtendedProtocolDiscriminator:{Octet:126} SpareHalfOctetAndSecurityHeaderType:{Octet:0} ULNASTRANSPORTMessageIdentity:{Octet:103} SpareHalfOctetAndPayloadContainerType:{Octet:1} PayloadContainer:{Iei:0 Len:20 Buffer:[46 10 1 193 255 255 145 123 0 10 128 0 10 0 0 13 0 0 3 0]} PduSessionID2Value:0xc001317142 OldPDUSessionID:<nil> RequestType:0xc001317146 SNSSAI:<nil> DNN:<nil> AdditionalInformation:<nil>} {"component": "AMF", "category": "GMM", "amf_ue_ngap_id": "AMF_UE_NGAP_ID:323545", "suci": "suci-0-208-93-0-0-0-0100007511", "supi": "SUPI:imsi-208930100007511"}

Note: I added the above log right before if ulNasTransport.SNSSAI != nil {

ue.GmmLog.Warnf("GA: ulNasTransport is: %+v", ulNasTransport)

If this is the case we should have some way to provide default DNN in AMF. We had added similar support in 4G. I would prefer that approach vs hardcoded internet dnn.

patriciareinoso added 2 commits July 16, 2025 17:50

remove potential panic in SupportDnnLists

b2b27bc

Signed-off-by: Patricia Reinoso <[email protected]>

exported

cc4e6b7

Signed-off-by: Patricia Reinoso <[email protected]>

patriciareinoso mentioned this pull request Jul 16, 2025

feat: make DNN configurable (clean up charm config) canonical/sdcore-amf-k8s-operator#555

Closed

7 tasks

gab-arrobo requested review from a team and Copilot July 16, 2025 16:35

Copilot AI reviewed Jul 16, 2025

View reviewed changes

gmm/handler.go Outdated Show resolved Hide resolved

gmm/handler.go Outdated Show resolved Hide resolved

patriciareinoso added 2 commits July 17, 2025 09:22

Merge branch 'main' into fix-potential-panic-in-support-dnn-list

a7e5287

review comments

090a0fb

Signed-off-by: Patricia Reinoso <[email protected]>

gab-arrobo reviewed Jul 17, 2025

View reviewed changes

patriciareinoso added 3 commits July 18, 2025 12:04

Merge branch 'main' into fix-potential-panic-in-support-dnn-list

4c897c9

fix log

1b1f003

Signed-off-by: Patricia Reinoso <[email protected]>

fix lint

11bb00a

Signed-off-by: Patricia Reinoso <[email protected]>

thakurajayL reviewed Jul 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: remove potential panic in SupportDnnLists #482

fix: remove potential panic in SupportDnnLists #482

patriciareinoso commented Jul 16, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

gab-arrobo commented Jul 16, 2025

Uh oh!

patriciareinoso commented Jul 17, 2025 •

edited

Loading

Uh oh!

gab-arrobo left a comment

Uh oh!

gab-arrobo Jul 17, 2025

Uh oh!

gab-arrobo commented Jul 17, 2025

Uh oh!

thakurajayL Jul 18, 2025

Uh oh!

patriciareinoso Jul 18, 2025

Uh oh!

gab-arrobo Jul 18, 2025 •

edited

Loading

Uh oh!

thakurajayL Jul 20, 2025

Uh oh!

Uh oh!

	ue.GmmLog.Warnf("Subscription context obtained from UDM does not contain the default DNN, using %s", dnn)
	ue.GmmLog.Warnf("Subscription context obtained from UDM does not contain the DNN, using %s", dnn)

fix: remove potential panic in SupportDnnLists #482

Are you sure you want to change the base?

fix: remove potential panic in SupportDnnLists #482

Conversation

patriciareinoso commented Jul 16, 2025

Description

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

gab-arrobo commented Jul 16, 2025

Uh oh!

patriciareinoso commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gab-arrobo left a comment

Choose a reason for hiding this comment

Uh oh!

gab-arrobo Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

gab-arrobo commented Jul 17, 2025

Uh oh!

thakurajayL Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

patriciareinoso Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

gab-arrobo Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thakurajayL Jul 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patriciareinoso commented Jul 17, 2025 •

edited

Loading

gab-arrobo Jul 18, 2025 •

edited

Loading