-
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
Changes from 2 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 |
---|---|---|
|
@@ -210,8 +210,9 @@ func (t *Tls) validate() (bool, error) { | |
} | ||
|
||
type PFCP struct { | ||
Addr string `yaml:"addr,omitempty" valid:"host,required"` | ||
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 commentThe reason will be displayed to describe this comment to others. Learn more.
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. See my comment on L162. |
||
} | ||
|
||
func (p *PFCP) validate() (bool, error) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
package service | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"time" | ||
|
||
"github.com/free5gc/pfcp" | ||
"github.com/free5gc/pfcp/pfcpType" | ||
|
@@ -10,6 +12,46 @@ import ( | |
"github.com/free5gc/smf/internal/pfcp/message" | ||
) | ||
|
||
func toBeAssociatedWithUPF(ctx context.Context, upf *smf_context.UPF) { | ||
var upfStr string | ||
if upf.NodeID.NodeIdType == pfcpType.NodeIdTypeFqdn { | ||
upfStr = fmt.Sprintf("[%s](%s)", upf.NodeID.FQDN, upf.NodeID.ResolveNodeIdToIp().String()) | ||
} else { | ||
upfStr = fmt.Sprintf("[%s]", upf.NodeID.ResolveNodeIdToIp().String()) | ||
} | ||
ensureSetupPfcpAssociation(ctx, upf, upfStr) | ||
} | ||
|
||
func isDone(ctx context.Context) bool { | ||
select { | ||
case <-ctx.Done(): | ||
return true | ||
default: | ||
return false | ||
} | ||
} | ||
|
||
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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. That's true, so I revised it. |
||
err := setupPfcpAssociation(upf, upfStr) | ||
if err == nil { | ||
return | ||
} | ||
now := time.Now() | ||
if alertTime.IsZero() || now.After(alertTime.Add(alertInterval)) { | ||
logger.AppLog.Errorf("Failed to setup an association with UPF%s, error:%+v", upfStr, err) | ||
alertTime = now | ||
} | ||
|
||
if isDone(ctx) { | ||
logger.AppLog.Infof("Canceled association request to UPF%s", upfStr) | ||
return | ||
} | ||
} | ||
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. 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 commentThe 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 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. ok, I got it. 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. but this routine will be blocked in 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.
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.
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. got it |
||
} | ||
|
||
func setupPfcpAssociation(upf *smf_context.UPF, upfStr string) error { | ||
logger.AppLog.Infof("Sending PFCP Association Request to UPF%s", upfStr) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package service | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"os" | ||
"os/signal" | ||
|
@@ -16,8 +17,7 @@ import ( | |
ngapLogger "github.com/free5gc/ngap/logger" | ||
"github.com/free5gc/openapi/models" | ||
pfcpLogger "github.com/free5gc/pfcp/logger" | ||
"github.com/free5gc/pfcp/pfcpType" | ||
"github.com/free5gc/smf/internal/context" | ||
smf_context "github.com/free5gc/smf/internal/context" | ||
"github.com/free5gc/smf/internal/logger" | ||
"github.com/free5gc/smf/internal/pfcp" | ||
"github.com/free5gc/smf/internal/pfcp/udp" | ||
|
@@ -222,10 +222,10 @@ func (smf *SMF) Start() { | |
keyPath = sbi.Tls.Key | ||
} | ||
|
||
context.InitSmfContext(&factory.SmfConfig) | ||
smf_context.InitSmfContext(&factory.SmfConfig) | ||
// allocate id for each upf | ||
context.AllocateUPFID() | ||
context.InitSMFUERouting(&factory.UERoutingConfig) | ||
smf_context.AllocateUPFID() | ||
smf_context.InitSMFUERouting(&factory.UERoutingConfig) | ||
|
||
logger.InitLog.Infoln("Server started") | ||
router := logger_util.NewGinWithLogrus(logger.GinLog) | ||
|
@@ -266,21 +266,15 @@ func (smf *SMF) Start() { | |
} | ||
udp.Run(pfcp.Dispatch) | ||
|
||
for _, upf := range context.SMF_Self().UserPlaneInformation.UPFs { | ||
var upfStr string | ||
if upf.NodeID.NodeIdType == pfcpType.NodeIdTypeFqdn { | ||
upfStr = fmt.Sprintf("[%s](%s)", upf.NodeID.FQDN, upf.NodeID.ResolveNodeIdToIp().String()) | ||
} else { | ||
upfStr = fmt.Sprintf("[%s]", upf.NodeID.IP.String()) | ||
} | ||
if err = setupPfcpAssociation(upf.UPF, upfStr); err != nil { | ||
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 commentThe 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 commentThe 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. |
||
for _, upNode := range smf_context.SMF_Self().UserPlaneInformation.UPFs { | ||
go toBeAssociatedWithUPF(ctx, upNode.UPF) | ||
} | ||
|
||
time.Sleep(1000 * time.Millisecond) | ||
|
||
HTTPAddr := fmt.Sprintf("%s:%d", context.SMF_Self().BindingIPv4, context.SMF_Self().SBIPort) | ||
HTTPAddr := fmt.Sprintf("%s:%d", smf_context.SMF_Self().BindingIPv4, smf_context.SMF_Self().SBIPort) | ||
server, err := httpwrapper.NewHttp2Server(HTTPAddr, smf.KeyLogPath, router) | ||
|
||
if server == nil { | ||
|
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.