Hi,

First of all, thanks for the prompt fixes from previous issues raised.

Couple more:

1/ FTEID encoding: When the CH bit is set, the IPv4 address field is still present. It should be omitted, even though the IPv4 bit may still be set.

2/ UEIPAddress encoding: In func (f *UEIPAddressFields) MarshalLen() and others in the file it references the 5th bit twice. The second one should be has6thbit, for IPv6. Also if say the CHV4 bit is set then MarshalLen() returns 1, which then makes MarshalTo() return io.ErrUnexpectedEOF because a length of 1 is < 2 in the check even though an IE of just the flags octet is valid. Handling IPv6PL field is also missing.

Hope the feedback helps further the project.

Many thanks

Hi,
Most IEs in 29.244 are optional or conditional.
But from what I can see, the functions such as NewPDI() expect all parameters to be present and non-nil.
If any parmeter is nil, eg to indicate that the application doesn't wish to specify that parm, then the code fails within ie.newGroupedIE() because it tries to dereference pointers at line 563, without checking for nil ones, as it ranges through the provided child IEs, some of which will be nil.
Or have I misunderstood how optional parameters are skipped when creating IEs (or messages)?
Thanks

Hi,

In ie/ue-ip-address-pool-identity.go shouldn't line 68 be:

return string(v[1:(idlen+1)]), nil

instead of

return string(v[1:idlen]), nil

The last character of the name is dropped otherwise?

Thanks

My encapsulation code：

dl := ie.NewDestinationInterface(1)
nt := ie.NewNetworkInstance("11")
ie.NewRemoteGTPUPeer("192.168.2.1", "", dl, nt)

Your encapsulation logic

PCAP file
s_5.zip

Can you help check your encoding problem?

Hi,

In go-pfcp/ie/pfd-contents.go , the type of this field may need to be []byte.
AdditionalDomainNameAndProtocol []string

According to 29.244, Figure 8.2.39-4, this field is a compound field which contains length field. So, I think it should not be of type []string, but []byte instead?

I read the introduction about f_teid in TS29244, but I do n’t understand why you want to calculate the flag like this? Can you explain?

Parameter's name should be far, to not create confusion.

Commit b6fbe25 introduced parsing of UserPlaneIPResourceInformation for association setup response but allows only one ie of that type. We were looking at upf which sends some flows to N9 vs N6, The SMF would probably need to know the Core IP and we were thinking of sending it via AssociationSetupRespoinse. We cannot do that anymore since Access details get overwritten with Core details. Does this sound right? Should i send 2 responses instead?

@TheWayYouMakeMeFeel @wmnsk

Library

		case ie.UserPlaneIPResourceInformation:
			m.UserPlaneIPResourceInformation = i

App

	asres, err := message.NewAssociationSetupResponse(asreq.SequenceNumber,
		ie.NewRecoveryTimeStamp(time.Now()),
		ie.NewNodeID(sourceIP, "", ""),       /* node id (IPv4) */
		ie.NewCause(ie.CauseRequestAccepted), /* accept it blindly for the time being */
		// 0x41 = Spare (0) | Assoc Src Inst (1) | Assoc Net Inst (0) | Tied Range (000) | IPV6 (0) | IPV4 (1)
		//      = 01000001
		ie.NewUserPlaneIPResourceInformation(0x41, 0, accessIP, "", "", ie.SrcInterfaceAccess),
		//ie.NewUserPlaneIPResourceInformation(0x41, 0, coreIP, "", "", ie.SrcInterfaceCore),
	).Marshal() /* userplane ip resource info */

/cc @badhrinathpa @thakurajayL

Hi,

Many thanks for the last fixes and merge.

Looking further, I have a couple more observations:

1/ In f-teid.go at line 215 (v0.0.8) where the CH bit is set, I don't believe there is meant to be an IP address (v4 or v6) expected. If the IPv4 or IPv6 bit is set then that means that the UPF should allocate/choose an IP address, not that one is provided in this message.

