-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
feature: Allow creation for "any"
and "untagged"
as EVCs
#258
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Alopalao thanks for this PR.
On issue #219, we'll need to align the approach that we're going go for on of_core
as well just so this change works as intended. We'll need to give a semantics of probably value 0 or None to give the semantics of any VLAN tag since currently, of_core
is using an logical OR with VlanId.OFPVID_PRESENT
@italovalcy did an initial assessment about this, I'll suggest that you discuss with him and document on that issue the approach we'll go for, and then after then you can adapt this PR here and map the rest of the changes.
Thank you
As discussed:
|
@Alopalao, thanks for anticipating some PRs, as we've just briefly talked on the planning meeting today, let's also make sure that the approach we end up is aligned with MEF vlan range, here's some initial points to include in the discussion:
cc'ing also @ajoaoff since he'll work on mef eline vlan range |
"any"
and "untagged"
as EVCs
"any"
and "untagged"
as EVCs"any"
and "untagged"
as EVCs
- Added tests.
- Added tests.
…into untagged_uni
- Added comments
- Ensured string for `dl_vlan` flow data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature turned out really great @Alopalao, congrats.
I've asked small changes though. Also, could you update the changelog accordingly?
-Added new function `get_priority` -Added enum to yml `Tag.value` -Added `self.subTest` to for loop -Deleted leftover string
Aldo, the chagelog aldo needs to be updated if you could updated it. I've just reviewed your e2e tests too, nicely done. Once these minor suggestions here are addressed I'll approve this PR, and once @italovalcy approves the e2e too since he was also interested in ensuring some flows being asserted then we'll merge this PR here OK? |
- Replace leftover dictionaries with `special_cases`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done, @Alopalao. Thanks for pushing these commits. I'll leave the PR approved, once e2e is also approved feel free to merge this PR.
Also, when this is about to get merged, please notify on #kytos-ng-dev
channel, that until sdntrace_cp
issue 79 is addressed, this feature here is available, but after a controller restart consistency check will for any/untagged won't succeed and as a result, unnecessary periodic redeploys will end up happening, which is OK for now in master
during development since that sdntrace_cp issue is also scheduled for 2023.1
, but let's make that clear for developers when they're using master
version. Maybe we could have a marked xfail
e2e test for this too, and once sdntrace_cp
fixes it, then we remove the @pytest.mark.xfail
decorator
Closes #219
Summary
Removed
required
property from#/components/schemas/Tag
Allow
string
values inuni.tag.value
, (only "any" and "untagged" for now)Local Tests
New
uni
allowed in EVCs requests:Comparing results to OVS
Case: Matching only packets with a VLAN tag regardless of its value (
any
to"4096/4096"
)OVS flow created:
cookie=0xaae4abdd793f8e41, duration=15.738s, table=0, n_packets=0, n_bytes=0, send_fTlow_rem priority=20000,in_port="s1-eth1",vlan_tci=0x1000/0x1000 actions=push_vlan:0x88a8,output:"s1-eth3"
Case: Matching only packets without a VLAN tag (
untagged
to0
)OVS flow created:
cookie=0xaaa16d05a1189a4f, duration=2.930s, table=0, n_packets=0, n_bytes=0, send_flow_rem priority=20000,in_port="s1-eth1",vlan_tci=0x0000/0x1fff actions=push_vlan:0x88a8,output:"s1-eth3"
Packets reception tested with mininet
ping
feature.Cases are working with
CONSISTENCY_CHECK
enabled.End-to-End Tests
These test include the e2e test recently committed.
Also this new PR.
Helpful document
A Google sheet was created. It contains the every result for all the possibles
uni.tag.value
's for both intra and inter switch EVCs. The results shown on this documents reflect the expected results on kytos after this pull request is merged.