Skip to content
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

Remove internal application and release ID if not used by any UE session #583

Merged
merged 17 commits into from
Mar 25, 2022

Conversation

osinstom
Copy link
Contributor

@osinstom osinstom commented Mar 18, 2022

TODO:

  • improve naming
  • fix tests

@osinstom osinstom marked this pull request as ready for review March 23, 2022 11:02
ccascone
ccascone previously approved these changes Mar 25, 2022
Copy link
Contributor

@ccascone ccascone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left only minor comments. Please feel free to merge when ready.

pfcpiface/up4.go Outdated

applicationsEntry, err := up4.p4RtTranslator.BuildApplicationsTableEntry(pdr, up4.conf.SliceID, newAppID)
if err != nil {
releaseID()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious, when not just calling up4.unsafeReleaseInternalApplicationID(appFilter) instead of defining a new function?

pfcpiface/up4.go Outdated
appFilter.appProto = pdr.appFilter.proto

if up4Application, exists := up4.applicationIDs[appFilter]; exists {
// application already exists, increment ref count.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no ref count

pfcpiface/up4.go Outdated
Comment on lines 856 to 869
var appFilter up4ApplicationFilter
if pdr.IsUplink() {
app = application{
appFilter = up4ApplicationFilter{
appIP: pdr.appFilter.dstIP,
appL4Port: pdr.appFilter.dstPortRange,
}
} else if pdr.IsDownlink() {
app = application{
appFilter = up4ApplicationFilter{
appIP: pdr.appFilter.srcIP,
appL4Port: pdr.appFilter.srcPortRange,
}
}

app.appProto = pdr.appFilter.proto
appFilter.appProto = pdr.appFilter.proto
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already saw this in addInternalApplicationIDAndGetP4rtEntry. To reduce duplication, I suggest extracting into a function.

@osinstom osinstom merged commit ec5aa0e into master Mar 25, 2022
@osinstom osinstom deleted the app-id-release branch March 25, 2022 14:53
ccascone added a commit that referenced this pull request Mar 25, 2022
ccascone added a commit that referenced this pull request Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants