Skip to content

VLAN ACL change detection to be able to warm reload #4733

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
Open
hieunt79 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

VLAN ACL change detection to be able to warm reload #4733

hieunt79 wants to merge 2 commits into from

Conversation

@hieunt79
Copy link

@hieunt79 hieunt79 commented Nov 10, 2025

I want to perform a warm reload when the VLAN ACL changes, so I made the following updates:

  • For port changes:
    • Handle ports in changed_vlans and added_vlans separately.
    • When a port ACL changes (changed_acl_ports), handle it later through Valve._apply_config_changes().
  • For VLAN changes: add: added_vlans, changed_acl_vlans
  • Lastly: optimize return OpenFlow messages in Valve _apply_config_change()

@codecov
Copy link

codecov bot commented Nov 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.42%. Comparing base (429d3b8) to head (07280a6).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4733      +/-   ##
==========================================
+ Coverage   91.36%   91.42%   +0.06%     
==========================================
  Files          46       46              
  Lines        8922     8936      +14     
==========================================
+ Hits         8151     8169      +18     
+ Misses        771      767       -4

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gizmoguy
Copy link
Member

gizmoguy commented Nov 13, 2025

Thanks for the contribution, it appears that this patch has some test failures at the moment that need to be addressed.

If I look at this test failure:

FAIL: test_mclag_warmstart (mininet_multidp_tests.FaucetSingleLAGOnUniqueVLANTest.test_mclag_warmstart)
Test LACP MCLAG after a warm start
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/faucet/faucet/tests/integration/mininet_multidp_tests.py", line 2156, in test_mclag_warmstart
    self.reload_conf(
  File "/faucet-src/clib/mininet_test_base.py", line 2020, in reload_conf
    verify_faucet_reconf_func()
  File "/faucet-src/clib/mininet_test_base.py", line 2784, in verify_faucet_reconf
    self.assertEqual(
AssertionError: 0 != 1 : FAUCET faucet-0-1176 faucet_config_reload_warm_total incremented: 1

It looks like this test was expecting a cold restart, but now with your changes a warm reload happens so the test fails. I think you'll need to update this test (and perhaps others) to account for this behavioural change.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hieunt79 @gizmoguy