DHCP Patch for Correct MAC Limitations #4066
Closed
Conversation
This file contains 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
Contributor
reb00tz
commented
May 15, 2019
•
reb00tz
commented
- Redmine Issue: https://redmine.pfsense.org/issues/1605
- Ready for review
git pull origin master
rbgarga
requested changes
May 30, 2019
As per: pfsense@3590f5e#r289076521: added _explicit_ if-else clause to catch "disabled", just in case engine is ever changed where ``isset("disabled") == true``.
…d hosts without fixed IPs Fixed bug when restricting DHCP classes (i.e. when ``$poolconf['denyunknown']=="class"``) accidentally ignoring declared hosts that are not explicitly assigned fixed IPs.
Fixed bug when restricting DHCP classes accidentally ignoring declare…
jim-p
requested changes
Jun 5, 2019
Modified as suggested: https://github.com/pfsense/pfsense/pull/4066/files/3590f5ebb732cb900bc9045eb60b6b365a052b3b#r290902061 (pfsense#595-pfsense#599) https://github.com/pfsense/pfsense/pull/4066/files/3590f5ebb732cb900bc9045eb60b6b365a052b3b#r290902550 (pfsense#892) https://github.com/pfsense/pfsense/pull/4066/files/3590f5ebb732cb900bc9045eb60b6b365a052b3b#r290903116 (pfsense#893) https://github.com/pfsense/pfsense/pull/4066/files/3590f5ebb732cb900bc9045eb60b6b365a052b3b#r290903231 (pfsense#894) https://github.com/pfsense/pfsense/pull/4066/files/3590f5ebb732cb900bc9045eb60b6b365a052b3b#r290903410 (pfsense#896-pfsense#898)
rbgarga
requested changes
Jun 6, 2019
jim-p
approved these changes
Aug 19, 2019
rbgarga
requested changes
Aug 19, 2019
Removed commented out code (previously line pfsense#850) upon @rbgarga's request (as per pfSense PR 4066).
rbgarga
approved these changes
Sep 3, 2019
|
Note: The Redmine issue for this is actually https://redmine.pfsense.org/issues/1605 (the newer one was a duplicate) |
rbgarga
requested changes
Sep 12, 2019
src/etc/inc/services.inc
Outdated
| // subclass for DHCP limiting | ||
| // unknown if the "1:" prefix is still needed, so adding it just in case | ||
| $dhcpdconf .= "subclass \"s_{$dhcpif}\" 1:{$sm['mac']};\n"; | ||
| $dhcpdconf .= "subclass \"s_{$dhcpif}\" {$sm['mac']};\n"; |
There was a problem hiding this comment.
For ethernet connections the subtype is 1 so 1: prefix is mandatory. The line without it and the comment can be removed
src/etc/inc/services.inc
Outdated
| @@ -841,7 +844,13 @@ EOPP; | |||
| $dhcpdconf .= " $deny_action dynamic bootp clients;\n"; | |||
| } | |||
|
|
|||
| if (isset($poolconf['denyunknown'])) { | |||
| // set pool MAC limitations | |||
| if ($poolconf['denyunknown'] == "class") { | |||
There was a problem hiding this comment.
if $poolconf['denyunknown'] is not set PHP will throw a warning. Make sure it is set before try to use its value
Added $poolconf['denyunknown'] guard check as per @rbgarga's warning (https://github.com/pfsense/pfsense/pull/4066/files/6d9ab9b7d5dcb1cf7da1cc678f6aae2a3cfc6207#r323684343)
Removed incorrect DHCP subclass non-prefixed MACs as per rbarga's warning (https://github.com/pfsense/pfsense/pull/4066/files#r323683287). Also edited inline comments.
rbgarga
approved these changes
Nov 25, 2019
2 tasks
|
Superseded by #4066 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.