[WIP] configure vlan sub interfaces

[pperiyasamy]

Currently ovs-cni supports selective vlan trunking in the networking configuration and creates ovs port on host ovs bridge with given selective trunk vlan IDs. But the vlan handling on the pod container is upto the applications like creating vlan sub interfaces, ip assignment etc.

This PR attempts to solve this by enhancing ovs-cni to accept IPAM configuration for every VLAN and let ovs-cni creates vlan sub interface and invokes relevant IPAM plugin for every vlan ID for the ip assignment. Of course we could also provide an option in net-attach-def not to create vlan sub interface for every vlan ID in trunks parameter. This is just an initial PR and would make changes accordingly based on your feedback.

The multus-cni and network-attachment-definition-clients need code changes to report all interfaces with ip information in the nw status, will raise PRs on those repos and get their opinon.

/cc @JanScheurich

[kubevirt-bot]

@pperiyasamy: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[kubevirt-bot]

@pperiyasamy: GitHub didn't allow me to request PR reviews from the following users: JanScheurich.

Note that only kubevirt members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Currently ovs-cni supports selective vlan trunking in the networking configuration and creates ovs port on host ovs bridge with given selective trunk vlan IDs. But the vlan handling on the pod container is upto the applications like creating vlan sub interfaces, ip assignment etc.

This PR attempts to solve this by enhancing ovs-cni to accept IPAM configuration for every VLAN and let ovs-cni creates vlan sub interface and invokes relevant IPAM plugin for every vlan ID for the ip assignment. Of course we could also provide an option in net-attach-def not to create vlan sub interface for every vlan ID in trunks parameter. This is just an initial PR and would make changes accordingly based on your feedback.

The multus-cni and network-attachment-definition-clients need code changes to report all interfaces with ip information in the nw status, will raise PRs on those repos and get their opinon.

/cc @JanScheurich

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[kubevirt-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pperiyasamy
To complete the pull request process, please assign phoracek after the PR has been reviewed.
You can assign the PR to them by writing /assign @phoracek in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[phoracek]

@pperiyasamy can we accomplish the same result by using multiple NetworkAttachmentDefinitions?

[JanScheurich]

@pperiyasamy can we accomplish the same result by using multiple NetworkAttachmentDefinitions?

@phoracek: We introduced the support for VLAN-trunking in ovs-cni for pods that require access to up to hundreds of VLANs in parallel. Using one NAD per VLAN would imply hundreds of NADs and hundreds of veth pairs connecting the pod to OVS. This was considered impractical. Furthermore, the set of trunked VLANs changes over time. With trunks it is possible to adapt by updating the trunk NAD and restart the pods. Using individual access NADs would require updating the pod specs every time.

We have realized now that just exposing the raw VLANs to pods on a trunk interface is sometimes not enough. Applications may want to use additional CNI services like interface creation, IPAM or other interface configuration also for the trunked networks.

I appreciate that creating hundreds of VLAN sub-interfaces would, from the pod perspective, look not much different than hundreds of veth endpoints, even though it does make a difference for OVS. But some of our target applications use raw packet sockets to handle the VLAN tags internally and would still like to rely on IPAM to provide an IP address on each and every trunked VLAN, either in-band through dhcp IPAM or out-of-band through network status annotations.

[phoracek]

I see, makes sense.

How would you like to handle this? Shall we first review the proposed API and approach before we get to implementation of tests and docs?

[JanScheurich]

I also thought the end-to-end proposal including supporting changes in net-attach-def-client and multus should first be discussed in the NPWG before we go ahead with implementation everywhere.

[pperiyasamy]

Thanks @phoracek and @JanScheurich for your input , Let me propose the changes in multus-cni and net-attach-def-client via pull requests and get their feedback.

[kubevirt-bot]

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

📝 Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

• 1f0fda3 Revert "make ipam struct as private"

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[kubevirt-bot]

@pperiyasamy: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.