[ovs-dev] [PATCH 17/17] rstp: refactor RSTP setters.
Jarno Rajahalme
jrajahalme at nicira.com
Wed Nov 19 18:59:34 UTC 2014
Daniele,
Series pushed to master, thank you!
It might be good to have a short statement in NEWS about the conformance testing, what do you think?
Jarno
On Nov 13, 2014, at 4:21 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>-
>
> Thanks!
>
> Jarno
>
> On Nov 6, 2014, at 7:31 AM, Daniele Venturino <daniele.venturino at m3s.it> wrote:
>
>> With this patch setters invoke procedures only if values have changed.
>> Also rstp_set_bridge_address__() keeps the existing priority in the
>> bridge_identifier.
>>
>> Signed-off-by: Daniele Venturino <daniele.venturino at m3s.it>
>> ---
>> lib/rstp.c | 80 ++++++++++++++++++++++++++++++++++++--------------------------
>> 1 file changed, 46 insertions(+), 34 deletions(-)
>>
>> diff --git a/lib/rstp.c b/lib/rstp.c
>> index 5256740..144f2ba 100644
>> --- a/lib/rstp.c
>> +++ b/lib/rstp.c
>> @@ -322,10 +322,12 @@ rstp_set_bridge_address__(struct rstp *rstp, rstp_identifier bridge_address)
>> {
>> VLOG_DBG("%s: set bridge address to: "RSTP_ID_FMT"", rstp->name,
>> RSTP_ID_ARGS(bridge_address));
>> -
>> - rstp->address = bridge_address;
>> - rstp->bridge_identifier = bridge_address;
>> - set_bridge_priority__(rstp);
>> + if (rstp->address != bridge_address) {
>> + rstp->address = bridge_address;
>> + rstp->bridge_identifier &= 0xffff000000000000ULL;
>> + rstp->bridge_identifier |= bridge_address;
>> + set_bridge_priority__(rstp);
>> + }
>> }
>>
>> /* Sets the bridge address. */
>> @@ -370,7 +372,8 @@ rstp_set_bridge_priority__(struct rstp *rstp, int new_priority)
>> {
>> new_priority = ROUND_DOWN(new_priority, RSTP_PRIORITY_STEP);
>>
>> - if (new_priority >= RSTP_MIN_PRIORITY
>> + if (rstp->priority != new_priority
>> + && new_priority >= RSTP_MIN_PRIORITY
>> && new_priority <= RSTP_MAX_PRIORITY) {
>> VLOG_DBG("%s: set bridge priority to %d", rstp->name, new_priority);
>>
>> @@ -584,7 +587,8 @@ rstp_set_bridge_transmit_hold_count__(struct rstp *rstp,
>> int new_transmit_hold_count)
>> OVS_REQUIRES(rstp_mutex)
>> {
>> - if (new_transmit_hold_count >= RSTP_MIN_TRANSMIT_HOLD_COUNT
>> + if (rstp->transmit_hold_count != new_transmit_hold_count
>> + && new_transmit_hold_count >= RSTP_MIN_TRANSMIT_HOLD_COUNT
>> && new_transmit_hold_count <= RSTP_MAX_TRANSMIT_HOLD_COUNT) {
>> struct rstp_port *p;
>>
>> @@ -664,7 +668,8 @@ static void
>> rstp_port_set_priority__(struct rstp_port *port, int priority)
>> OVS_REQUIRES(rstp_mutex)
>> {
>> - if (priority >= RSTP_MIN_PORT_PRIORITY
>> + if (port->priority != priority
>> + && priority >= RSTP_MIN_PORT_PRIORITY
>> && priority <= RSTP_MAX_PORT_PRIORITY) {
>> VLOG_DBG("%s, port %u: set RSTP port priority to %d", port->rstp->name,
>> port->port_number, priority);
>> @@ -713,19 +718,21 @@ rstp_port_set_port_number__(struct rstp_port *port, uint16_t port_number)
>> {
>> /* If new_port_number is available, use it, otherwise use the first free
>> * available port number. */
>> - port->port_number =
>> - is_port_number_available__(port->rstp, port_number, port)
>> - ? port_number
>> - : rstp_first_free_number__(port->rstp, port);
>> + if (port->port_number != port_number || port_number == 0) {
>> + port->port_number =
>> + is_port_number_available__(port->rstp, port_number, port)
>> + ? port_number
>> + : rstp_first_free_number__(port->rstp, port);
>>
>> - set_port_id__(port);
>> - /* [17.13] is not clear. I suppose that a port number change
>> - * should trigger reselection like a port priority change. */
>> - port->selected = false;
>> - port->reselect = true;
>> + set_port_id__(port);
>> + /* [17.13] is not clear. I suppose that a port number change
>> + * should trigger reselection like a port priority change. */
>> + port->selected = false;
>> + port->reselect = true;
>>
>> - VLOG_DBG("%s: set new RSTP port number %d", port->rstp->name,
>> - port->port_number);
>> + VLOG_DBG("%s: set new RSTP port number %d", port->rstp->name,
>> + port->port_number);
>> + }
>> }
>>
>> /* Converts the link speed to a port path cost [Table 17-3]. */
>> @@ -752,7 +759,8 @@ static void
>> rstp_port_set_path_cost__(struct rstp_port *port, uint32_t path_cost)
>> OVS_REQUIRES(rstp_mutex)
>> {
>> - if (path_cost >= RSTP_MIN_PORT_PATH_COST
>> + if (port->port_path_cost != path_cost
>> + && path_cost >= RSTP_MIN_PORT_PATH_COST
>> && path_cost <= RSTP_MAX_PORT_PATH_COST) {
>> VLOG_DBG("%s, port %u, set RSTP port path cost to %d",
>> port->rstp->name, port->port_number, path_cost);
>> @@ -986,8 +994,9 @@ rstp_port_set_administrative_bridge_port__(struct rstp_port *p,
>> {
>> VLOG_DBG("%s, port %u: set RSTP port admin-port-state to %d",
>> p->rstp->name, p->port_number, admin_port_state);
>> - if (admin_port_state == RSTP_ADMIN_BRIDGE_PORT_STATE_DISABLED
>> - || admin_port_state == RSTP_ADMIN_BRIDGE_PORT_STATE_ENABLED) {
>> + if (p->is_administrative_bridge_port != admin_port_state
>> + && (admin_port_state == RSTP_ADMIN_BRIDGE_PORT_STATE_DISABLED
>> + || admin_port_state == RSTP_ADMIN_BRIDGE_PORT_STATE_ENABLED)) {
>>
>> p->is_administrative_bridge_port = admin_port_state;
>> update_port_enabled__(p);
>> @@ -1005,8 +1014,9 @@ rstp_port_set_oper_point_to_point_mac__(struct rstp_port *p,
>> uint8_t new_oper_p2p_mac)
>> OVS_REQUIRES(rstp_mutex)
>> {
>> - if (new_oper_p2p_mac == RSTP_OPER_P2P_MAC_STATE_DISABLED
>> - || new_oper_p2p_mac == RSTP_OPER_P2P_MAC_STATE_ENABLED) {
>> + if (p->oper_point_to_point_mac != new_oper_p2p_mac
>> + && (new_oper_p2p_mac == RSTP_OPER_P2P_MAC_STATE_DISABLED
>> + || new_oper_p2p_mac == RSTP_OPER_P2P_MAC_STATE_ENABLED)) {
>>
>> p->oper_point_to_point_mac = new_oper_p2p_mac;
>> update_port_enabled__(p);
>> @@ -1197,17 +1207,19 @@ static void rstp_port_set_admin_point_to_point_mac__(struct rstp_port *port,
>> {
>> VLOG_DBG("%s, port %u: set RSTP port admin-point-to-point-mac to %d",
>> port->rstp->name, port->port_number, admin_p2p_mac_state);
>> - if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_FORCE_TRUE) {
>> - port->admin_point_to_point_mac = admin_p2p_mac_state;
>> - rstp_port_set_oper_point_to_point_mac__(port,
>> - RSTP_OPER_P2P_MAC_STATE_ENABLED);
>> - } else if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_FORCE_FALSE) {
>> - port->admin_point_to_point_mac = admin_p2p_mac_state;
>> - rstp_port_set_oper_point_to_point_mac__(port,
>> - RSTP_OPER_P2P_MAC_STATE_DISABLED);
>> - } else if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_AUTO) {
>> - port->admin_point_to_point_mac = admin_p2p_mac_state;
>> - /* FIXME fix auto behaviour */
>> + if (port->admin_point_to_point_mac != admin_p2p_mac_state) {
>> + if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_FORCE_TRUE) {
>> + port->admin_point_to_point_mac = admin_p2p_mac_state;
>> + rstp_port_set_oper_point_to_point_mac__(port,
>> + RSTP_OPER_P2P_MAC_STATE_ENABLED);
>> + } else if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_FORCE_FALSE) {
>> + port->admin_point_to_point_mac = admin_p2p_mac_state;
>> + rstp_port_set_oper_point_to_point_mac__(port,
>> + RSTP_OPER_P2P_MAC_STATE_DISABLED);
>> + } else if (admin_p2p_mac_state == RSTP_ADMIN_P2P_MAC_AUTO) {
>> + port->admin_point_to_point_mac = admin_p2p_mac_state;
>> + /* FIXME fix auto behaviour */
>> + }
>> }
>> }
>>
>> --
>> 1.8.1.2
>>
>
More information about the dev
mailing list