SDCORE-472 SMF Scale issue, SMF not responding with PDU Sess Modify Rsp #75

UpadhyayaAnkur · 2021-09-28T17:05:28Z

No description provided.

fsm/handler.go

UpadhyayaAnkur · 2021-09-29T15:48:58Z

fsm/txnFsm.go

+			httpResponse := &http_wrapper.Response{
+				Header: nil,
+				Status: http.StatusNotFound,
+				Body: models.UpdateSmContextErrorResponse{


I shall fix this, it should be DeleteSmContextErrorResponse.

4xx/5xx Error not defined in spec 29502 for Release SM ctxt error, so shall be initiating 404 Not Found(with Problem detail as per note-1, Spec-29502,
table - 6.1.3.3.4.3.2-2) for context not found case.

transaction/transaction.go

pfcp/handler/handler.go

transaction/transaction.go

vthiruveedula · 2021-09-29T11:56:44Z

fsm/handler.go

+
+	if err := producer.SendPduSessN1N2Transfer(smCtxt, false); err != nil {
+		smCtxt.SubFsmLog.Error("N1N2 transfer failure error, %v ", err.Error())
+		return smf_context.SmStateN1N2TransferPending, fmt.Errorf("N1N2 Transfer failure error, %v ", err.Error())


N1N2 transfer is failed then should move the smcontext state to the pending state?

vthiruveedula · 2021-10-01T10:24:20Z

fsm/handler.go

+type SmEvent uint
+
+const (
+	SmEventInvalid SmEvent = iota


As we discussed, i would like to free5gc fsm to define states, transitions etc. this we can take care in our next commit

Vijaya can you pls elaborate your comment ?

vthiruveedula · 2021-10-01T10:27:57Z

fsm/txnFsm.go

+	smContext := txn.Ctxt.(*smf_context.SMContext)
+
+	//If already Active Txn running then post it to SMF Txn Bus
+	if smContext.ActiveTxn != nil {


can we add error/warning log if active txn is not exist

It is not bug that there is no active txn, it simply checks that if there is already active txn running then queue current txn and for that log is already there at line 80

vthiruveedula · 2021-10-01T10:31:29Z

fsm/txnFsm.go

+			//Initiate N1N2 Transfer
+
+			//nextTxn.StartTxnLifeCycle(SmfTxnFsmHandle)
+			<-nextTxn.Status


this goroutine block until it receives txn status. can we add timer to come out after n sec?

Yes, txn timeout code need to be added.

producer/pdu_session.go

vthiruveedula · 2021-10-01T10:47:08Z

producer/pdu_session.go

 	}

 	// Query UDM
 	if problemDetails, err := consumer.SendNFDiscoveryUDM(); err != nil {
 		smContext.SubPduSessLog.Errorf("PDUSessionSMContextCreate, send NF Discovery Serving UDM Error[%v]", err)
-		var httpResponse = smContext.GeneratePDUSessionEstablishmentReject("UDMDiscoveryFailure")
-		return httpResponse, "UdmError", smContext
+		txn.Rsp = smContext.GeneratePDUSessionEstablishmentReject("UDMDiscoveryFailure")


can we have enum for this reject cause

These are keyword(string) for picking up pre-configured error response from map. However, I agree that it should be enum rather than plain string.

UpadhyayaAnkur · 2021-10-02T18:03:23Z

test container

UpadhyayaAnkur · 2021-10-04T10:12:37Z

test container

yoooou · 2021-10-04T16:18:28Z

test container

yoooou · 2021-10-04T19:02:36Z

test container

thakurajayL · 2021-10-28T04:35:54Z

context/sm_context.go

+	SmStateActivePending
+	SmStateActive
+	SmStateInActivePending
+	SmStateModify


Modify is not stable state right ? Should be ModifyPending ?

thakurajayL · 2021-10-28T04:36:19Z

context/sm_context.go

+	SmStatePfcpRelease
+	SmStateRelease
+	SmStateN1N2TransferPending
+	SmStateMax


Carefully name the states.

States need to be revisited. With initial framework, I just defined for completeness and consistency in naming. I shall be renaming/removing as required for procedures

thakurajayL · 2021-10-28T04:36:56Z

context/sm_context.go

+	SubFsmLog      *logrus.Entry
+
+	//TxnBus per subscriber
+	TxnBus       transaction.TxnBus


Is TxnBus same as PendingTransactions ?

Yes, pending txn queue of that SM context

thakurajayL · 2021-10-28T04:37:54Z

context/sm_context.go

+	SmStateInit SMContextState = iota
+	SmStateActivePending
+	SmStateActive
+	SmStateInActivePending


Is this PDU release pending or something else ?

thakurajayL · 2021-10-28T04:38:31Z

context/sm_context.go

+	SmStatePfcpCreatePending
+	SmStatePfcpModify
+	SmStatePfcpRelease
+	SmStateRelease


is this stable ngap release state ? Where paging should be triggered in case downlink data is received

thakurajayL · 2021-10-28T04:46:29Z

fsm/handler.go

+	Txn interface{}
+}
+
+//Define FSM Func Point Struct here


would be good, if we tell what's expected out of this function.. Looks like it gives next state and error ?

This is definition of function pointer matrix of FSM. Later in init(), I have initialised it with default handlers and overridden with actual handlers wherever applicable.

thakurajayL · 2021-10-28T04:54:12Z

fsm/handler.go

+	SmfFsmHandler[smf_context.SmStatePfcpCreatePending][SmEventPduSessN1N2Transfer] = HandleStatePfcpCreatePendingEventN1N2Transfer
+	SmfFsmHandler[smf_context.SmStateN1N2TransferPending][SmEventPduSessN1N2Transfer] = HandleStateN1N2TransferPendingEventN1N2Transfer
+	SmfFsmHandler[smf_context.SmStateActive][SmEventPduSessModify] = HandleStateActiveEventPduSessModify
+	SmfFsmHandler[smf_context.SmStateActive][SmEventPduSessRelease] = HandleStateActiveEventPduSessRelease


Do you define Abort, timeout events somewhere else ?

Abort shall be defined as part of this matrix itself. Yet to do

thakurajayL · 2021-10-28T04:54:51Z

fsm/handler.go

+func InitFsm() {
+
+	SmfFsmHandler[smf_context.SmStateInit][SmEventPduSessCreate] = HandleStateInitEventPduSessCreate
+	SmfFsmHandler[smf_context.SmStatePfcpCreatePending][SmEventPfcpSessCreate] = HandleStatePfcpCreatePendingEventPfcpSessCreate


Should context replacement state/event handling be part of this array or its handled as a part of function handling.

Yeah, I wanted to keep state/event based handling, however, it required additional changes so kept it as part of new call handling function(as existing) But I shall do it as state/event based as that looks very clean approach

thakurajayL · 2021-10-28T04:57:41Z

fsm/handler.go

+	SmfFsmHandler[smf_context.SmStateActive][SmEventPduSessRelease] = HandleStateActiveEventPduSessRelease
+}
+
+func HandleEvent(smContext *smf_context.SMContext, event SmEvent, eventData SmEventData) error {


Comment in this function will help. 1/2 liner.

thakurajayL · 2021-10-28T05:00:21Z

fsm/handler.go

+	txn := eventData.Txn.(*transaction.Transaction)
+	smCtxt := txn.Ctxt.(*smf_context.SMContext)
+	smCtxt.SubFsmLog.Errorf("unhandled event[%s] ", event.String())
+	return smCtxt.SMContextState, fmt.Errorf("fsm error, unhandled event[%s] and event data[%s] ", event.String(), eventData.String())


What will happen if we hit events here. Just logging or something more ?

Since Error is returned for unhandled event, Txn FSM shall take it to Txn-Failure handling and there we should add sending error rsp as part of txn failure for each type of event.

thakurajayL · 2021-10-28T05:02:59Z

fsm/handler.go

+	txn := eventData.Txn.(*transaction.Transaction)
+	smCtxt := txn.Ctxt.(*smf_context.SMContext)
+
+	producer.SendPFCPRules(smCtxt)


is this always success ? What if peer is down ?

This shall send PFCP msg and then block on response channel. If peer is down then timeout/failure response shall come from PFCP layer

thakurajayL · 2021-10-28T11:24:43Z

fsm/handler.go

+	case smf_context.SessionEstablishFailed:
+		fallthrough
+	default:
+		smCtxt.SubFsmLog.Error("pfcp session establish response failure")


when we come in default case ?

Timeout and Abort cases I had in mind so kept default case

thakurajayL · 2021-10-28T11:27:45Z

fsm/handler.go

+	txn := eventData.Txn.(*transaction.Transaction)
+	smCtxt := txn.Ctxt.(*smf_context.SMContext)
+
+	if err := producer.SendPduSessN1N2Transfer(smCtxt, false); err != nil {


little bit confusion here. SendPduSessN1N2Transfer is the API smf consuming. Why it's part of producer package ?

thakurajayL · 2021-10-28T11:31:04Z

fsm/handler.go

+
+func HandleStateActiveEventPduSessCreate(event SmEvent, eventData *SmEventData) (smf_context.SMContextState, error) {
+
+	//Context Replacement


Log would help here.

thakurajayL · 2021-10-28T11:50:32Z

fsm/txnFsm.go

+
+func (SmfTxnFsm) TxnCtxtRun(txn *transaction.Transaction) (transaction.TxnEvent, error) {
+
+	txn.TxnFsmLog.Debugf("handle event[%v] ", transaction.TxnEventRun.String())


Can this log be in common place instead of every handler adding this log at the start

All txn state level logs I shall remove as generic caller already as log as requested by you.

transaction/transaction.go

UpadhyayaAnkur · 2021-11-11T11:57:19Z

retest this please

UpadhyayaAnkur · 2021-11-12T03:05:52Z

retest container

gajjarvini reviewed Sep 29, 2021

View reviewed changes

fsm/handler.go Show resolved Hide resolved

UpadhyayaAnkur commented Sep 29, 2021

View reviewed changes

transaction/transaction.go Outdated Show resolved Hide resolved

UpadhyayaAnkur commented Sep 29, 2021

View reviewed changes

pfcp/handler/handler.go Outdated Show resolved Hide resolved

UpadhyayaAnkur commented Sep 29, 2021

View reviewed changes

transaction/transaction.go Outdated Show resolved Hide resolved

vthiruveedula reviewed Oct 1, 2021

View reviewed changes

SDCORE-472 SMF Scale issue, SMF not responding with PDU Sess Modify Rsp

c5c621a

UpadhyayaAnkur force-pushed the dev-sdcore-472 branch from 59d708d to c5c621a Compare October 2, 2021 17:54

thakurajayL reviewed Oct 28, 2021

View reviewed changes

UpadhyayaAnkur commented Nov 10, 2021

View reviewed changes

transaction/transaction.go Outdated Show resolved Hide resolved

thakurajayL approved these changes Nov 11, 2021

View reviewed changes

Merge branch 'onf-release3.0.5' into dev-sdcore-472

38aaf44

UpadhyayaAnkur force-pushed the dev-sdcore-472 branch from bb3ff9e to 38aaf44 Compare November 11, 2021 12:08

SDCORE-472 : Review comments incorporated

16f5253

UpadhyayaAnkur merged commit 1872bb2 into onf-release3.0.5 Nov 12, 2021

UpadhyayaAnkur deleted the dev-sdcore-472 branch November 15, 2021 06:32


		func HandleStateActiveEventPduSessCreate(event SmEvent, eventData *SmEventData) (smf_context.SMContextState, error) {

		//Context Replacement


		func (SmfTxnFsm) TxnCtxtRun(txn *transaction.Transaction) (transaction.TxnEvent, error) {

		txn.TxnFsmLog.Debugf("handle event[%v] ", transaction.TxnEventRun.String())

SDCORE-472 SMF Scale issue, SMF not responding with PDU Sess Modify Rsp #75

SDCORE-472 SMF Scale issue, SMF not responding with PDU Sess Modify Rsp #75

Conversation

UpadhyayaAnkur commented Sep 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UpadhyayaAnkur commented Oct 2, 2021

UpadhyayaAnkur commented Oct 4, 2021

yoooou commented Oct 4, 2021

yoooou commented Oct 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UpadhyayaAnkur commented Nov 11, 2021

UpadhyayaAnkur commented Nov 12, 2021