Allow rspconfig to disable VLAN on IPMI BMCs#27
Closed
viniciusferrao wants to merge 2 commits into
Closed
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5488a6578
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
viniciusferrao
force-pushed
the
fix/ipmi-vlan-disable
branch
4 times, most recently
from
dc97354 to
1c9627b
Compare
Two pre-existing bugs in the alert on/off conditions: 1. Operator precedence: 'and' with 'or' without parens caused any subcommand with argument matching /^en/ or /^dis/ to silently trigger the alert handler. 2. Loose prefix matching: /^en/ and /^dis/ accepted typos like "enterprise" or "discover". Replace with exact token matching while preserving the "en"/"dis" abbreviations used by snmpmon.pm.
rspconfig vlan= only accepted values 1-4096 with no way to disable VLAN tagging. Users had to resort to raw IPMI commands to clear a stale VLAN after ip=dhcp. - Accept vlan=off/disable/disabled to clear VLAN tagging via standard IPMI parameter 0x14 with the enable bit unset - Fix valid range from 1-4096 to 1-4094 (IEEE 802.1Q) - Use strict digit matching to reject malformed inputs To clear VLAN after a DHCP reset: rspconfig <node> vlan=off Tested on Supermicro IPMI BMC (10.20.0.51). Partially addresses xcat2#3725
viniciusferrao
force-pushed
the
fix/ipmi-vlan-disable
branch
from
1c9627b to
2fa7fca
Compare
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
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.
Summary
rspconfig vlan=only accepted values 1-4096 with no way to disable VLAN taggingChanges:
vlan=off,vlan=disable,vlan=disabledto clear VLAN tagging (standard IPMI parameter 0x14)/^\d+$/) to reject malformed inputs like123abcoralternatives so they're scoped to$subcommand eq "alert"en/disabbreviations used by snmpmon.pmTest plan
Tested on Supermicro IPMI BMC (10.20.0.51):
perlcritic: no new findings (all pre-existing in ipmi.pm)
Fixes xcat2#3725