2/ In ie.go UnmarshalBinary(), I believe:

    if i.IsVendorSpecific() && l >= 6 {
        i.EnterpriseID = binary.BigEndian.Uint16(b[4:6])
        offset += 2
    }

should be

    if i.IsVendorSpecific() && l >= 6 {
        i.EnterpriseID = binary.BigEndian.Uint16(b[4:6])
        offset += 2
        i.Length -= 2   
    }

Otherwise it panics lower down because the slice upper range is out of bounds.

Many thanks

Currently NewSMFSetID encodes string, but should be same way as NewNodeID instead

It's like this, I researched and understood that the news is just a combination of various IEs, and some IEs are also a combination of other IEs. I found that the IE codes given in the agreement are those independent IE. Therefore, I want to finish the independent IE that has not been completed first.I would like to ask, how do you determine what type of IE this IE belongs to through the agreement.

Packaging process

wireshark Parsing results

In message/header.go the documentation says GTP instead of PFCP for the following functions:

• ParseHeader
• UnmarshalBinary
• Version
• String

Hi, thanks for your gorgeous code, it really helps a lot!
I know that this code done perfect with single IE & group IE & all kinds of messages, but I don't know how to code with array IEs, like multi CreatePDR in one SessionEstablishmentRequest，or multi URRID in one CreatePDR. Does this code done with this kind of array IEs, how to use it? Please tell me, thanks a lot!

You can use this IE, give me an example, how do you encapsulate it? For example, how do you judge the type of each field in IE?

The following code is packaged according to my understanding. Can you see what is wrong?

type UserPlaneIpResourceInformation struct {
	Flags           uint8
	TEIDRange       uint32
	IPv4            net.IP
	IPv6            net.IP
	NetworkInstance string
	SourceInterface uint8
}

What types should TEIDRange, NetworkInstance, and SourceInterface take?

Thank you for your help and guidance

hi @wmnsk
It'll be safer to check if offset>offset+end before
https://github.com/wmnsk/go-pfcp/blob/master/ie/ie.go#L557
also, check if i.MarshalLen()>len(b) before
https://github.com/wmnsk/go-pfcp/blob/master/ie/ie.go#L500

we found "slice bounds out of range" error if
pfcp_heartbeat_request = b'\x20\x01\x00\x0c\x00\x00\x02\x00\x00\x60\x00\x00\xe8\x1f\xe7\xb4'
or
pfcp_heartbeat_request = b'\x20\x01\x00\x0f\x00\x00\x00\xff\xff\xff\x00\x00\x60\x00\x04\xe8\x1f\xdc\x30'

BRs,
Linpoyi

Hi,
I am tryring to parse UserPlane Information from the Setup Association Response provided by Traveling VPP :
https://github.com/travelping/upg-vpp

I got this payoad : []uint8 len: 13, cap: 1456, [41,0,192,168,1,120,6,97,99,99,101,115,115]

I do not really understund the condition l< offset-n , since l=n from above, this mean the offset must be 0 in my case.
In my opinion, offset+n must be < l in order to remain in the array size.

Parsing works great without this Eof check.

Thanks you

Some of the IE fields that are defined as OctetString type should perhaps accept []byte or something instead of string in their constructors, as they expect more complicated contents than just a string (and most of which are defined in another specification 😡).

e.g., PortManagementInformationContainer, BridgeManagementInformationContainer

This is just a reminder issue, to be considered some time later (hopefully).

New IEs

Others

to be examined...

Leaving an idea that I may actually work on or I may just discarded.

// instead of this;
nodeID, err := assocSetupRes.NodeID.NodeID()
cause, err := assocSetupRes.Cause.Cause()

// this would be more useful
nodeID, err := assocSetupRes.NodeID.Value()
cause, err := assocSetupRes.Cause.Value()

// especially when iterating over multiple IEs
for _, i := range multipleIEs {
	v, err := i.Value()
	if err != nil {
		log.Println(v, err)
		continue
	}

	// switch-case is necessary anyway, but no need to call `.IEName()` within it
	// `.Value()` will be less efficient, as its implementation will be a huge switch-case, though
	switch i.Type {
	case ie.NodeID:
		log.Println("NodeID:", v)
	case ie.Cause:
		log.Println("Cause:", v)
	default:
		log.Println("Unknown:", v)
	}
}

