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
apn fqdn encode fix #319
apn fqdn encode fix #319
Conversation
Update the description of the commit and the first comment in this conversation. Have the description point to relevant section of spec. Also should this be handled upstream in go-pfcp library or not. |
f5cf512
to
bec84b7
Compare
pfcpiface/messages.go
Outdated
@@ -73,6 +73,7 @@ func (pc *PFCPConn) handleAssociationSetupRequest(upf *upf, msg message.Message, | |||
// Timestamp shouldn't be the time message is sent in the real deployment but anyway :D | |||
log.Println("Dnn info : ", upf.dnn) | |||
|
|||
networkInstance := encodeFQDN(upf.dnn) |
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.
networkInstance := encodeFQDN(upf.dnn) | |
networkInstance := string(ie.NewNetworkInstanceFQDN(upf.dnn).Payload) |
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.
is ie.NewNetworkInstanceFQDN available in go-pfcp ?
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.
pfcpiface/utils.go
Outdated
func encodeFQDN(fqdn string) string { | ||
b := make([]byte, len(fqdn)+1) | ||
|
||
var offset int | ||
|
||
for _, label := range strings.Split(fqdn, ".") { | ||
|
||
l := len(label) | ||
b[offset] = uint8(l) | ||
copy(b[offset+1:], label) | ||
offset += l + 1 | ||
} | ||
|
||
return string(b) | ||
} | ||
|
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.
we can remove this function
Split the commits for DNN and lint fixes |
bec84b7
to
55b08f2
Compare
pfcpiface/messages.go
Outdated
@@ -73,6 +73,7 @@ func (pc *PFCPConn) handleAssociationSetupRequest(upf *upf, msg message.Message, | |||
// Timestamp shouldn't be the time message is sent in the real deployment but anyway :D | |||
log.Println("Dnn info : ", upf.dnn) | |||
|
|||
networkInstance := string(ie.NewNetworkInstanceFQDN("internet").Payload) |
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.
networkInstance := string(ie.NewNetworkInstanceFQDN("internet").Payload) | |
networkInstance := string(ie.NewNetworkInstanceFQDN(upf.dnn).Payload) |
55b08f2
to
021252e
Compare
retest this please |
1 similar comment
retest this please |
In PFCP versions before 16.3 we send User Plane IP Resource Information in association setup response.
This contains the network instance (DNN) parameter which should be encoded as FQDN. But it is encoded as string.
This can be fixed in the go-pfcp library also. But since the whole User Plane IP Resource Information parameter itself is deprecated, we are doing this change in pfcp agent. This will be updated when we start supporting release 16 and above in 5gc and upf.