[Bug] TEID is not added to teidmap on Handle Create Session Request

[krsna1729]

As part of #67 restructuring we addedfunc (c *Conn) AddTEID(teid uint32, session *Session), but this was used only in CreateSession, which in turn is only used to send CreateSessionRequest.

go-gtp/v2/conn.go

Lines 505 to 515 in a342610

	case ies.FullyQualifiedTEID:
	it, err := i.InterfaceType()
	if err != nil {
	return nil, 0, err
	}
	teid, err := i.TEID()
	if err != nil {
	return nil, 0, err
	}
	sess.AddTEID(it, teid)
	c.AddTEID(teid, sess)

CreateSessionResponse message handling in the examples is missing the step of invoking AddTEID on conn.

This can be rectified in 2 ways -

1. Add explict call to func (c *Conn) AddTEID(teid uint32, session *Session) in the examples

   • Adds to boilerplate
   • Asymmetric use of APIs where app does not need to invoke when sending CSReq but needs to for CSRes

2. Add session arg to func (c *Conn) NewFTEID(ifType uint8, v4, v6 string) so that TEID is generated and stored at the same time.

   • Symmetric for Req/Res handling in the app
   • No additional boilerplate

[wmnsk]

Thanks. I think the second option is better, as it'd be simpler for users.

[krsna1729]

Ooh what i forgot for point 2 is it moves the problem now to CreateSession API being used to allocate a new session, create default bearer, populate the session struct from the IEs and send the message too. In this case NewFTEID is being called before CreateSession.

[wmnsk]

Ah yeah you're right, CreateSession API is troublesome...
How about updating(adding) the TEID that is stored when NewFTEID is called, if FTEID IE is given to CreateSession, and adding some notes in godoc comments above NewFTEID about this behavior(tell users to give nil at first when using it before CreateSession).

1. Call NewFTEID with session = nil (in NewFTEID, do nothing if session == nil).
2. Call CreateSession with the FTEID IE created with NewFTEID (in CreateSession, store the TEID as NewFTEID does normally).

As another issue, I think we should consider the design of API again (or at least the name of API should be changed) because what each TEID in conn.AddTEID and session.AddTEID stands for is too ambiguous. In conn, it is to distinguish the incoming messages(toward which subscriber/session it is sent) but in session it is mainly used to set a TEID to the outgoing message(it can be used for other purposes as it stores everything related to that session but not mandatory for conn/session to work). As a maintainer I'm aware of that but it might not be friendly to users.

[krsna1729]

Yes you have captured all the dilemma I have had for the past few days trying to make this right and intuitive.

[krsna1729]

#30 seems relevant here? Also point 1 and 2 in #72.

[krsna1729]

One question - Why is NewFTEID on usersplane being allocated on v2.Conn instead of v1.Conn?

[wmnsk]

#30 seems relevant here? Also point 1 and 2 in #72.

It seems so. Actually I don't clearly remember what I was thinking when filing #30... 😅

One question - Why is NewFTEID on usersplane being allocated on v2.Conn instead of v1.Conn?

Do you mean the one like this?
https://github.com/wmnsk/go-gtp/blob/master/examples/gw-tester/sgw/s11_handlers.go#L136

NewFTEID's primary purpose is to create an GTPv2 F-TEID IE that is to be sent in GTPv2 messages. In the above case the TEID for S5-U interface is chosen by S-GW and sent to P-GW on GTPv2 Create Session Request. Since GTPv2 is responsible for managing the bearer, it should know the TEID assigned to U-Plane interface, too.