OCPBUGS-31679: localnet, multi-homing: introduce localnet alias

[maiqueb]

- What this PR does and why is it needed

This PR introduces a network name alias to use when configuring the localnet networks.

By using this alias, the user can create multiple networks - on different vlans - without having to reconfigure the node's OVN bridge mappings. Essentially, the use can provide a single mapping and piggy-back multiple physical configurations in it.

Users in OpenShift use kubernetes-nmstate to configure these mappings; this way, they get to skip provisioning multiple policies.

- Special notes for reviewers

In this PR we are adding an alias to the network name attribute in the localnet logical switch port mandatory options.

By doing that, we can create multiple networks pointing at the same OVS bridge without having to provision additional bridge mappings, which provides a leaner and more focuses user workflow since any time the admin wants to define a new VLAN, all they need to do is to create a NetworkAttachmentDefinition (and re-use the existing binding).

This would be the new secondary networks definitions:

---
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  name: tenantblue123
spec:
  config: |2
    {
            "cniVersion": "0.4.0",
            "name": "tenantblue123",
            "type": "ovn-k8s-cni-overlay",
            "topology":"localnet",
            "vlanID": 123,
            "physicalNetworkName": "tenantblue",
            "subnets": "192.100.200.0/24",
            "excludeSubnets": "192.100.200.1/32",
            "netAttachDefName": "default/tenantblue123"
    }
---
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  name: tenantblue321
spec:
  config: |2
    {
            "cniVersion": "0.4.0",
            "name": "tenantblue321",
            "type": "ovn-k8s-cni-overlay",
            "topology":"localnet",
            "vlanID": 321,
            "physicalNetworkName": "tenantblue",
            "subnets": "192.150.250.0/24",
            "excludeSubnets": "192.150.250.1/32",
            "netAttachDefName": "default/tenantblue321"
    }

- How to verify it

Provision workloads ataching to those networks, and check the OVS databases contents:

# logical switch ports (localnet)
_uuid               : 014037c2-e34a-4ae1-b798-23133dbba72b
addresses           : [unknown]
dhcpv4_options      : []
dhcpv6_options      : []
dynamic_addresses   : []
enabled             : []
external_ids        : {}
ha_chassis_group    : []
mirror_rules        : []
name                : tenantblue321_ovn_localnet_port
options             : {network_name=tenantblue}
parent_name         : []
port_security       : []
tag                 : 321
tag_request         : 321
type                : localnet
up                  : false


_uuid               : f9092ca2-a876-47c1-9897-1b12be6d573f
addresses           : [unknown]
dhcpv4_options      : []
dhcpv6_options      : []
dynamic_addresses   : []
enabled             : []
external_ids        : {}
ha_chassis_group    : []
mirror_rules        : []
name                : tenantblue123_ovn_localnet_port
options             : {network_name=tenantblue}
parent_name         : []
port_security       : []
tag                 : 123
tag_request         : 123
type                : localnet
up                  : false

# OVS bridges port configuration
kubectl exec ovs-node-m6ch6 -novn-kubernetes -- ovs-vsctl show  
27800d09-e777-4798-86c6-45a5adfc5079
    Bridge breth0
        fail_mode: standalone
        Port patch-tenantblue123_ovn_localnet_port-to-br-int
            Interface patch-tenantblue123_ovn_localnet_port-to-br-int
                type: patch
                options: {peer=patch-br-int-to-tenantblue123_ovn_localnet_port}
        Port breth0
            Interface breth0
                type: internal
        Port patch-breth0_ovn-worker-to-br-int
            Interface patch-breth0_ovn-worker-to-br-int
                type: patch
                options: {peer=patch-br-int-to-breth0_ovn-worker}
        Port patch-tenantblue321_ovn_localnet_port-to-br-int
            Interface patch-tenantblue321_ovn_localnet_port-to-br-int
                type: patch
                options: {peer=patch-br-int-to-tenantblue321_ovn_localnet_port}
        Port eth0
            Interface eth0
    Bridge br-int
        fail_mode: secure
        datapath_type: system
