[Nexthop] Refactor MacAddressCheck to support configurable network interfaces #713
+26
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…facebook#177) <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. --> **Pre-submission checklist** - [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install` - [x] `pre-commit run` # Summary The MacAddressCheck test in `platform_hw_test` (`PlatformHwTest.CorrectMacX86`) validates that a network interface's MAC address matches the base MAC address stored in the EEPROM. The interface was hardcoded to `eth0`. On NH-4010, `eth0` has a locally-administered MAC (02:00:00:...) and is unused while `eth1` has the EEPROM base MAC address, causing `PlatformHwTest.CorrectMacX86` to fail. <!-- Explain the motivation for making this change and any other context that you think would help reviewers of your code. What existing problem does the pull request solve? --> Made the interface configurable via a command-line flag `-mgmt_interface` of `platform_hw_test` (defaults to `eth0`). NOTE: *Existing platforms continue to work without any changes. Platforms like NH-4010 can specify an alternate interface.* ``` ./bin/platform_hw_test -config_file=... -mgmt_interface=eth1 ``` # Test Plan <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? --> Ran the test `platform_hw_test` on NH-4010 platform with `-mgmt_interface=eth1` and it passed without any errors. <!-- If a relevant Github issue exists for this PR, please make sure you link that issue to this PR -->
marif-nexthop
force-pushed
the
marif.fboss-mac-addr-check
branch
from
0f9921b to
b0ed3d8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit installpre-commit runSummary
Remove hardcoded
eth0fromMacAddressCheckand add configurable interface support.The
MacAddressCheckclass had a hardcodedeth0interface when validating that a network interface's MAC address matches the base MAC address stored in EEPROM. This hardcoded limitation prevented the class from being reusable for platforms with different management interface names.In this PR:
MacAddressCheckto remove the hardcodedeth0and make the interface name configurableplatform_hw_testto leverage the improvedMacAddressCheckby adding a-mgmt_interfacecommand-line flageth0when no interface is specifiedTest Plan
Ran the test
platform_hw_teston a Nexthop platform with-mgmt_interface=eth1and it passed without any errors.and it also passed on FBOSS supported HW platform with default options as expected.