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

Updated method for identifying devices by IPv4 address #139

Merged
merged 17 commits into from
May 16, 2023

Conversation

eric-murray
Copy link
Collaborator

@eric-murray eric-murray commented Apr 20, 2023

What type of PR is this?

  • enhancement/feature
  • documentation

What this PR does / why we need it:

This PR introduces the new schema DeviceIpv4Addr, which is used for the ipv4Address property of the Device object. The DeviceIpv4Addr object allows the device to be identified by a combination of its public IPv4 address (publicAddress) and at least one of either its public port (publicPort) or private (device local) IPv4 address (privateAddress).

This modified schema is required to support the scenario where it is desired to identify the device by its private (local) IPv4 address, as this alone is generally insufficient to identify the device, and the public (NATed) IPv4 address must also be provided.

Which issue(s) this PR fixes:

Fixes #34 : Resolving ueAddr ambiguity when it is the UE private address

Special notes for reviewers:

  • For NAT64 scenarios, it has been confirmed that the device IPv6 address alone is sufficient to identify it, and hence this PR does not affect those scenarios
  • Flow rules are unaffected by this PR, with the exception that IPv4 subnets can no longer be specified for the device. If publicPort is specified to identify the device, this value is not used for flow rules. Description of devicePorts has been added to clarify that these are the local ports used directly by the device for flow rules.

Changelog input

Updated qod-api.yaml
- Schema objects DeviceIpv4Addr and SingleIpv4Addr defined
- Device property ipv4Address now of type DeviceIpv4Addr
- Updated error message definitions
- Minor format changes

Updated documentation QoD_API.md

- Schema objects DeviceIpv4Addr and SingleIpv4Addr defined
- Device property ipv4Address now of type DeviceIpv4Addr
- CreateSession property devicePorts renamed deviceAllocatedPorts
- Updated error message definitions
- Minor format changes
Fix error message InvalidDevicePublicPortValue
Updated documentation for change in identifying devices by IPv4 address
Fixed error table numbering
code/API_definitions/qod-api.yaml Outdated Show resolved Hide resolved
code/API_definitions/qod-api.yaml Outdated Show resolved Hide resolved
code/API_definitions/qod-api.yaml Outdated Show resolved Hide resolved
@emil-cheung
Copy link
Collaborator

@hdamker ,
please help to arrange a community voting for:

Observed IP address/Local IP address vs. Public IP address/Private IP address

@eric-murray
Copy link
Collaborator Author

@emil-cheung
I had put some suggestions in the issue, but we can refine them here. Do you still want assigned to be considered as was proposed here? If not, then I think we have the following options:

Vote 1: DeviceIpv4Addr object properties

Option 1 : publicAddress, privateAddress & publicPort
Option 2: observedAddress, localAddress & observedPort

Vote 2: Rename of devicePorts property of CreateSession object

Option 1 : deviceLocalPorts
Option 2 : deviceSourcePorts

@emil-cheung
Copy link
Collaborator

@emil-cheung I had put some suggestions in the issue, but we can refine them here. Do you still want assigned to be considered as was proposed here? If not, then I think we have the following options:

Vote 1: DeviceIpv4Addr object properties

Option 1 : publicAddress, privateAddress & publicPort Option 2: observedAddress, localAddress & observedPort

Vote 2: Rename of devicePorts property of CreateSession object

Option 1 : deviceLocalPorts Option 2 : deviceSourcePorts

Yes, we prefer 'local' instead of 'assigned'.

Replace oneOf with anyOf in DeviceIpv4Addr
@hdamker
Copy link
Collaborator

hdamker commented Apr 27, 2023

@eric-murray As already written in the issues:

Votes are created and open until next Thursday, May 4th, 12:00 UTC:

  1. Vote: DeviceIpv4Addr object properties (from PR #139)
  2. Vote: Rename of devicePorts property of CreateSession object

- reverted deviceAllocatedPorts to devicePorts, but added description of modified usage
- removed `format: ipv4` from SingleIpv4Addr
- Update documentation for devicePorts
- Fix InvalidDevicePortsRanges error decription
- Fix error camaraproject#9 description
- Fix CannotIdentifyDevice error
@eric-murray
Copy link
Collaborator Author

Ready for final review now

code/API_definitions/qod-api.yaml Outdated Show resolved Hide resolved
code/API_definitions/qod-api.yaml Outdated Show resolved Hide resolved
code/API_definitions/qod-api.yaml Outdated Show resolved Hide resolved
- Renamed InsufficientProperties to InsufficientDeviceProperties
- Renamed InconsistentProperties to InconsistentDeviceProperties
- Fixed typo in PhoneNumber object description
Copy link
Collaborator

@sfnuser sfnuser left a comment

Choose a reason for hiding this comment

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

Looks good to me.

documentation/API_documentation/QoD_API.md Outdated Show resolved Hide resolved
Co-authored-by: Ramesh Shanmugasundaram - Spry Fox Networks <[email protected]>
@jlurien
Copy link
Collaborator

jlurien commented May 12, 2023

@hdamker I see that there will be a a lot of conflicts between this PR and the pending one I have to incorporate and update doc within spec, so I will wait to have this merged. If done in the next days I will submit mine before next meeting.

@hdamker
Copy link
Collaborator

hdamker commented May 16, 2023

Seems that we are good to go ... 3+ final reviews and 4 days no new comments.

@hdamker hdamker merged commit 8e0d2ea into camaraproject:main May 16, 2023
@hdamker hdamker mentioned this pull request Jun 17, 2023
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.

Resolving ueAddr ambiguity when it is the UE private address
6 participants