Skip to content

VLAN filtering on VIF#7123

Open
semarie wants to merge 2 commits into
xapi-project:masterfrom
xcp-ng:dev/srt/master/vlan-filtering
Open

VLAN filtering on VIF#7123
semarie wants to merge 2 commits into
xapi-project:masterfrom
xcp-ng:dev/srt/master/vlan-filtering

Conversation

@semarie

@semarie semarie commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • update idl/datamodel to add trunks property on VIF
  • add validation constraints (trunks filtering on access port isn't valid)
  • add support inside vif-real for initial configuration
  • add support inside xenopsd for dynamic changes
  • add test coverage to new attributes

References:

Internal references:

  • XCPNG-768 : initial spike (with PoC)
  • XCPNG-3258: design
  • XCPNG-3259: implementation

@semarie semarie force-pushed the dev/srt/master/vlan-filtering branch from 32d343e to 013dff3 Compare June 10, 2026 10:16
github-advanced-security[bot]
github-advanced-security AI found potential problems Jun 10, 2026
View reviewed changes
Comment thread ocaml/xenopsd/scripts/vif-real Fixed
Comment thread ocaml/xenopsd/scripts/vif-real Fixed
Comment thread ocaml/xenopsd/scripts/vif-real Fixed
Comment thread ocaml/xenopsd/scripts/vif-real Fixed
Comment thread ocaml/xenopsd/scripts/vif-real Fixed
@semarie semarie force-pushed the dev/srt/master/vlan-filtering branch from 013dff3 to 3b3f2b5 Compare June 10, 2026 10:21
github-advanced-security[bot]
github-advanced-security AI found potential problems Jun 10, 2026
View reviewed changes
Comment thread ocaml/xenopsd/scripts/vif-real Fixed
Comment thread ocaml/xenopsd/scripts/vif-real Fixed
@semarie semarie force-pushed the dev/srt/master/vlan-filtering branch 3 times, most recently from 54e858c to f62adbe Compare June 10, 2026 13:34
@semarie semarie marked this pull request as draft June 10, 2026 14:30
@semarie semarie force-pushed the dev/srt/master/vlan-filtering branch from f62adbe to 86df0ef Compare June 10, 2026 14:46
@semarie semarie marked this pull request as ready for review June 10, 2026 14:55
psafont
psafont reviewed Jun 11, 2026
View reviewed changes
Comment thread ocaml/xenopsd/scripts/vif-real
psafont
psafont reviewed Jun 11, 2026
View reviewed changes
Comment thread ocaml/xenopsd/xc/xenops_server_xen.ml
psafont
psafont requested changes Jun 11, 2026
View reviewed changes
Comment thread ocaml/idl/schematest.ml
@semarie semarie force-pushed the dev/srt/master/vlan-filtering branch from 86df0ef to ccb6e9d Compare June 11, 2026 14:13
@semarie semarie mentioned this pull request Jun 12, 2026
Merged
gthvn1
gthvn1 reviewed Jun 12, 2026
View reviewed changes
Comment thread ocaml/idl/datamodel_lifecycle.ml Outdated
@semarie
vlan_tag_invalid: change the documentation string
b86a526 
Use a more generic message to permit to use it in other context than VLAN creation.

Signed-off-by: Sebastien Marie <semarie@kapouay.eu.org>
@semarie semarie force-pushed the dev/srt/master/vlan-filtering branch from ccb6e9d to 27a9666 Compare June 12, 2026 08:36
gthvn1
gthvn1 reviewed Jun 12, 2026
View reviewed changes
Comment thread ocaml/xapi/xapi_vif.ml Outdated
gthvn1
gthvn1 reviewed Jun 12, 2026
View reviewed changes
Comment thread ocaml/xapi/xapi_vlan.ml
@@ -83,12 +91,7 @@ let create ~__context ~tagged_PIF ~tag ~network =
~pif_topo ~tag ;
Xapi_network_helpers.assert_vlan_network_compatible_with_pif ~__context

@gthvn1 gthvn1 Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it is not related directly with your code but shouldn't we add, now that you are introducing trunks VIF, Xapi_pif_helpers.assert_network_compatible_with_trunks_on_vif ~__context ~network.

@semarie semarie Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adding the check is possible, but it should never trigger : currently, there is already a check when you a creating the new PIF (tagged_PIF) on Network with VIF using trunks, which is done before creating the VLAN object itself.

see test_trunks_coherence_pif_vlan()

@gthvn1

gthvn1 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Have you tried using xe to create some VIF with wrong trunk ID ?

gthvn1
gthvn1 requested changes Jun 12, 2026
View reviewed changes

@gthvn1 gthvn1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some questions but otherwise, the PR (even though I'm a dummy when it comes to networking) looks really good, congrats!

@semarie
VLAN filtering on VIF
0dbfb8b 
- update idl/datamodel to add trunks property on VIF
- add validation constraints (trunks filtering on access port isn't valid)
- add support inside `vif-real` for initial configuration
- add support inside xenopsd for dynamic changes
- add test coverage to new attributes

References:

- Design document: https://github.com/xapi-project/xen-api/blob/788869e5a92e10332ba2428eb91e5a2caf4c7131/doc/content/design/vlan-filtering.md

Signed-off-by: Sebastien Marie <semarie@kapouay.eu.org>
@semarie semarie force-pushed the dev/srt/master/vlan-filtering branch from 27a9666 to 0dbfb8b Compare June 12, 2026 09:43
@semarie

semarie commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Have you tried using xe to create some VIF with wrong trunk ID ?

It depends what you call a "wrong trunk ID".

If it is an invalid VLAN tag (outside the valid range of VLAN), yes it is working as expected.

# xe vif-param-set uuid=80e5f415-c385-ea18-28c7-4c8ababcc76a trunks=15,9999
The VLAN tag you gave was invalid -- it must be between 0 and 4094. The parameter echoes the VLAN tag you gave.
VLAN: 9999
# xe vif-param-set uuid=80e5f415-c385-ea18-28c7-4c8ababcc76a trunks=15,20
#

If it is a valid VLAN tag but not used on the network, it doesn't trigger any error as XAPI have not way to know if the specified VLAN tag is used or not outside the managed network (the VLAN could exist outside XAPI). So as long the VLAN tag is valid, you could set it, and use it in the VM (to send and receive tagged Ethernet packets).

@semarie semarie requested a review from gthvn1 June 12, 2026 10:08
@gthvn1

gthvn1 commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

If it is an invalid VLAN tag (outside the valid range of VLAN), yes it is working as expected.

Yes I wanted to say invalid 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gthvn1 gthvn1 gthvn1 approved these changes

@psafont psafont psafont approved these changes

+1 more reviewer

@github-advanced-security github-advanced-security[bot] github-advanced-security[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@semarie @gthvn1 @psafont @github-advanced-security