-
Notifications
You must be signed in to change notification settings - Fork 95
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: support SDM Subscription and Unsubscription for UE Session #123
feat: support SDM Subscription and Unsubscription for UE Session #123
Conversation
dd8ef0b
to
24ef1ec
Compare
479c658
to
3650397
Compare
@andy89923 |
3650397
to
690c8a0
Compare
Hi @andy89923 , |
Hi @andy89923 |
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.
@saileshvvr Sorry for the late review!
After testing w/wo OAuth, this PR can subscribe when a new UE and unsubscribe when the UE's last PDU session is released.
Thanks for the PR, LGTM!
@@ -121,6 +121,17 @@ func (p *Processor) HandlePDUSessionSMContextCreate( | |||
} | |||
} | |||
|
|||
if !p.Context().Ues.UeExists(smContext.Supi) { |
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.
What if the error happens after the SDM data changed?
I think use the defer
would be better in this case:
defer func(err error) {
if err != nil {
if !p.Context().Ues.UeExists(smContext.Supi) {
if problemDetails, err := p.Consumer().
Subscribe(ctx, smContext, smPlmnID); problemDetails != nil {
smContext.Log.Errorln("SDM Subscription Failed Problem:", problemDetails)
} else if err != nil {
smContext.Log.Errorln("SDM Subscription Error:", err)
}
} else {
p.Context().Ues.IncrementPduSessionCount(smContext.Supi)
}
}
}(err)
It looks good to me except the requested changes. Hi @saileshvvr |
Hi @ianchen0119 , Thank you for the feedback. As I was occupied with other work, missed the comments. Thanks! |
Hi @saileshvvr , We can't use the defer function and pass the error to differentiate if the error happens after the defer function. func main() {
var err error
defer func() {
if err != nil {
fmt.Printf("defer: %s\n", err)
} else {
fmt.Println("defer: defer not error")
}
}()
err = errors.New("new error 1")
if err != nil {
return
}
}
/*output
defer: new error 1
*/ You can refer to https://stackoverflow.com/questions/42703707/when-defer-func-evaluates-its-parameters |
Hi @andy89923 , @ianchen0119 , |
Hi @saileshvvr ,
No, we should revise this in the same PR. If you don't have time, I will help revise if the PR isn't ready until next Wednesday. |
Thank you @andy89923 . |
Hi @saileshvvr , Sorry, I don't have time to help with this PR; please fix the comments when you are free! |
Hi @ianchen0119 , @andy89923 , For all other cases (Session Release(UE/NW triggered) or Local Release or Failure cases) we can just call Unsubscribe(). As Unsubscribe() will decrement the session count if more than 1 session else session HTTP DELETE to Unsubscribe. Please let me know your comments so as to update the latest patch accordingly. Thanks |
Thanks @ianchen0119 @andy89923 for uploading the latest changes. |
Description