...
        Port patch-br-int-to-breth0_ovn-worker
            Interface patch-br-int-to-breth0_ovn-worker
                type: patch
                options: {peer=patch-breth0_ovn-worker-to-br-int}
        Port patch-br-int-to-tenantblue321_ovn_localnet_port
            Interface patch-br-int-to-tenantblue321_ovn_localnet_port
                type: patch
                options: {peer=patch-tenantblue321_ovn_localnet_port-to-br-int}
        Port patch-br-int-to-tenantblue123_ovn_localnet_port
            Interface patch-br-int-to-tenantblue123_ovn_localnet_port
                type: patch
                options: {peer=patch-tenantblue123_ovn_localnet_port-to-br-int}
        Port "43bd51029b658_3"
            Interface "43bd51029b658_3"
    ovs_version: "3.2.2"

# OVS global configuration - check the external ids
kubectl exec ovs-node-m6ch6 -novn-kubernetes -- ovs-vsctl list open .
_uuid               : 27800d09-e777-4798-86c6-45a5adfc5079
bridges             : [580eae59-db86-4eb8-81f1-da44f4a0a23d, e65ed98f-bc0d-46d3-aef6-bf67aa67e517]
cur_cfg             : 45
datapath_types      : [netdev, system]
datapaths           : {system=a7910ab8-fe2d-4df8-83bd-b889d684fd7d}
db_version          : "8.4.0"
dpdk_initialized    : false
dpdk_version        : "DPDK 22.11.1"
external_ids        : {hostname=ovn-worker, ovn-bridge-mappings="physnet:breth0,tenantblue:breth0", ovn-enable-lflow-cache="true", ovn-encap-ip="10.89.0.5", ovn-encap-type=geneve, ovn-is-interconn="true", ovn-monitor-all="true", ovn-ofctrl-wait-before-clear="0", ovn-openflow-probe-interval="180", ovn-remote="unix:/var/run/ovn/ovnsb_db.sock", ovn-remote-probe-interval="100000", ovn-set-local-ip="true", rundir="/var/run/openvswitch", system-id="02e441c7-2c53-493e-94d1-9c0bc14359b0"}
iface_types         : [afxdp, afxdp-nonpmd, bareudp, erspan, geneve, gre, gtpu, internal, ip6erspan, ip6gre, lisp, patch, srv6, stt, system, tap, vxlan]
manager_options     : []
next_cfg            : 45
other_config        : {bundle-idle-timeout="180", ovn-chassis-idx-02e441c7-2c53-493e-94d1-9c0bc14359b0="", vlan-limit="0"}
ovs_version         : "3.2.2"
ssl                 : []
statistics          : {}
system_type         : fedora
system_version      : "39"

We'll still see multiple patch ports (one per network) while using a single mapping - localnet tenantblue to bridge breth0.

- Description for the changelog

Introduce a localnet network name alias, which allows OVN bridge mapping re-use.

[maiqueb]

@trozet we should agree on a name to put on the configuration to allow the users to re-use an existing mapping.

Maybe this is too low level / detail. How about directly bridgeMapping ?

[phoracek]

bridgeMapping would make sense to me. The user flow looks something like this:

1. Admin uses NNCP to configure bridge-mappings

    ovn:
      bridge-mappings:
      - localnet: localnet1 
        bridge: br-ex 
        state: present 

2. Admin proceeds to create a NAD where they have to reference localnet1 from the example above. For me, the clearest hint to what I should be setting in the NAD would be if it asked for bridgeMapping. Another option would be localnet, but I like it less since we already use the term for the topology

[trozet]

I agree alias is probably too generic, but I think bridgeMapping is not accurate either. The bridge mapping describe a mapping between an OVS bridge and a physical network. The part that you actually put into the NAD is the physical network name. So does that make PhysicalNetwork a good choice? I feel like @jcaamano will have some good thoughts on this.

