AMWA-TV / is-06

AMWA IS-06 NMOS Network Control Specification (Deprecated)
https://specs.amwa.tv/is-06
Apache License 2.0
14 stars 10 forks source link

Proposal for network address translation support #50

Closed garethsb closed 4 years ago

garethsb commented 4 years ago

This PR suggests adding a "Simple NAT policy" resource type.

As I understand it, GET and PATCH could be supported for this resource type at:

/network-devices/{networkDeviceId}/nat/{policyId}

and/or

/network-devices/{networkDeviceId}/interfaces/{portId}/nat/{policyId}

A number of open questions related to this:

garethsb commented 4 years ago

The current schema seems to invite factoring out a small schema that could be used to describe both a "match" sub-object and a "translated" sub-object.

I.e.

{
  "match": {
     "source_ip": "10.1.2.3"
  },
  "translated": {
    "source_ip": "40.1.2.3"
  }
}
garethsb commented 4 years ago

My action from today's meeting was (a) to update the JSON Schemas as identified above, and (b) to add the appropriate endpoints with the appropriate HTTP verbs (GET, PUT, DELETE, not sure about PATCH) to the RAML.

(Edited to reflect that creating these resources is done via PUT rather than POST.)

  1. Presumably, PUT would be supported at the same URLs?

    Yes

  2. Therefore the "id" property (to be used as {policyId} in the URLs) would be allocated by the client (Broadcast Controller) rather the Network Control API?

    Yes, I think was the agreement?

  3. Should GET also be supported at the parent resource URL (i,e. ending .../nat) to get a (paged) collection of NAT policies?

    Yes

  4. Could a top-level collection of policies be offered as well/instead, making it the Network Controller's job to apply the policy at the correct device (switch) or interface.

    Yes (see below)

  5. Would GET also be supported at the parent resources like /network-devices/{networkDeviceId}/interfaces/{portId} or /network-devices/{networkDeviceId}/interfaces/? This information is available in the network device itself, so probably not??

    (Not discussed)

We discussed that in addition to the Network Device scope and the interface scope, we could add a 'global' scope for NAT policies, i.e. at the API top-level, /nat/{policyId}.

Each of these NAT scopes would be optionally implemented by Network Controllers, and could independently respond 501, depending on their underlying capability.

vsachin33 commented 4 years ago

My vote is for /network-address-translations/

..Sachin

garethsb commented 4 years ago
vsachin33 commented 4 years ago

Would be nice to have IPv6 support though it may not be used by any vendors right away, regards ..Sachin

garethsb commented 4 years ago

My action from today's meeting was (a) to update the JSON Schemas as identified above

This proposal is this commit on a sub-branch nat-tuple: https://github.com/AMWA-TV/nmos-network-control/commit/21afafd3ed6625439eaeb7c619d5bf3d31ced603

garethsb commented 4 years ago

The commit 6cbbbe24d33e62cd79ebf1c51e65910afe7c7ed6 adds the top-level /network-address-translations resource for global NAT policies. In order to avoid three repeated sections in the RAML to add Network Device-specific and interface-specific NAT policies, I need to check how best to extract this as a reusable chunk, perhaps using the 'traits' mechanism.

garethsb commented 4 years ago

In order to avoid significant repetition, I have experimented with using !include to allow the NAT API resources to be defined in a separate RAML file. This proposal is on a Git sub-branch nat-scopes: currently a single commit ebd661e87358b8400e06d4020f208ff6214f971e.

garethsb commented 4 years ago

As mentioned in the initial comment, the proposed URL for interface NAT scope has been:

/network-devices/{networkDeviceId}/interfaces/{portId}/nat/{policyId}

My concern here is that the interface.portId attributes in the Network Devices are specified to be the ifAlias of the interface. While this is used as a stable 'handle' to identify an interface, its string format is not well constrained. Our examples include "Ethernet 1/2". If used in the URL, the string would have to be URL encoded as Ethernet%201%2F2, especially to ensure that the '/' is not treated as signifying hierarchy.

If we don't like using portId, we're going to need something else, and only an array index or adding a UUID property to each interface spring to mind right now.

NEOAdvancedTechnology commented 4 years ago

RFC 8343 "A YANG Data Model for Interface Management" has a leaf "name" of type "string". Forward slash is not a reserved character in XML, so NETCONF doesn't have a problem with "Ethernet 1/2".