As in 3GPP TS 29.244 version 16.5.0 Release 16 8.2.132 Quota Validity Time : It contains the quota validity time in seconds.
Current implementation parses and creates it as time.Time

Currently we need to iterate over children to find specific IEs and to get its values, which is a bit painful considering the number of children. Adding a struct for them and set values directly to its fields would make it easier for users.

cf. README

type CreatePDRFields struct {
	PDRID uint16
	FARID uint32
	...
}

Here is the list of IEs defined in Table 7.x instead of Clause 8.x in TS 29.244

• 1 Create PDR
• 2 PDI
• 3 Create FAR
• 4 Forwarding Parameters
• 5 Duplicating Parameters
• 6 Create URR
• 7 Create QER
• 8 Created PDR
• 9 Update PDR
• 10 Update FAR
• 11 Update Forwarding Parameters
• 12 Update BAR (PFCP Session Report Response)
• 13 Update URR
• 14 Update QER
• 15 Remove PDR
• 16 Remove FAR
• 17 Remove URR
• 18 Remove QER
• 51 Load Control Information
• 54 Overload Control Information
• 58 Application ID's PFDs
• 59 PFD context
• 68 Application Detection Information
• 77 Query URR
• 78 Usage Report (Session Modification Response)
• 79 Usage Report (Session Deletion Response)
• 80 Usage Report (Session Report Request)
• 83 Downlink Data Report
• 85 Create BAR
• 86 Update BAR (Session Modification Request)
• 87 Remove BAR
• 99 Error Indication Report
• 102 User Plane Path Failure Report
• 105 Update Duplicating Parameters
• 118 Aggregated URRs
• 127 Create Traffic Endpoint
• 128 Created Traffic Endpoint
• 129 Update Traffic Endpoint
• 130 Remove Traffic Endpoint
• 132 Ethernet Packet Filter
• 143 Ethernet Traffic Information
• 147 Additional Monitoring Time
• 165 Create MAR
• 166 3GPP Access Forwarding Action Information
• 167 Non-3GPP Access Forwarding Action Information
• 168 Remove MAR
• 169 Update MAR
• 175 Update 3GPP Access Forwarding Action Information
• 176 Update Non 3GPP Access Forwarding Action Information
• 183 PFCP Session Retention Information (within PFCP Association Setup Request)
• 187 User Plane Path Recovery Report
• 189 Join IP Multicast Information IE within Usage Report
• 190 Leave IP Multicast Information IE within Usage Report
• 195 Created Bridge Info for TSC
• 199 TSC Management Information IE within PFCP Session Modification Request
• 200 TSC Management Information IE within PFCP Session Modification Response
• 201 TSC Management Information IE within PFCP Session Report Request
• 203 Clock Drift Control Information
• 205 Clock Drift Report
• 211 Remove SR
• 212 Create SR
• 213 Update SR
• 214 Session Report
• 216 Access Availability Control Information
• 218 Access Availability Report
• 220 Provide ATSSS Control Information
• 221 ATSSS Control Parameters
• 225 MPTCP Parameters
• 226 ATSSS-LL Parameters
• 227 PMF Parameters
• 233 UE IP address Pool Information
• 238 GTP-U Path QoS Control Information
• 239 GTP-U Path QoS Report (PFCP Node Report Request)
• 240 QoS Information in GTP-U Path QoS Report
• 242 QoS Monitoring per QoS flow Control Information
• 247 QoS Monitoring Report
• 252 Packet Rate Status Report
• 254 Ethernet Context Information
• 255 Redundant Transmission Parameters
• 256 Updated PDR
• 261 Provide RDS configuration information
• 263 Query Packet Rate Status IE within PFCP Session Modification Request
• 264 Packet Rate Status Report IE within PFCP Session Modification Response
• 267 UE IP Address Usage Information
• 270 Redundant Transmission Forwarding Parameters
• 271 Transport Delay Reporting
• 272 Partial Failure Information
• 275 RAT Type
• 276 L2TP Tunnel Information
• 277 L2TP Session Information
• 279 Created L2TP Session
• 290 PFCP Session Change Info
• 295 Direct Reporting Information
• 300 MBS Session N4mb Control Information
• 301 MBS Multicast Parameters
• 302 Add MBS Unicast Parameters
• 303 MBS Session N4mb Information
• 304 Remove MBS Unicast Parameters
• 310 MBS Session N4 Control Information
• 311 MBS Session N4 Information
• 316 DSCP to PPI Control Information