[maiqueb]

The part that you actually put into the NAD is the physical network name

Good point; that is really what I am using - the value for the localnet port network_name option.

I am OK with PhysicalNetworkName.

@phoracek thoughts ?

[phoracek]

Nice, that works for me

[jcaamano]

I don't follow: you can have multiple networks in the same OVS bridge today with the localnet feature. Each network will use a different patch port.

I am not explaining myself very well. I know you can do that today. I am not saying you can't.

But doing so today requires two bridge mappings physnet:br-ex,localnet:br-ex. OVN-K does not care, is not aware nor handles the bridge mappings, so if this leads to any problem it is not OVN-K responsibility. Something else is setting the bridge mappings and that something else is responsible.

Now with this new attribute PhysicalNetworkName as part of the OVN-K netconf API and set to physnet you only need the default bridge mapping physnet:br-ex to have the equivalent functionality as before. In the general case this attribute makes possible to map multiple networks to the same bridge, through OVN-K API, introducing a gray area of responsibility whereas before it was clearly not OVN-K responsibility because OVN-K was simply not aware.

One example of the checks we could potentially make:
Check that all the localnets with the same PhysicalNetworkName and VlanID don't have overlapping IPs.

We want to support the namespace selector for the secondary localnet UDNs for sure.

So the plan is also to have this field in the CRD as well?

[maiqueb]

We want to support the namespace selector for the secondary localnet UDNs for sure.

So the plan is also to have this field in the CRD as well?

Yes.

Now with this new attribute PhysicalNetworkName as part of the OVN-K netconf API and set to physnet you only need the default bridge mapping physnet:br-ex to have the equivalent functionality as before. In the general case this attribute makes possible to map multiple networks to the same bridge, through OVN-K API, introducing a gray area of responsibility whereas before it was clearly not OVN-K responsibility because OVN-K was simply not aware.

I understand better what you mean.

IMHO, we can simply call out in the docs this is not a gray area of responsibility, and that OVN-K is in no way responsible for validating the physical network configuration - the admin is.

One example of the checks we could potentially make:
Check that all the localnets with the same PhysicalNetworkName and VlanID don't have overlapping IPs.

Thanks for the example; I just don't see how can we perform or enforce such validation: localnet networks (at least for virt use cases) are commonly known for not having subnets defined. I.e. ovn-k has no clue of the IP assignments of the workloads attached to it.

While saying that, I do see the potential for an higher orchestration layer you've called out in previous comments.

[jcaamano]

IMHO, we can simply call out in the docs this is not a gray area of responsibility, and that OVN-K is in no way responsible for validating the physical network configuration - the admin is.

That works for the NAD but I don't think we can do that for the CRD because validation is one of its main purposes.

I just don't see how can we perform or enforce such validation: localnet networks (at least for virt use cases) are commonly known for not having subnets defined.

For the ones that have it defined I guess. We have the responsibility to think about all use cases not just virt.

[maiqueb]

IMHO, we can simply call out in the docs this is not a gray area of responsibility, and that OVN-K is in no way responsible for validating the physical network configuration - the admin is.

That works for the NAD but I don't think we can do that for the CRD because validation is one of its main purposes.

Sure, let's defer validation to the cUDN CRD then. That makes sense to me.

I just don't see how can we perform or enforce such validation: localnet networks (at least for virt use cases) are commonly known for not having subnets defined.

For the ones that have it defined I guess. We have the responsibility to think about all use cases not just virt.

I'm OK with best effort - i.e. validating whenever possible.

[maiqueb]

Renamed the attribute to PhysicalNetworkName.

Please take another look @trozet

[maiqueb]

maybe it wouldn't hurt to have this function idempotent ...

It's really not needed right now, since we only use it from a single place, but could be helpful going forward.