-
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
Retry PFCP Association Setup #60
Conversation
logger.AppLog.Errorf("Failed to setup an association with UPF%s, error:%+v", upfStr, err) | ||
} | ||
ctx, cancel := context.WithCancel(context.Background()) | ||
smf_context.SMF_Self().PFCPCancelFunc = cancel |
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.
When to use the cancel() function?
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.
It is used to accommodate dynamic configuration changes that may be implemented in the future.
logger.AppLog.Infof("Canceled association request to UPF%s", upfStr) | ||
return | ||
} | ||
} |
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.
This is a busy loop and can add Sleep 1 second here.
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.
I don't think so. Because PFCP Association Setup Request is retransmitted at the interval defined as ResendRequestTimeOutPeriod
(= 3s) in pfcp library.
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.
ok, I got it.
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.
but this routine will be blocked in now.After(alertTime.Add(alertInterval)
. It will cause ctx.Done()
can't be handled immediately.
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.
for {
err := setupPfcpAssociation(upf, upfStr)
if err == nil {
return
}
alertInterval := smf_context.SMF_Self().AssociationSetupFailedAlertInterval
now := time.Now()
select {
case <-now.After(alertTime.Add(alertInterval)):
logger.AppLog.Errorf("Failed to setup an association with UPF%s, error:%+v", upfStr, err)
case <-ctx.Done():
logger.AppLog.Infof("Canceled association request to UPF%s", upfStr)
return
}
}
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.
now.After(...)
is func After
of type Time
(see https://pkg.go.dev/time#Time.After) , not func After
of package time
.(see https://pkg.go.dev/time#After).
So this routine is not blocked.
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.
got it
@@ -154,6 +158,12 @@ func InitSmfContext(config *factory.Config) { | |||
smfContext.CPNodeID.NodeIdType = pfcpType.NodeIdTypeIpv6Address | |||
smfContext.CPNodeID.IP = addr.IP | |||
} | |||
|
|||
if pfcp.AlertInterval == 0 { |
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.
if AlertInterval is 0, smf should not launch the retry mechanism.
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.
AssociationSetupFailedAlertInterval
represents the interval at which error messages are output, not retry association.
Our system monitors error messages and notifies operators, this value is used to reduce the frequency of notifications when the same error message is output consecutively.
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.
ok, but it is a little bit confusing that no alert trigger in code and only retry to set up association.
Port uint16 `yaml:"port,omitempty" valid:"port,optional"` | ||
Addr string `yaml:"addr,omitempty" valid:"host,required"` | ||
Port uint16 `yaml:"port,omitempty" valid:"port,optional"` | ||
AlertInterval time.Duration `yaml:"associationSetupFailedAlertInterval,omitempty" valid:"type(time.Duration),optional"` |
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.
AssocFailRetryPeriod time.Duration `yaml:"assocFailRetryPeriod,omitempty" valid:"type(time.Duration),optional"`
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.
See my comment on L162.
pkg/service/association.go
Outdated
func ensureSetupPfcpAssociation(ctx context.Context, upf *smf_context.UPF, upfStr string) { | ||
var alertTime time.Time | ||
for { | ||
alertInterval := smf_context.SMF_Self().AssociationSetupFailedAlertInterval |
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.
alertInterval can be moved out for loop
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.
That's true, so I revised it.
No description provided.