To ensure each method returns the expected value, we should add unit tests for them.

• uint8 (ie_uint8_test.go / #63, #69)
• uint16 (ie_uint16_test.go / #66)
• uint32 (ie_uint32_test.go / WIP / 9389c03)
• uint64 (ie_uint64_test.go / 9389c03)
• string (ie_string_test.go / #60 / WIP: tests for grouped IE should be added)
• bool
• time.Duration (ie_duration_test.go / #67)
• time.Time (ie_time_test.go / #68)
• others

I found many helper functions will ParseMultiIEs(i.Payload) directly even it was already parsed. Fox example:

func (i *IE) CreatePDR() ([]*IE, error) {
	if i.Type != CreatePDR {
		return nil, &InvalidTypeError{Type: i.Type}
	}

	return ParseMultiIEs(i.Payload)
}

I think it can enhance performance to add ChildIEs length checking to determine if this IE was parsed.

func (i *IE) CreatePDR() ([]*IE, error) {
	if i.Type != CreatePDR {
		return nil, &InvalidTypeError{Type: i.Type}
	}
	if len(i.ChildIEs) > 0 {
		return i.ChildIEs
	}
	return ParseMultiIEs(i.Payload)
}

Or something I missed?

I would like to ask, how to distinguish these different FQ-CSIDs when assigning values?

I'm trying through your library to mount a session on the travelping UPF but it can't read Network Instance IE. when i asked them they told me it was related to the encoding (your IE instance is encoded as a DNN name while theirs is encoded as an APN name).

My question is if it would be possible to adapt your Network Instance IE?
Thanks in advance

here his the full upg-travelping response:

Network Instance IE encoding is wrong. The value is encoded as DNN name, but UPG expects a APN name there.

TS 29.244 is very confusing on the details of how Network Instance needs to be encoded. It states that APN is not encoded as FQDN string, that implies the same encoding as in the GTP APN IE is used. The specification is silent about how a DNN is encoded into that IE. That has lead some implementations to encode DNN's as plain binary (FQDN with dots in there, no length indicators).

UPG only understands APN encoding at the moment.

It can possibly be useful if the IEs can be retrieved all at once regardless of the message type (without asserting by the message type). The example implementation for AssociationSetupRequest would be like this.

func (m *AssociationSetupRequest) AllIEs() []*ie.IE {
	ies := []*ie.IE{
		m.NodeID,
		m.RecoveryTimeStamp,
		m.UPFunctionFeatures,
		m.CPFunctionFeatures,
		m.SMFSetID,
		m.PFCPSessionRetentionInformation,
		m.PFCPASReqFlags,
	}

	ies = append(ies, m.UserPlaneIPResourceInformation...)
	ies = append(ies, m.AlternativeSMFIPAddress...)
	ies = append(ies, m.UEIPAddressPoolInformation...)
	ies = append(ies, m.GTPUPathQoSControlInformation...)
	ies = append(ies, m.ClockDriftControlInformation...)
	ies = append(ies, m.IEs...)

	return ies
}

*Maybe the inefficient append can be avoided, as the length of returned slice (=number of IEs) can be determined beforehand.

Adding more strict rules(c0519d2) made golangci-lint mad. We need to follow its advice.

How to express 6 bits binary integer in golang? Thanks, guidance

Should it be 'has4thBit' ?

If enable bit 1 with out enable NAI, it will crash.