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

feat: propagate DG changes to all IMSIs #283

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft
2 changes: 1 addition & 1 deletion proto/server/clientEvtHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -975,7 +975,7 @@ func postConfigHss(client *clientNF, lastDevGroup *configmodels.DeviceGroups, la
}
config.StartImsi = uint64(num)
config.EndImsi = uint64(num)
authSubsData := imsiData[imsi]
authSubsData := getSubscriberAuthDataByUeId("imsi-" + imsi)
Copy link
Contributor

@patriciareinoso patriciareinoso Jan 28, 2025

Choose a reason for hiding this comment

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

this piece of code is executed by the 4G configuration , in which case the authSubsDataColl used in getSubscriberAuthDataByUeId will not be filled and authSubsData will be nil

And it's not 100% clear to me if getAddedImsisList if only used by 5G code since the code is shared between both configurations so I wouldn't touch it.

please do not merge these changes until thi is fixed

if authSubsData == nil {
client.clientLog.Infoln("SIM card details not found for IMSI ", imsi)
continue
Expand Down
69 changes: 52 additions & 17 deletions proto/server/configEvtHandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/json"
"fmt"
"os/exec"
"slices"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -40,14 +41,9 @@ type Update5GSubscriberMsg struct {

var (
execCommand = exec.Command
imsiData map[string]*models.AuthenticationSubscription
rwLock sync.RWMutex
)

func init() {
imsiData = make(map[string]*models.AuthenticationSubscription)
}

func configHandler(configMsgChan chan *configmodels.ConfigMessage, configReceived chan bool) {
// Start Goroutine which will listens for subscriber config updates
// and update the mongoDB. Only for 5G
Expand All @@ -65,9 +61,6 @@ func configHandler(configMsgChan chan *configmodels.ConfigMessage, configReceive
if configMsg.MsgType == configmodels.Sub_data {
imsiVal := strings.ReplaceAll(configMsg.Imsi, "imsi-", "")
logger.ConfigLog.Infoln("received imsi from config channel:", imsiVal)
rwLock.Lock()
imsiData[imsiVal] = configMsg.AuthSubData
rwLock.Unlock()
logger.ConfigLog.Infof("received Imsi [%v] configuration from config channel", configMsg.Imsi)
handleSubscriberPost(configMsg)
if factory.WebUIConfig.Configuration.Mode5G {
Expand Down Expand Up @@ -292,9 +285,10 @@ func getAddedImsisList(group, prevGroup *configmodels.DeviceGroups) (aimsis []st
if group == nil {
return
}
provisionedSubscribers := getProvisionedSubscribers()
Copy link
Contributor

Choose a reason for hiding this comment

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

i am concerned that here you are using the information in authSubsDataColl but that table is only filled up in factory.WebUIConfig.Configuration.Mode5G Do you know what happens in all the function in clientEvtHandler that use getAddedImsisList? are all of them used when factory.WebUIConfig.Configuration.Mode5G ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In clientEvtHandler when 5GMode is disabled there are other functions (in this case, postConfigHss) that take care of the subscribers and these are using another functions (in this case, addedImsis() and deletedImsis()).

for _, imsi := range group.Imsis {
if prevGroup == nil {
if imsiData[imsi] != nil {
if slices.Contains(provisionedSubscribers, imsi) {
aimsis = append(aimsis, imsi)
}
} else {
Expand Down Expand Up @@ -339,6 +333,40 @@ func getDeletedImsisList(group, prevGroup *configmodels.DeviceGroups) (dimsis []
return
}

func getProvisionedSubscribers() []string {
var provisionedSubscribers []string
rawProvisionedSubscribers, errGetMany := dbadapter.AuthDBClient.RestfulAPIGetMany(authSubsDataColl, nil)
if errGetMany != nil {
logger.DbLog.Warnln(errGetMany)
Copy link
Contributor

Choose a reason for hiding this comment

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

we can inmmediately return here

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, if we continue rawProvisionedSubscribers might be uninitialized and return an nil pointer dereference in line 405.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can return an empty slice.

return provisionedSubscribers
}
for _, rawProvisionedSubscriber := range rawProvisionedSubscribers {
ueId, ok := rawProvisionedSubscriber["ueId"].(string)
if !ok {
logger.DbLog.Warnf("cannot retrieve ueId for subscriber: %v", rawProvisionedSubscriber)
continue
}
provisionedSubscribers = append(provisionedSubscribers, ueId)
}
return provisionedSubscribers
}

func getSubscriberAuthDataByUeId(ueId string) *models.AuthenticationSubscription {
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 move this function to clientEvtHandler.go since it is only used there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In clientEvtHandler we are not istantiating the DBClient, therefore moving this function there will increase the imports and in general the complexity. In the client handler we are also already importing getAddedImsisList() func from configEvtHandler

filter := bson.M{"ueId": ueId}
subscriberAuthDataInterface, errGetOne := dbadapter.AuthDBClient.RestfulAPIGetOne(authSubsDataColl, filter)
if errGetOne != nil {
logger.DbLog.Warnln(errGetOne)
Copy link
Contributor

Choose a reason for hiding this comment

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

we can immediately return here

return nil
}
var subscriberAuthData models.AuthenticationSubscription
err := json.Unmarshal(configmodels.MapToByte(subscriberAuthDataInterface), &subscriberAuthData)
if err != nil {
logger.DbLog.Errorf("could not unmarshall subscriber %v", subscriberAuthDataInterface)
return nil
}
return &subscriberAuthData
}

func updateAmPolicyData(imsi string) {
// ampolicydata
var amPolicy models.AmPolicyData
Expand Down Expand Up @@ -572,20 +600,27 @@ func Config5GUpdateHandle(confChan chan *Update5GSubscriberMsg) {
sVal, err := strconv.ParseUint(slice.SliceId.Sst, 10, 32)
if err != nil {
logger.DbLog.Errorf("could not parse SST %v", slice.SliceId.Sst)
return
}
snssai := &models.Snssai{
Sd: slice.SliceId.Sd,
Sst: int32(sVal),
Copy link
Contributor

Choose a reason for hiding this comment

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

We shoud return after line 637. If strconv.ParseUint fails, we can not calculate snssai properly.

}

aimsis := getAddedImsisList(confData.Msg.DevGroup, confData.PrevDevGroup)
for _, imsi := range aimsis {
dnn := confData.Msg.DevGroup.IpDomainExpanded.Dnn
updateAmPolicyData(imsi)
updateSmPolicyData(snssai, dnn, imsi)
updateAmProvisionedData(snssai, confData.Msg.DevGroup.IpDomainExpanded.UeDnnQos, slice.SiteInfo.Plmn.Mcc, slice.SiteInfo.Plmn.Mnc, imsi)
updateSmProvisionedData(snssai, confData.Msg.DevGroup.IpDomainExpanded.UeDnnQos, slice.SiteInfo.Plmn.Mcc, slice.SiteInfo.Plmn.Mnc, dnn, imsi)
updateSmfSelectionProviosionedData(snssai, slice.SiteInfo.Plmn.Mcc, slice.SiteInfo.Plmn.Mnc, dnn, imsi)
/* skip delete case */
if confData.Msg.DevGroup != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can move the first if clause outside of this block to increase readability.

if confData.Msg.DevGroup == nil {
logger.DbLog.Warn("DevGroup is nil in confData.Msg")
return
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If DevGroup is nil we want to remove the subscribers data from the collections, but not update them. I don't fully understand the comment however.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's talk about it. I will ping you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please disregard my comment.

provisionedSubscribers := getProvisionedSubscribers()
for _, imsi := range confData.Msg.DevGroup.Imsis {
/* update only if the imsi is provisioned */
if slices.Contains(provisionedSubscribers, "imsi-"+imsi) {
dnn := confData.Msg.DevGroup.IpDomainExpanded.Dnn
updateAmPolicyData(imsi)
updateSmPolicyData(snssai, dnn, imsi)
updateAmProvisionedData(snssai, confData.Msg.DevGroup.IpDomainExpanded.UeDnnQos, slice.SiteInfo.Plmn.Mcc, slice.SiteInfo.Plmn.Mnc, imsi)
updateSmProvisionedData(snssai, confData.Msg.DevGroup.IpDomainExpanded.UeDnnQos, slice.SiteInfo.Plmn.Mcc, slice.SiteInfo.Plmn.Mnc, dnn, imsi)
updateSmfSelectionProviosionedData(snssai, slice.SiteInfo.Plmn.Mcc, slice.SiteInfo.Plmn.Mnc, dnn, imsi)
}
}
}

dimsis := getDeletedImsisList(confData.Msg.DevGroup, confData.PrevDevGroup)
Expand Down