From 3017e0281d62e51d857af000bb5b7e721f9ba29d Mon Sep 17 00:00:00 2001 From: olabjorsne Date: Tue, 16 Feb 2021 15:02:10 +0100 Subject: [PATCH] DCP handle alias id request Do not store alias name for each LLDP peer. Instead build and check alias when alias is part of id request received. Exclude alias name from DCP response as it shall only be used for filtering requests. --- src/common/pf_dcp.c | 76 ++++++++++++++++++++++++------------ src/common/pf_lldp.c | 91 ++++++++++++++++++++++++++++++++----------- src/common/pf_lldp.h | 10 +++++ src/device/pf_cmina.c | 5 +-- src/pf_types.h | 2 - 5 files changed, 132 insertions(+), 52 deletions(-) diff --git a/src/common/pf_dcp.c b/src/common/pf_dcp.c index 09d548f7..b4a8dda0 100644 --- a/src/common/pf_dcp.c +++ b/src/common/pf_dcp.c @@ -369,7 +369,7 @@ static int pf_dcp_put_block ( * @param sub In: Sub-option key. * @param request_is_identify In: Usage in response to Identify request * (skips some blocks) - * @param append_alias_name In: Append alias name + * @param alias_name In: Alias name appended if != NULL * @return */ static int pf_dcp_get_req ( @@ -380,7 +380,7 @@ static int pf_dcp_get_req ( uint8_t opt, uint8_t sub, bool request_is_identify, - bool append_alias_name) + const char * alias_name) { int ret = 0; /* Assume all OK */ uint8_t block_error = PF_DCP_BLOCK_ERROR_NO_ERROR; @@ -492,8 +492,13 @@ static int pf_dcp_get_req ( value_length = (uint16_t)strlen ((char *)p_value); break; case PF_DCP_SUB_DEV_PROP_ALIAS: - skip = append_alias_name ? 0 : 1; - value_length = (uint16_t)strlen ((char *)p_value); + skip = alias_name ? 0 : 1; + if (skip == 0) + { + value_length = (uint16_t)strlen (alias_name); + p_value = (uint8_t *)alias_name; + ret = 0; + } break; case PF_DCP_SUB_DEV_PROP_VENDOR: value_length = (uint16_t)strlen ((char *)p_value); @@ -1442,7 +1447,6 @@ static int pf_dcp_identify_req ( bool first = true; /* First of the blocks */ bool match = false; /* Is it for us? */ bool filter = false; /* Is it IdentifyFilter or IdentifyAll? */ - bool alias_request = false; /*Is this a request via an alias name?*/ uint8_t * p_src; uint16_t src_pos = 0; pf_ethhdr_t * p_src_ethhdr; @@ -1463,6 +1467,8 @@ static int pf_dcp_identify_req ( uint8_t * p_value; uint8_t block_error; const pnet_ethaddr_t * mac_address = pf_cmina_get_device_macaddr (net); + char alias[PF_ALIAS_NAME_MAX_SIZE]; /** Terminated */ + char * p_req_alias_name = NULL; /* For diagnostic logging */ bool identify_all = false; @@ -1754,24 +1760,6 @@ static int pf_dcp_identify_req ( ret = -1; } break; - case PF_DCP_SUB_DEV_PROP_ALIAS: - alias_position = src_pos; - alias_len = src_block_len; - if ( - (memcmp (p_value, &p_src[src_pos], value_length) != 0) || - (src_block_len != value_length)) - { - if (first == true) - { - alias_request = true; - filter = true; - } - } - else - { - match = false; - } - break; case PF_DCP_SUB_DEV_PROP_INSTANCE: if (filter == true) { @@ -1840,6 +1828,46 @@ static int pf_dcp_identify_req ( src_block_len = ntohs (p_src_block_hdr->block_length); src_pos += sizeof (*p_src_block_hdr); } + else if ( + (p_src_block_hdr->option == PF_DCP_OPT_DEVICE_PROPERTIES) && + (p_src_block_hdr->sub_option == PF_DCP_SUB_DEV_PROP_ALIAS)) + { + alias_position = src_pos; + alias_len = src_block_len; + if (src_block_len < PF_ALIAS_NAME_MAX_SIZE) + { + memcpy (alias, &p_src[src_pos], src_block_len); + alias[src_block_len] = '\0'; + if (pf_lldp_is_alias_matching (net, alias)) + { + if (first == true) + { + p_req_alias_name = alias; + filter = true; + } + } + else + { + match = false; + } + } + else + { + match = false; + } + + src_pos += src_block_len; + + /* Skip padding to align on uint16_t */ + while (src_pos & 1) + { + src_pos++; + } + /* Prepare for the next round */ + p_src_block_hdr = (pf_dcp_block_hdr_t *)&p_src[src_pos]; + src_block_len = ntohs (p_src_block_hdr->block_length); + src_pos += sizeof (*p_src_block_hdr); + } else { LOG_DEBUG ( @@ -1866,7 +1894,7 @@ static int pf_dcp_identify_req ( device_options[ix].opt, device_options[ix].sub, true, - alias_request); + p_req_alias_name); } /* Insert final response length and ship it! */ diff --git a/src/common/pf_lldp.c b/src/common/pf_lldp.c index eac1fc3c..b190a039 100644 --- a/src/common/pf_lldp.c +++ b/src/common/pf_lldp.c @@ -502,7 +502,11 @@ static void pf_lldp_receive_timeout ( pf_port_t * p_port_data = (pf_port_t *)arg; p_port_data->lldp.rx_timeout = 0; - LOG_WARNING (PF_LLDP_LOG, "LLDP(%d): Receive timeout expired\n", __LINE__); + LOG_INFO ( + PF_LLDP_LOG, + "LLDP(%d): Port %d, receive timeout expired\n", + __LINE__, + p_port_data->port_num); pf_pdport_peer_lldp_timeout (net, p_port_data->port_num); pf_lldp_invalidate_peer_info (net, p_port_data->port_num); @@ -1816,6 +1820,37 @@ int pf_lldp_generate_alias_name ( return 0; } +bool pf_lldp_is_alias_matching (pnet_t * net, const char * alias) +{ + char port_alias[PF_ALIAS_NAME_MAX_SIZE]; /** Terminated */ + int port; + pf_port_iterator_t port_iterator; + pf_port_t * p_port_data = NULL; + + pf_port_init_iterator_over_ports (net, &port_iterator); + port = pf_port_get_next (&port_iterator); + while (port != 0) + { + p_port_data = pf_port_get_state (net, port); + if ( + p_port_data->lldp.is_peer_info_received && + (pf_lldp_generate_alias_name ( + p_port_data->lldp.peer_info.port_id.string, + p_port_data->lldp.peer_info.chassis_id.string, + port_alias, + sizeof (port_alias)) == 0)) + { + if (strcmp (alias, port_alias) == 0) + { + return true; + } + } + + port = pf_port_get_next (&port_iterator); + } + return false; +} + /** * @internal * Store peer information @@ -1865,8 +1900,6 @@ void pf_lldp_update_peer ( int loc_port_num, const pf_lldp_peer_info_t * lldp_peer_info) { - int error = 0; - char new_alias[PF_ALIAS_NAME_MAX_SIZE]; /** Terminated */ pf_port_t * p_port_data = pf_port_get_state (net, loc_port_num); pf_lldp_reset_peer_timeout (net, loc_port_num, lldp_peer_info->ttl); @@ -1881,27 +1914,41 @@ void pf_lldp_update_peer ( return; } - error = pf_lldp_generate_alias_name ( - lldp_peer_info->port_id.string, - lldp_peer_info->chassis_id.string, - new_alias, - sizeof (new_alias)); - if (!error && (strcmp (new_alias, net->cmina_current_dcp_ase.alias_name) != 0)) - { - LOG_INFO ( - PF_LLDP_LOG, - "LLDP(%d): Updating alias name: %s -> %s\n", - __LINE__, - net->cmina_current_dcp_ase.alias_name, - new_alias); + LOG_INFO ( + PF_LLDP_LOG, + "LLDP(%d): Port %d, peer info changed\n", + __LINE__, + loc_port_num); - strncpy ( - net->cmina_current_dcp_ase.alias_name, - new_alias, - sizeof (net->cmina_current_dcp_ase.alias_name)); + LOG_INFO ( + PF_LLDP_LOG, + "LLDP(%d): Old peer info - MAC: %02X:%02X:%02X:%02X:%02X:%02X " + "Chassis ID: %s Port ID: %s\n", + __LINE__, + p_port_data->lldp.peer_info.mac_address.addr[0], + p_port_data->lldp.peer_info.mac_address.addr[1], + p_port_data->lldp.peer_info.mac_address.addr[2], + p_port_data->lldp.peer_info.mac_address.addr[3], + p_port_data->lldp.peer_info.mac_address.addr[4], + p_port_data->lldp.peer_info.mac_address.addr[5], + p_port_data->lldp.peer_info.chassis_id.string, + p_port_data->lldp.peer_info.port_id.string); + + LOG_INFO ( + PF_LLDP_LOG, + "LLDP(%d): New peer info - MAC: %02X:%02X:%02X:%02X:%02X:%02X " + "Chassis ID: %s Port ID: %s\n", + __LINE__, + lldp_peer_info->mac_address.addr[0], + lldp_peer_info->mac_address.addr[1], + lldp_peer_info->mac_address.addr[2], + lldp_peer_info->mac_address.addr[3], + lldp_peer_info->mac_address.addr[4], + lldp_peer_info->mac_address.addr[5], + lldp_peer_info->chassis_id.string, + lldp_peer_info->port_id.string); - pf_pdport_peer_indication (net, loc_port_num, lldp_peer_info); - } + pf_pdport_peer_indication (net, loc_port_num, lldp_peer_info); pf_lldp_store_peer_info (net, loc_port_num, lldp_peer_info); } diff --git a/src/common/pf_lldp.h b/src/common/pf_lldp.h index dd8d33be..7bafcfad 100644 --- a/src/common/pf_lldp.h +++ b/src/common/pf_lldp.h @@ -430,6 +430,16 @@ int pf_lldp_recv ( pnal_buf_t * p_frame_buf, uint16_t offset); +/** + * Check if an alias name matches any of the LLDP peers. + * + * @param net InOut: The p-net stack instance + * @param alias In: Alias to be compared with LLDP peers + * return true if the alias matches any of the peers, + * false if not. + */ +bool pf_lldp_is_alias_matching (pnet_t * net, const char * alias); + /************ Internal functions, made available for unit testing ************/ int pf_lldp_generate_alias_name ( diff --git a/src/device/pf_cmina.c b/src/device/pf_cmina.c index d55ca781..f92a16a3 100644 --- a/src/device/pf_cmina.c +++ b/src/device/pf_cmina.c @@ -1150,10 +1150,6 @@ int pf_cmina_dcp_get_req ( *p_block_error = PF_DCP_BLOCK_ERROR_SUBOPTION_NOT_SUPPORTED; ret = -1; break; - case PF_DCP_SUB_DEV_PROP_ALIAS: - *p_value_length = sizeof (net->cmina_current_dcp_ase.alias_name); - *pp_value = (uint8_t *)&net->cmina_current_dcp_ase.alias_name; - break; case PF_DCP_SUB_DEV_PROP_INSTANCE: *p_value_length = sizeof (net->cmina_current_dcp_ase.instance_id); *pp_value = (uint8_t *)&net->cmina_current_dcp_ase.instance_id; @@ -1167,6 +1163,7 @@ int pf_cmina_dcp_get_req ( sizeof (net->cmina_current_dcp_ase.standard_gw_value); *pp_value = (uint8_t *)&net->cmina_current_dcp_ase.standard_gw_value; break; + case PF_DCP_SUB_DEV_PROP_ALIAS: default: *p_block_error = PF_DCP_BLOCK_ERROR_SUBOPTION_NOT_SUPPORTED; ret = -1; diff --git a/src/pf_types.h b/src/pf_types.h index 7d8f87a0..b61225f2 100644 --- a/src/pf_types.h +++ b/src/pf_types.h @@ -1103,8 +1103,6 @@ typedef struct pf_cmina_dcp_ase uint8_t device_role; /* Only value "1" supported */ uint16_t device_initiative; /* 1: Should send hello. 0: No sending of hello */ - - char alias_name[PF_ALIAS_NAME_MAX_SIZE]; /** Terminated */ struct { /* Order is important!! */