-
Notifications
You must be signed in to change notification settings - Fork 41
Support for Multiple DNNs per Device Group with a Single UPF #439
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -184,12 +184,16 @@ func (upi *UserPlaneInformation) GenerateDefaultPath(selection *UPFSelectionPara | |
destinations = upi.selectMatchUPF(selection) | ||
|
||
if len(destinations) == 0 { | ||
logger.CtxLog.Errorf("can not find UPF with DNN[%s] S-NSSAI[sst: %d sd: %s] DNAI[%s]", selection.Dnn, | ||
selection.SNssai.Sst, selection.SNssai.Sd, selection.Dnai) | ||
for _, dnn := range selection.DnnList { | ||
logger.CtxLog.Errorf("Can't find UPF with DNN[%s] S-NSSAI[sst: %d sd: %s] DNAI[%s]\n", dnn, | ||
selection.SNssai.Sst, selection.SNssai.Sd, selection.Dnai) | ||
} | ||
return false | ||
} else { | ||
logger.CtxLog.Debugf("found UPF with DNN[%s] S-NSSAI[sst: %d sd: %s] DNAI[%s]", selection.Dnn, | ||
selection.SNssai.Sst, selection.SNssai.Sd, selection.Dnai) | ||
for _, dnn := range selection.DnnList { | ||
logger.CtxLog.Infof("Found UPF with DNN[%s] S-NSSAI[sst: %d sd: %s] DNAI[%s]\n", dnn, | ||
selection.SNssai.Sst, selection.SNssai.Sd, selection.Dnai) | ||
} | ||
} | ||
|
||
// Run DFS | ||
|
@@ -223,17 +227,20 @@ func (upi *UserPlaneInformation) GenerateDefaultPath(selection *UPFSelectionPara | |
|
||
func (upi *UserPlaneInformation) selectMatchUPF(selection *UPFSelectionParams) []*UPNode { | ||
upList := make([]*UPNode, 0) | ||
|
||
logger.CtxLog.Infof("Selecting matching UPFs for DNNs[%v] and S-NSSAI[sst: %d, sd: %s]", selection.DnnList, selection.SNssai.Sst, selection.SNssai.Sd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any logging event which is not error and per call should not be at info level. Should stay at debug level |
||
for _, upNode := range upi.UPFs { | ||
for _, snssaiInfo := range upNode.UPF.SNssaiInfos { | ||
currentSnssai := &snssaiInfo.SNssai | ||
targetSnssai := selection.SNssai | ||
|
||
if currentSnssai.Equal(targetSnssai) { | ||
for _, dnnInfo := range snssaiInfo.DnnList { | ||
if dnnInfo.Dnn == selection.Dnn && dnnInfo.ContainsDNAI(selection.Dnai) { | ||
upList = append(upList, upNode) | ||
break | ||
for _, dnn := range selection.DnnList { | ||
if dnnInfo.Dnn == dnn && dnnInfo.ContainsDNAI(selection.Dnai) { | ||
upList = append(upList, upNode) | ||
logger.CtxLog.Infof("Matching UPF found: %v for DNN[%s] and DNAI[%s]", upNode, dnn, selection.Dnai) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update logging level to debug as suggested in previous comment. |
||
break | ||
} | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,7 +160,7 @@ func TestGenerateDefaultPath(t *testing.T) { | |
Sst: 1, | ||
Sd: "112232", | ||
}, | ||
Dnn: "internet", | ||
DnnList: []string{"internet"}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good if you could add test which coveres multiple DNN. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, we will do this. |
||
}, | ||
expected: true, | ||
}, | ||
|
@@ -171,7 +171,7 @@ func TestGenerateDefaultPath(t *testing.T) { | |
Sst: 2, | ||
Sd: "112233", | ||
}, | ||
Dnn: "internet", | ||
DnnList: []string{"internet"}, | ||
}, | ||
expected: true, | ||
}, | ||
|
@@ -182,7 +182,7 @@ func TestGenerateDefaultPath(t *testing.T) { | |
Sst: 3, | ||
Sd: "112234", | ||
}, | ||
Dnn: "internet", | ||
DnnList: []string{"internet"}, | ||
}, | ||
expected: true, | ||
}, | ||
|
@@ -193,7 +193,7 @@ func TestGenerateDefaultPath(t *testing.T) { | |
Sst: 1, | ||
Sd: "112235", | ||
}, | ||
Dnn: "internet", | ||
DnnList: []string{"internet"}, | ||
}, | ||
expected: true, | ||
}, | ||
|
@@ -204,7 +204,7 @@ func TestGenerateDefaultPath(t *testing.T) { | |
Sst: 1, | ||
Sd: "010203", | ||
}, | ||
Dnn: "internet", | ||
DnnList: []string{"internet"}, | ||
}, | ||
expected: false, | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ package factory | |
import ( | ||
"testing" | ||
|
||
protos "github.com/omec-project/config5g/proto/sdcoreConfig" | ||
protos "github.com/5GC-DEV/config5g-cdac/proto/sdcoreConfig" | ||
"github.com/omec-project/openapi/models" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
@@ -50,7 +50,7 @@ func makeDummyConfig(sst, sd string) *protos.NetworkSliceResponse { | |
|
||
ns.DeviceGroup = make([]*protos.DeviceGroup, 0) | ||
ipDomain := protos.IpDomain{DnnName: "internet", UePool: "60.60.0.0/16", DnsPrimary: "8.8.8.8", Mtu: 1400} | ||
devGrp := protos.DeviceGroup{IpDomainDetails: &ipDomain} | ||
devGrp := protos.DeviceGroup{IpDomainDetails: []*protos.IpDomain{&ipDomain}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest add one more additional testcase to cover 2 DNNs. |
||
ns.DeviceGroup = append(ns.DeviceGroup, &devGrp) | ||
|
||
rsp.NetworkSlice = append(rsp.NetworkSlice, &ns) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,6 @@ require ( | |
github.com/google/uuid v1.6.0 | ||
github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 | ||
github.com/omec-project/aper v1.3.1 | ||
github.com/omec-project/config5g v1.6.2 | ||
github.com/omec-project/nas v1.6.0 | ||
github.com/omec-project/ngap v1.5.0 | ||
github.com/omec-project/openapi v1.5.0 | ||
|
@@ -23,6 +22,8 @@ require ( | |
gopkg.in/yaml.v2 v2.4.0 | ||
) | ||
|
||
require github.com/5GC-DEV/config5g-cdac v0.2.1 // indirect | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gab-arrobo - this change may not be good right? We should be using omec-project config5g. We can not be using forked version of config5g. |
||
|
||
require ( | ||
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect | ||
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,9 +18,9 @@ import ( | |
"syscall" | ||
"time" | ||
|
||
grpcClient "github.com/5GC-DEV/config5g-cdac/proto/client" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will have to port this change once other big change from Canonical team merges. |
||
protos "github.com/5GC-DEV/config5g-cdac/proto/sdcoreConfig" | ||
aperLogger "github.com/omec-project/aper/logger" | ||
grpcClient "github.com/omec-project/config5g/proto/client" | ||
protos "github.com/omec-project/config5g/proto/sdcoreConfig" | ||
nasLogger "github.com/omec-project/nas/logger" | ||
ngapLogger "github.com/omec-project/ngap/logger" | ||
openapiLogger "github.com/omec-project/openapi/logger" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anaswarac-dac - I am curious to know if you found any case where upfSelectionParams was null in this function call. Though this check will not have side effects but I would like to why this input parameter would be null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thakurajayL , We haven't encountered a case where upfSelectionParams was nil in this function call. The check was added purely as a precautionary measure to avoid potential nil dereference issues and to assist in debugging, if such a case ever arises.