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.

Sign up for GitHub

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

Jump to bottom

Enhancement: OVN secondary network should have the possibility to disable mac spoof check #3926

Open
agonzalezrh opened this issue Sep 25, 2023 · 9 comments · May be fixed by #4377
Open

Enhancement: OVN secondary network should have the possibility to disable mac spoof check #3926

agonzalezrh opened this issue Sep 25, 2023 · 9 comments · May be fixed by #4377
Labels
feature/multi-networks Issues related to secondary networks, L3, L2, localnet kind/enhancement All issues/PRs that are new enhancement requests

Comments

@agonzalezrh
Copy link

Using bridge for secondary network allows to specify macspoofchk to enable or disable the mac spoofing check.

It would be useful for the OVN secondary network to have the same feature. I think a simply condition can be added here:

ovn-kubernetes/go-controller/pkg/ovn/base_network_controller_pods.go

Line 563 in c463cea

lsp.PortSecurity = addresses

Use case: nested virtualization
@girishmg
Copy link
Member

This should be easy to add. We have it downstream @cathy-zhou can you PTAL?

@agonzalezrh
Copy link
Author

From my tests is needed as well to set "Addresses" to unknown.

@cathy-zhou
Copy link
Contributor

cathy-zhou commented Sep 26, 2023 •
edited

the way we support it downstream is to add a particular annotation in the net-attach-def to indicate on this particular network, spoofcheck is disabled.

@maiqueb
Copy link
Contributor

maiqueb commented Sep 27, 2023

o/

could you elaborate on the design choice @cathy-zhou ?

I.e. why have this a per-network attribute rather than per pod attachment ?

Like ... assume everything connected to a network like this is "unsafe" ?

@agonzalezrh
Copy link
Author

Do we have any new information about this?

@maiqueb
Copy link
Contributor

maiqueb commented Feb 6, 2024

I've been thinking again about this; it could make sense, and if the setting is done per network - and defaulted to having MAC spoofing - I am OK with it.

@agonzalezrh
Copy link
Author

Thanks for the update, I will work on implement it and create a PR if you agree

@maiqueb
Copy link
Contributor

maiqueb commented Feb 6, 2024 •
edited

Thanks for the update, I will work on implement it and create a PR if you agree

Before you spend time on it, let's hear from another maintainer; @trozet / @jcaamano / @tssurya any thoughts about this feature ?

@trozet
Copy link
Contributor

trozet commented Apr 25, 2024

Yes, we are OK to support this. Seems like NAD is a natural place for the setting to live. Perhaps when we move NAD creation to a CRD it should be a field there.

@tssurya tssurya added kind/enhancement All issues/PRs that are new enhancement requests feature/multi-networks Issues related to secondary networks, L3, L2, localnet labels Apr 25, 2024
@maiqueb maiqueb linked a pull request May 21, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature/multi-networks Issues related to secondary networks, L3, L2, localnet kind/enhancement All issues/PRs that are new enhancement requests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Secondary nets: implement support for nested virtualization maiqueb/ovn-kubernetes
6 participants
@girishmg @tssurya @trozet @agonzalezrh @maiqueb @cathy-zhou