feature/switch-template-bounce #5735
Conversation
|
I think it will be better if we don't use the locationlog to determine the mac of the node, but to pass the mac as argument in: |
| @@ -474,17 +474,18 @@ sub bouncePort { | |||
| return $TRUE; | |||
| } | |||
|
|
|||
| return $self->_bouncePortCoa($ifindex); | |||
| my $radiusBounce = $self->{_template}{bounce}; | |||
| if (!defined $radiusBounce) { | |||
There was a problem hiding this comment.
Add isenabled($self->{_template}->{snmpDisconnect}) in the if
There was a problem hiding this comment.
If I add that in the if statement it would be a logical error.
Since I am expecting radiusBounce to be defined here https://github.com/inverse-inc/packetfence/pull/5735/files#diff-e7181847796bf1ef221178bfd4822927R483
There was a problem hiding this comment.
Yes, but if you enable snmpDisconnect and add bounce in the template configuration, the disconnection method will call handleReAssignVlanTrapForWiredMacAuth which uses bouncePort and then it will preform COA not SNMP.
There was a problem hiding this comment.
Thank you for the better explanation of the issue.
There was a problem hiding this comment.
To avoid the least surprise.
If you define bounce attributes we will use that when performing a bounce.
If you enable snmpDisconnect then we will use SNMP to disconnect.
It is the responsibility of the person configuring the Template Switch to choose the behavior they want.
I pushed the code to do this.
That makes sense I would need to review the case of VOIP. |
|
@jrouzierinverse i tested passing the mac as argument and everything is working as it should. So i think we should use the argument method instead of locationlog. |
|
LGTM, I will push minor adjustements to documentation before I merged. |
There was a problem hiding this comment.
Last thing I noticed @jrouzierinverse during my review of Switch Template's documentation is that following syntax: VendorName:Attribute-Name1 = value1 is not valid anymore. If you use a Vendor attribute in GUI, resulting attribute is written like any other.
|
@nqb & @jrouzierinverse what are we doing based on Nicolas' last comment? |
Description
Template based bouncePort
NEWS file entries
Enhancements
Issue
fixes #5633
Delete branch after merge
NO
Checklist