However RFC 8343 also has a mandatory leaf "if-index" of type int32 {range "1..2147483647"}.

garethsb commented 4 years ago

(What about between policies at the same scope? How is that handled by network devices now? Implementation-specific, or should some ordering be apparent?)

garethsb commented 4 years ago

Doing this raised one new question: should PATCH for any attributes be supported on these resources?

We identified a specific use-case: removing the NAT for one receiver endpoint.

This is potentially even more complicated because the JSON patch algorithm used commonly in NMOS specs doesn't have a way to describe removing one or more specific elements from even a JSON array ["foo", "bar", "baz"].

This is why IS-06 /network-flows/{networkFlowId} supports PATCH but not for "receiver_endpoint_ids" (https://github.com/AMWA-TV/nmos-network-control/blob/v1.0.x/APIs/NetworkControlAPI.raml#L308-L310) and instead has the complicated arrangement of a child resource /receivers with POST and /receivers/{receiverId} supporting DELETE.

garethsb commented 4 years ago

(This point has been made on Slack)

garethsb commented 4 years ago

RAML/schema review:

garethsb commented 4 years ago

If we don't like using portId, we're going to need something else, and only an array index or adding a UUID property to each interface spring to mind right now.

The likely problem with array index is that when interfaces are added or split (for example 100Gb/s into 4 x 25Gb/s) that seems likely to change the ordering of the array. Using ifAlias as the id seems least worst, but will need implementers of both API and clients to be VERY CAREFUL. Let's discuss on next call.

garethsb commented 4 years ago
  1. Would GET also be supported at the parent resources like /network-devices/{networkDeviceId}/interfaces/{portId} or /network-devices/{networkDeviceId}/interfaces/? This information is available in the network device itself, so probably not??

    Yes, this should provide the usual 'child resources' response used in all NMOS specs if we don't adopt the proposal in the next https://github.com/AMWA-TV/nmos-network-control/pull/50#issuecomment-584104043.

garethsb commented 4 years ago

One issue with multiple locations in the API for network-address-translations is that it makes it harder (requires more requests) for a client to discover the applicable NAT policies.

An alternative implementation would be to have all NAT policies under the top-level /network-address-translations resource, and add e.g. a scope property to the JSON objects, which could be e.g.

(Or two new properties, device_id and port_id (possibly under a scope object?), depending on whether it would be better or worse for a query using basic query syntax with a {networkDeviceId} to find everything associated with one Network Device and all its ports or not.)

This would allow avoiding the awkward use of {portId} in an URL path too!

garethsb commented 4 years ago
  1. Did you want to allow IPv4/IPv6 prefix matching/translation as well as matching of individual addresses?

Several participants have now expressed the opinion that in a first version of this functionality, only exact matches are necessary, no ranges or wildcards on either IP addresses or port numbers. So let's stick to that unless a compelling use case is demonstrated.

NEOAdvancedTechnology commented 4 years ago

It is the consensus of the group that (at least for now) this API should map an individual IP address to another single individual IP address (and optionally one port to one other port). In the future, we can consider masks/wildcards.

garethsb commented 4 years ago

As discussed in today's call, asuming we move to only having a top-level /network-address-translations collection, we still want to be able to clearly indicate the scope of each NAT policy and to be able to GET:

Since basic query syntax only supports exact text match, and uses an implicit 'and' operator when multiple query parameters are specified, I believe this simply isn't possible to achieve in a single GET query.

But at least we should design the data model so that it takes at most three non-overlapping GET requests (one per scope!) using basic query syntax, and that a single RQL query could accomplish the same thing if we were to add support for RQL.

Something like:

/network-address-translations?scope= (only global policies) assuming we actually require scope to be present and a string, with the empty string meaning globak /network-address-translations?scope={networkDeviceId} (only policies applied to that network device) /network-address-translations?scope={networkDeviceId}/{portId} (only policies applied to that interface of that network device)

In RQL that would be:

/network-address-translations?query.rql=or(eq(scope,),eq(scope,{networkDeviceId}),eq(scope,{networkDeviceId}/{portId})) (all of the above in a single query)

The alternative approach I outlined, using separate, optional or null-supporting scope.device_id and scope.port_id properties actually seems to be slightly more awkward to use in both basic query syntax and RQL...

@andrewbonney What do you think? Can we do better while sticking to familiar API idioms?

One thing the above doesn't allow is to GET all the NAT policies that are applied at network device or interface scope, rather than for a specific network device or interface. For that RQL matches would be necessary. But I'm not sure this is an important use case.


The above was superseded, following discussion, by the approach in PR #61, which was merged on 13 Feb. Examples of the queries that are now allowed are given in https://github.com/AMWA-TV/nmos-network-control/pull/61#issuecomment-584776971.

garethsb commented 4 years ago

Doing this raised one new question: should PATCH for any attributes be supported on these resources?

Now done for the NAT policy "label".

We identified a specific use-case: removing the NAT for one receiver endpoint.

/network-flows/{networkFlowId} [...] has the complicated arrangement of a child resource /receivers with POST and /receivers/{receiverId} supporting DELETE.

  1. Do we need to adopt the same approach here for NAT policies?

Before I make the additions to the RAML and schemas for this, we would like another discussion to understand the overlap between/need for both the proposed NAT network device interface scope and specifying "receiver_endpoint_ids".

garethsb commented 4 years ago
  1. Do we need to adopt the same approach here for NAT policies?

Before I make the additions to the RAML and schemas for this, we would like another discussion to understand the overlap between/need for both the proposed NAT network device interface scope and specifying "receiver_endpoint_ids".

@vsachin33: The intent of interface support is for scoping purpose. If you know certain endpoints who need to receive translated traffic then you just specify them w/o worrying about to which interface they are connected. Interface support was added to either limit translation on the device or support multiple translation policies on the same device for given (S,G)

@garethsb-sony: Is there is a one-to-one mapping between targeting an Endpoint and scoping the NAT policy to the switch interface it is connected to?

@vsachin33: I believe this would vary vendor to vendor based on their support. Say if you apply NAT policy at virtual layer 3 interface (SVI) – VLAN 100 & network 172.22.31.0/24 and have endpoints connected to different physical ports on the switch eth1/1 & eth1/2. You may choose to apply policy only to eth1/1 by including that receiver endpoint and eth1/2 receives traffic w/o translation. So there won’t be 1:1 mapping between interface and endpoint, Having said that vendor would need to have capabilities to support NAT at layer 2 ports.

@garethsb-sony: OK, I will add the /receivers child resource that will allow adding receiver endpoints to a NAT policy via POST and removing an existing receiver endpoint via DELETE /receivers/{receiverId}.

garethsb commented 4 years ago

Comments from Cisco via @vsachin33:

  1. The narrowest scoped policy would apply in the event of a conflict between policies. (What about between policies at the same scope? How is that handled by network devices now? Implementation-specific, or should some ordering be apparent?)
  1. Do IS-06 NAT policies need an explicit priority? (Like OpenFlow rule priority?)

  2. And/or can conflicting policies be identified at the point that they are PUT and an error response, 400 Bad Request or 409 Conflict, be generated?

garethsb commented 4 years ago
  1. Are these NAT policies designed only for UDP, or also TCP?
    • If so, didn't every policy ought to include a protocol identifier ("UDP", "TCP", or maybe the protocol number from the IP header)?

Jian-Rong Chen (Sony)

The original objective of NAT proposal was trying to solve a specific legacy device and ST2110 over WAN problem. It is very much concerns with media flows, they are all multicast UDP packets. Restrict the current proposal to UDP solves these problems quite nicely.

TCP is much more complicated. It is unicast only, right now no broadcast controller can handle unicast flow control. It is also bi-directional, to make it work, the broadcast controller has to do a lot more work.

I suggest that the NAT policy will check the protocol number in the IP header is UDP, and ignore the policy If it does not match. This could serve as a kind of error detection, since sending TCP packets using multicast address is itself an error anyway.

@garethsb-sony OK, let's add UDP-only clearly somewhere to the updated docs.

garethsb commented 4 years ago
  1. Should we allow PATCH of the NAT policy meaning (match/translated attributes)? I'm thinking not in first version at least?

Jian-Rong Chen (Sony) I couldn’t think of any use case of PATCH of a NAT policy. Maybe a receiver can switch between two streams from the same source node by modifying NAT without change of forwarding tables?

@garethsb-sony OK, keeping as-is for now (only "label" is directly PATCHable, plus "receiver_endpoint_ids" via POST /receivers and DELETE /receivers/{receiverEndpointId}).

garethsb commented 4 years ago

With some updated examples based on the Network Control activity group call this week, this PR is ready for final review and merging into the v1.1-dev branch! 🤞