Skip to content

Support for Multiple DNNs per Device Group with a Single UPF #929

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
+77 −28
Closed

Support for Multiple DNNs per Device Group with a Single UPF #929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion conf/upf.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,12 @@
"cpiface": {
"peers": ["148.162.12.214"],
// [Optional] Below parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as in ptf/config/upf.jsonc

"dnn": "internet",
"dnn_list": [
{
"dnn": "internet",
"ue_ip_pool": "10.250.0.0/16"
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that now you have the ue_ip_pool here, should not it be removed from below?

        // "hostname": "upf-0",
        "ue_ip_pool": "10.250.0.0/16"
    },

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, can we please update this to use a private IP range/pool?

Copy link
Contributor

Choose a reason for hiding this comment

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

@gab-arrobo Isn't it like 10.0.0.0/8 is complete Class A private IP address range. So 10.250.0.0/16 is part of private IP address range.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gab-arrobo Isn't it like 10.0.0.0/8 is complete Class A private IP address range. So 10.250.0.0/16 is part of private IP address range.

My bad. I misread this... for some reason I thought it was 172.250.0.0/16 (as it was in OnRamp before). @anaswarac-dac please disregard this comment

}
],
"http_port": "8080",
"enable_ue_ip_alloc": false,
// "use_fqdn": "true",
Expand Down
39 changes: 22 additions & 17 deletions pfcpiface/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,18 @@ type SimModeInfo struct {

// CPIfaceInfo : CPIface interface settings.
type CPIfaceInfo struct {
Peers []string `json:"peers"`
UseFQDN bool `json:"use_fqdn"`
NodeID string `json:"hostname"`
HTTPPort string `json:"http_port"`
Dnn string `json:"dnn"`
EnableUeIPAlloc bool `json:"enable_ue_ip_alloc"`
UEIPPool string `json:"ue_ip_pool"`
Peers []string `json:"peers"`
UseFQDN bool `json:"use_fqdn"`
NodeID string `json:"hostname"`
HTTPPort string `json:"http_port"`
DnnList []DNNInfo `json:"dnn_list"`
EnableUeIPAlloc bool `json:"enable_ue_ip_alloc"`
UEIPPool string `json:"ue_ip_pool"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that you are moving the UEIPPool to inside DNNInfo, should not it be removed from here?

Suggested change
UEIPPool string `json:"ue_ip_pool"`

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes we should remove UEIPPool config from here and also look at the functionality of enable_ue_ip_alloc flag.

}

type DNNInfo struct {
DNN string `json:"dnn"`
UEIPPool string `json:"ue_ip_pool"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add enable_ue_ip_alloc flag in this structure and see the flow around this flag.

}

// IfaceType : Gateway interface struct.
Expand All @@ -118,12 +123,12 @@ func validateConf(conf Conf) error {
if err != nil {
return ErrInvalidArgumentWithReason("conf.P4rtcIface.AccessIP", conf.P4rtcIface.AccessIP, err.Error())
}

_, _, err = net.ParseCIDR(conf.CPIface.UEIPPool)
if err != nil {
return ErrInvalidArgumentWithReason("conf.UEIPPool", conf.CPIface.UEIPPool, err.Error())
for _, dnn := range conf.CPIface.DnnList {
_, _, err := net.ParseCIDR(dnn.UEIPPool)
Copy link
Contributor

Choose a reason for hiding this comment

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

should we check that dnn is not an empty string?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. It seems good idea to check the pool validity. But see we have error handling as well. so that will indirectly handle the configuration error?

if err != nil {
return ErrInvalidArgumentWithReason("conf.CPIface.DnnList.UEIPPool", dnn.UEIPPool, err.Error())
}
}

if conf.Mode != "" {
return ErrInvalidArgumentWithReason("conf.Mode", conf.Mode, "mode must not be set for UP4")
}
Expand All @@ -140,14 +145,14 @@ func validateConf(conf Conf) error {
return ErrInvalidArgumentWithReason("conf.Mode", conf.Mode, "invalid mode")
}
}
Copy link
Contributor

@patriciareinoso patriciareinoso Jul 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

in this function there are 2 checks over the DnnList that seem redundant. One is done if EnableUeIPAlloc is true and the other if EnableP4rt is true.
Later on in the code, the information in DnnList seems to be used regardless of these 2 conditions being true or false.
Should the checks be done once here in this function, even if the 2 conditions are false?


if conf.CPIface.EnableUeIPAlloc {
_, _, err := net.ParseCIDR(conf.CPIface.UEIPPool)
if err != nil {
return ErrInvalidArgumentWithReason("conf.UEIPPool", conf.CPIface.UEIPPool, err.Error())
for _, dnn := range conf.CPIface.DnnList {
_, _, err := net.ParseCIDR(dnn.UEIPPool)
if err != nil {
return ErrInvalidArgumentWithReason("conf.CPIface.DnnList.UEIPPool", dnn.UEIPPool, err.Error())
}
}
}

for _, peer := range conf.CPIface.Peers {
ip := net.ParseIP(peer)
if ip == nil {
Expand Down
3 changes: 3 additions & 0 deletions pfcpiface/fteid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ func NewFTEIDGenerator() *FTEIDGenerator {

// Allocate and return an id in range [minValue, maxValue]
func (idGenerator *FTEIDGenerator) Allocate() (uint32, error) {
if idGenerator == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is redundant. Everytime we create upf object we assign/create FTEID Generator block. Perhaps you saw crash and that is the reason you want to add this nil check. But remember there is likely chance that we are masking some other bug by adding this nil check.

Copy link
Contributor

Choose a reason for hiding this comment

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

@anaswarac-dac - please confirm why this change is needed. As I said this is likely not required. Issue could be something else.

Copy link
Author

Choose a reason for hiding this comment

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

You're right — ideally the FTEIDGenerator should never be nil, as it's initialized during UPF object creation. However, I added this nil check because during testing I encountered a crash due to a nil dereference in this function.

return 0, errors.New("FTEIDGenerator is not initialized")
}
idGenerator.lock.Lock()
defer idGenerator.lock.Unlock()

Expand Down
3 changes: 2 additions & 1 deletion pfcpiface/messages_conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package pfcpiface

import (
"errors"
"strings"

"github.com/omec-project/upf-epc/logger"
"github.com/wmnsk/go-pfcp/ie"
Expand Down Expand Up @@ -92,7 +93,7 @@ func (pConn *PFCPConn) handleIncomingResponse(msg message.Message) {

func (pConn *PFCPConn) associationIEs() []*ie.IE {
upf := pConn.upf
networkInstance := string(ie.NewNetworkInstanceFQDN(upf.dnn).Payload)
networkInstance := string(ie.NewNetworkInstanceFQDN(strings.Join(upf.dnn, ",")).Payload)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please help me understand why this change is needed. It seems you are trying to add more DNNs in single message?

Copy link
Author

@anaswarac-dac anaswarac-dac Jul 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@thakurajayL ,Currently, the PFCP Association Setup Request is initiated by the SMF. The UPF responds with only one DNN. After that, no further PFCP Association Setup Request is triggered by the SMF, which means there's no opportunity to send additional DNNs separately. Because of this limitation, we include all required DNNs in a single PFCP Association Setup Request to ensure that the SMF receives the full list in one go.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to know which version of 29.244 you are referring to. As specification around this area has changed drastically in release 15/16/17.

Copy link
Author

@anaswarac-dac anaswarac-dac Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@thakurajayL ,We are following 3GPP TS 29.244 v16.5.0 (Release 16), and as per this version, User Plane IP Resource Information (IE Type 116) was removed. Instead, we are required to use UE IP Address Pool Information (IE Type 123) in the PFCP Association Setup Response.

As Per Section 7.2.2.2 of the spec, this IE allows the UPF to report multiple DNN and IP pool mappings to the SMF in a single response by including multiple UE IP Address Pool Information IEs, each containing:

DNN (Network Instance),

UE IP Pool Identity,

and IP version.

This change is necessary because:

The PFCP Association Setup Request is sent only once by the SMF to the UPF.

The UPF must respond once with all supported DNNs and corresponding UE IP pools.

If we send only one DNN in the PFCP Association Setup Response, the SMF won’t be aware of other available DNNs/pools, and there's no further opportunity to send them due to the single-shot nature of the PFCP association procedure.

Spec Reference:

TS 29.244 v16.5.0, Section 6.2.6.2.2:
"Optionally one or more UE IP address Pool Information IE which contains a list of UE IP Address Pool Identities per Network Instance, S-NSSAI and IP version."

DNN | Pool | IP Version

ims | ims-pool | IPv4
internet | internet-pool | IPv4

Will Update the code based on this?

flags := uint8(0x41)

if len(upf.dnn) != 0 {
Expand Down
4 changes: 4 additions & 0 deletions pfcpiface/messages_session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ func (pConn *PFCPConn) handleSessionEstablishmentRequest(msg message.Message) (m

if p.UPAllocateFteid {
var fteid uint32
if pConn.upf.fteidGenerator == nil {
logger.PfcpLog.Warnf("fteid is nill")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger.PfcpLog.Warnf("fteid is nill")
logger.PfcpLog.Warnf("fteid is nil")

pConn.upf.fteidGenerator = NewFTEIDGenerator()
}
fteid, err = pConn.upf.fteidGenerator.Allocate()
if err != nil {
return errProcessReply(err, ie.CauseNoResourcesAvailable)
Expand Down
6 changes: 5 additions & 1 deletion pfcpiface/up4.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,11 @@ func (up4 *UP4) SetUpfInfo(u *upf, conf *Conf) {

logger.PfcpLog.Infof("AccessIP: %v", up4.accessIP)

up4.ueIPPool = MustParseStrIP(conf.CPIface.UEIPPool)
if len(conf.CPIface.DnnList) > 0 {
up4.ueIPPool = MustParseStrIP(conf.CPIface.DnnList[0].UEIPPool)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that a device group can support multiple DNNs (which includes dnn itself and UE IP pool) but only the IP pool for the first dnn is actually used?

Copy link
Contributor

Choose a reason for hiding this comment

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

it seems like up4 should support multiple IP pools now ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@patriciareinoso as far as I see we do not have any active user for UP4. @gab-arrobo are you aware about any users?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, we need to decide what to do with that code (whether maintain it or remove it) because, as far as I know, we are not supporting UP4 anymore. So, I would say that we should remove it. Probably this is something we should bring to the TST meeting

Copy link
Author

Choose a reason for hiding this comment

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

@gab-arrobo When we added multiple DNNs, the integration test for UP4 failed.

} else {
logger.PfcpLog.Fatalln("No UE IP pool configured in DnnList")
}

logger.PfcpLog.Infof("UE IP pool: %v", up4.ueIPPool)

Expand Down
17 changes: 14 additions & 3 deletions pfcpiface/upf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type upf struct {
nodeID string
ippool *IPPool
peers []string
dnn string
dnn []string
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dnn []string
dnns []string

i think that making it plural would represent the multiple dnns better

reportNotifyChan chan uint64
sliceInfo *SliceInfo
readTimeout time.Duration
Expand Down Expand Up @@ -116,17 +116,28 @@ func NewUPF(conf *Conf, fp datapath) *upf {
nodeID = hosts[0]
}

var dnns []string
var ippoolCidr string

// Extract DNN names and UEIPPool from the new DnnList structure
for _, dnnInfo := range conf.CPIface.DnnList {
dnns = append(dnns, dnnInfo.DNN)
if ippoolCidr == "" {
ippoolCidr = dnnInfo.UEIPPool
Copy link
Contributor

Choose a reason for hiding this comment

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

In relation to my previous comment, here it actually looks like the UE IP pool for the last DNN is used, correct? or am I overlooking something?

}
}

u := &upf{
enableUeIPAlloc: conf.CPIface.EnableUeIPAlloc,
enableEndMarker: conf.EnableEndMarker,
enableFlowMeasure: conf.EnableFlowMeasure,
enableGtpuMonitor: conf.EnableGtpuPathMonitoring,
accessIface: conf.AccessIface.IfName,
coreIface: conf.CoreIface.IfName,
ippoolCidr: conf.CPIface.UEIPPool,
ippoolCidr: ippoolCidr,
nodeID: nodeID,
datapath: fp,
dnn: conf.CPIface.Dnn,
dnn: dnns,
peers: conf.CPIface.Peers,
reportNotifyChan: make(chan uint64, 1024),
maxReqRetries: conf.MaxReqRetries,
Expand Down
7 changes: 6 additions & 1 deletion ptf/config/upf.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,12 @@
"cpiface": {
"peers": ["148.162.12.214"],
// [Optional] Below parameters
"dnn": "internet",
"dnn_list": [
Copy link
Contributor

Choose a reason for hiding this comment

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

these parameter are marked as optionals but in pfcpiface/up4.go there is a fatal log if the IP pool is not present. Is the dnn_list mandatory?

Copy link
Contributor

Choose a reason for hiding this comment

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

if we set the dnn_list, should we make sure that both the dnn and ip pool are set ?

{
"dnn": "internet",
"ue_ip_pool": "10.250.0.0/16"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as in the other jsonc file. Should not the ue_ip_pool be removed from below (new line 168)

}
],
"http_port": "8080",
"enable_ue_ip_alloc": false,
// "use_fqdn": "true",
Expand Down
19 changes: 15 additions & 4 deletions test/integration/conf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,12 @@ func BESSConfigUPFBasedIPAllocation() pfcpiface.Conf {
config := BESSConfigDefault()
config.CPIface = pfcpiface.CPIfaceInfo{
EnableUeIPAlloc: true,
UEIPPool: UEPoolUPF,
DnnList: []pfcpiface.DNNInfo{
{
UEIPPool: UEPoolUPF,
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why you are not setting the DNN here?

},
},
}

return config
}

Expand Down Expand Up @@ -87,7 +90,11 @@ func UP4ConfigDefault() pfcpiface.Conf {
}

config.CPIface = pfcpiface.CPIfaceInfo{
UEIPPool: UEPoolCP,
DnnList: []pfcpiface.DNNInfo{
{
UEIPPool: UEPoolCP,
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why you are not setting the DNN here?

},
},
}

return config
Expand All @@ -97,7 +104,11 @@ func UP4ConfigUPFBasedIPAllocation() pfcpiface.Conf {
config := UP4ConfigDefault()
config.CPIface = pfcpiface.CPIfaceInfo{
EnableUeIPAlloc: true,
UEIPPool: UEPoolUPF,
DnnList: []pfcpiface.DNNInfo{
{
UEIPPool: UEPoolUPF,
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why you are not setting the DNN here?

},
},
}

return config
Expand Down