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.

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

Fix VLANs and LAGs #3697

Merged
merged 1 commit into from
Jan 8, 2024
Merged

Fix VLANs and LAGs #3697

merged 1 commit into from
Jan 8, 2024

Conversation

milan-zededa
Copy link
Contributor

Support for VLANs and LAGs has been added to EVE quite some time ago, but it has not been retested since. As we are soon planning to productize this feature, this PR contains a set of patches for regressions found while testing EVE with VLAN and LAG configurations.

Patches are mostly about skipping L2-only ports and not trying to use them for connectivity tests and HTTP requests.

I have also prepared an example for eden: lf-edge/eden#939
Hopefully in the near future we will be able to turn this into a fully automated test.

Support for VLANs and LAGs has been added to EVE quite some time ago,
but it has not been retested since. As we are soon planning
to productize this feature, this commit contains a set of patches
for regressions found while testing EVE with VLAN and LAG
configurations.

Patches are mostly about skipping L2-only ports and not trying to use
them for connectivity tests and HTTP requests.

Signed-off-by: Milan Lenco <milan@zededa.com>
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (03b8972) 19.60% compared to head (10b6230) 19.52%.

Files Patch % Lines
pkg/pillar/cmd/diag/diag.go 0.00% 3 Missing ⚠️
pkg/pillar/types/dns.go 76.92% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3697      +/-   ##
==========================================
- Coverage   19.60%   19.52%   -0.09%     
==========================================
  Files         232      232              
  Lines       50789    50787       -2     
==========================================
- Hits         9959     9917      -42     
- Misses      40104    40137      +33     
- Partials      726      733       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -1684,29 +1684,6 @@ func doActivateTail(ctx *domainContext, status *types.DomainStatus,
status.UUIDandVersion, status.DisplayName)
}

// VLAN filtering could have been enabled on the bridge immediately after
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this no longer needed due to some unknown netlink code? (New kernel version??)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some time ago, this was moved from domainmgr to zedrouter (NI Reconciler): https://github.com/lf-edge/eve/blob/master/pkg/pillar/nireconciler/linuxitems/vlanbridge.go#L106-L131
I just forgot to remove this no longer used function back then.

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

LGTM

@eriknordmark eriknordmark merged commit e683721 into lf-edge:master Jan 8, 2024
39 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants