From 99adb7f629cfc7680b296ad5d496d7741e03e115 Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Tue, 21 Jul 2020 16:33:35 +0000 Subject: [PATCH 1/5] Addind support for new SAI API sai_query_attribute_capability() --- lib/inc/Recorder.h | 20 +++++ lib/inc/RedisRemoteSaiInterface.h | 9 +++ lib/inc/Sai.h | 6 ++ lib/inc/SaiInterface.h | 6 ++ lib/inc/sairediscommon.h | 3 + lib/src/Recorder.cpp | 89 +++++++++++++++++++++ lib/src/RedisRemoteSaiInterface.cpp | 100 ++++++++++++++++++++++++ lib/src/Sai.cpp | 18 +++++ lib/src/sai_redis_interfacequery.cpp | 15 ++++ meta/DummySaiInterface.cpp | 11 +++ meta/DummySaiInterface.h | 6 ++ meta/Meta.cpp | 31 ++++++++ meta/Meta.h | 6 ++ syncd/Syncd.cpp | 56 +++++++++++++ syncd/Syncd.h | 3 + syncd/VendorSai.cpp | 21 +++++ syncd/VendorSai.h | 6 ++ vslib/inc/Sai.h | 6 ++ vslib/inc/VirtualSwitchSaiInterface.h | 6 ++ vslib/src/Sai.cpp | 17 ++++ vslib/src/VirtualSwitchSaiInterface.cpp | 31 ++++++++ vslib/src/sai_vs_interfacequery.cpp | 15 ++++ 22 files changed, 481 insertions(+) diff --git a/lib/inc/Recorder.h b/lib/inc/Recorder.h index 927bdc1f4..d63551a84 100644 --- a/lib/inc/Recorder.h +++ b/lib/inc/Recorder.h @@ -290,6 +290,18 @@ namespace sairedis _In_ sai_status_t status, _In_ const uint64_t *count); + void recordQueryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability); + + void recordQueryAattributeCapabilityResponse( + _In_ sai_status_t status, + _In_ sai_object_type_t objectType, + _In_ sai_attr_id_t attrId, + _In_ const sai_attr_capability_t* capability); + void recordQueryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, @@ -303,6 +315,14 @@ namespace sairedis _In_ const sai_s32_list_t* enumValuesCapability); // TODO move to private + void recordQueryAttributeCapability( + _In_ const std::string& key, + _In_ const std::vector& arguments); + + void recordQueryAttributeCapabilityResponse( + _In_ sai_status_t status, + _In_ const std::vector& arguments); + void recordQueryAttributeEnumValuesCapability( _In_ const std::string& key, _In_ const std::vector& arguments); diff --git a/lib/inc/RedisRemoteSaiInterface.h b/lib/inc/RedisRemoteSaiInterface.h index 8ed001b75..7f316d1ae 100644 --- a/lib/inc/RedisRemoteSaiInterface.h +++ b/lib/inc/RedisRemoteSaiInterface.h @@ -240,6 +240,12 @@ namespace sairedis _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; + virtual sai_status_t queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) override; + virtual sai_status_t queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, @@ -386,6 +392,9 @@ namespace sairedis private: // SAI API response + sai_status_t waitForQueryAattributeCapabilityResponse( + _Out_ sai_attr_capability_t* capability); + sai_status_t waitForQueryAattributeEnumValuesCapabilityResponse( _Inout_ sai_s32_list_t* enumValuesCapability); diff --git a/lib/inc/Sai.h b/lib/inc/Sai.h index 3cb1e9d9b..d48b59c49 100644 --- a/lib/inc/Sai.h +++ b/lib/inc/Sai.h @@ -231,6 +231,12 @@ namespace sairedis _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; + virtual sai_status_t queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) override; + virtual sai_status_t queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/lib/inc/SaiInterface.h b/lib/inc/SaiInterface.h index 48c26996a..a5f19d40b 100644 --- a/lib/inc/SaiInterface.h +++ b/lib/inc/SaiInterface.h @@ -242,6 +242,12 @@ namespace sairedis _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) = 0; + virtual sai_status_t queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) = 0; + virtual sai_status_t queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/lib/inc/sairediscommon.h b/lib/inc/sairediscommon.h index 4a4ffff74..d0de3dd26 100644 --- a/lib/inc/sairediscommon.h +++ b/lib/inc/sairediscommon.h @@ -37,6 +37,9 @@ #define REDIS_ASIC_STATE_COMMAND_FLUSH "flush" #define REDIS_ASIC_STATE_COMMAND_FLUSHRESPONSE "flushresponse" +#define REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_QUERY "attribute_capability_query" +#define REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_RESPONSE "attribute_capability_response" + #define REDIS_ASIC_STATE_COMMAND_ATTR_ENUM_VALUES_CAPABILITY_QUERY "attr_enum_values_capability_query" #define REDIS_ASIC_STATE_COMMAND_ATTR_ENUM_VALUES_CAPABILITY_RESPONSE "attr_enum_values_capability_response" diff --git a/lib/src/Recorder.cpp b/lib/src/Recorder.cpp index becc9ef4c..e2504c66c 100644 --- a/lib/src/Recorder.cpp +++ b/lib/src/Recorder.cpp @@ -265,6 +265,25 @@ void Recorder::recordFlushFdbEntriesResponse( recordLine("F|" + sai_serialize_status(status)); } +void Recorder::recordQueryAttributeCapability( + _In_ const std::string& key, + _In_ const std::vector& arguments) +{ + SWSS_LOG_ENTER(); + + recordLine("q|attribute_capability|" + key + "|" + joinFieldValues(arguments)); +} + +void Recorder::recordQueryAttributeCapabilityResponse( + _In_ sai_status_t status, + _In_ const std::vector& arguments) +{ + SWSS_LOG_ENTER(); + + recordLine("Q|attribute_capability|" + sai_serialize_status(status) + "|" + joinFieldValues(arguments)); +} + + void Recorder::recordQueryAttributeEnumValuesCapability( _In_ const std::string& key, _In_ const std::vector& arguments) @@ -886,6 +905,76 @@ void Recorder::recordObjectTypeGetAvailabilityResponse( recordObjectTypeGetAvailabilityResponse(status, values); } +void Recorder::recordQueryAattributeCapability( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t objectType, + _In_ sai_attr_id_t attrId, + _Out_ sai_attr_capability_t *Capability) +{ + SWSS_LOG_ENTER(); + + auto meta = sai_metadata_get_attr_metadata(objectType, attrId); + + if (meta == NULL) + { + SWSS_LOG_ERROR("Failed to find attribute metadata: object type %s, attr id %d", + sai_serialize_object_type(objectType).c_str(), attrId); + return; + } + + auto key = sai_serialize_object_type(SAI_OBJECT_TYPE_SWITCH) + ":" + sai_serialize_object_id(switchId); + + auto object_type_str = sai_serialize_object_type(objectType); + const std::string attr_id_str = meta->attridname; + const std::vector values = + { + swss::FieldValueTuple("OBJECT_TYPE", object_type_str), + swss::FieldValueTuple("ATTR_ID", attr_id_str) + }; + + SWSS_LOG_DEBUG("Query arguments: switch %s, object_type: %s, attribute: %s", + key.c_str(), + object_type_str.c_str(), + meta->attridname); + + recordQueryAttributeCapability(key, values); +} + + +void Recorder::recordQueryAattributeCapabilityResponse( + _In_ sai_status_t status, + _In_ sai_object_type_t objectType, + _In_ sai_attr_id_t attrId, + _In_ const sai_attr_capability_t* capability) +{ + SWSS_LOG_ENTER(); + + auto meta = sai_metadata_get_attr_metadata(objectType, attrId); + + if (meta == NULL) + { + SWSS_LOG_ERROR("Failed to find attribute metadata: object type %s, attr id %d", + sai_serialize_object_type(objectType).c_str(), attrId); + return; + } + + auto object_type_str = sai_serialize_object_type(objectType); + const std::string attr_id_str = meta->attridname; + const std::string create_str = (status == SAI_STATUS_SUCCESS ? (capability->create_implemented? "true":"false"): "false"); + const std::string set_str = (status == SAI_STATUS_SUCCESS ? (capability->set_implemented? "true":"false"): "false"); + const std::string get_str = (status == SAI_STATUS_SUCCESS ? (capability->get_implemented? "true":"false"): "false"); + const std::vector values = + { + swss::FieldValueTuple("OBJECT_TYPE", object_type_str), + swss::FieldValueTuple("ATTR_ID", attr_id_str), + swss::FieldValueTuple("CREATE_IMP", create_str), + swss::FieldValueTuple("SET_IMP", set_str), + swss::FieldValueTuple("GET_IMP", get_str) + }; + + recordQueryAttributeCapabilityResponse(status, values); +} + void Recorder::recordQueryAattributeEnumValuesCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, diff --git a/lib/src/RedisRemoteSaiInterface.cpp b/lib/src/RedisRemoteSaiInterface.cpp index 86de5e3a5..1dac7dcdb 100644 --- a/lib/src/RedisRemoteSaiInterface.cpp +++ b/lib/src/RedisRemoteSaiInterface.cpp @@ -827,6 +827,106 @@ sai_status_t RedisRemoteSaiInterface::waitForObjectTypeGetAvailabilityResponse( return status; } +sai_status_t RedisRemoteSaiInterface::queryAattributeCapability( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t objectType, + _In_ sai_attr_id_t attrId, + _Out_ sai_attr_capability_t *capability) +{ + SWSS_LOG_ENTER(); + + auto switch_id_str = sai_serialize_object_id(switchId); + auto object_type_str = sai_serialize_object_type(objectType); + + auto meta = sai_metadata_get_attr_metadata(objectType, attrId); + + if (meta == NULL) + { + SWSS_LOG_ERROR("Failed to find attribute metadata: object type %s, attr id %d", object_type_str.c_str(), attrId); + return SAI_STATUS_INVALID_PARAMETER; + } + + const std::string attr_id_str = meta->attridname; + + const std::vector entry = + { + swss::FieldValueTuple("OBJECT_TYPE", object_type_str), + swss::FieldValueTuple("ATTR_ID", attr_id_str) + }; + + SWSS_LOG_DEBUG( + "Query arguments: switch %s, object type: %s, attribute: %s", + switch_id_str.c_str(), + object_type_str.c_str(), + attr_id_str.c_str() + ); + + // This query will not put any data into the ASIC view, just into the + // message queue + + m_recorder->recordQueryAattributeCapability(switchId, objectType, attrId, capability); + + m_redisChannel->set(switch_id_str, entry, REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_QUERY); + + auto status = waitForQueryAattributeCapabilityResponse(capability); + + m_recorder->recordQueryAattributeCapabilityResponse(status, objectType, attrId, capability); + + return status; +} + +sai_status_t RedisRemoteSaiInterface::waitForQueryAattributeCapabilityResponse( + _Out_ sai_attr_capability_t* capability) +{ + SWSS_LOG_ENTER(); + + swss::KeyOpFieldsValuesTuple kco; + + auto status = m_redisChannel->wait(REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_RESPONSE, kco); + + if (status == SAI_STATUS_SUCCESS) + { + const std::vector &values = kfvFieldsValues(kco); + + if (values.size() != 3) + { + SWSS_LOG_ERROR("Invalid response from syncd: expected 3 values, received %zu", values.size()); + + return SAI_STATUS_FAILURE; + } + + const uint32_t create_implemented = std::stoi(fvValue(values[0])); + const uint32_t set_implemented = std::stoi(fvValue(values[1])); + const uint32_t get_implemented = std::stoi(fvValue(values[2])); + + SWSS_LOG_DEBUG("Received payload: create_implemented:%d, set_implemented:%d, get_implemented:%d", + create_implemented, set_implemented, get_implemented); + + if(create_implemented) + capability->create_implemented = true; + else + capability->create_implemented = false; + + if(set_implemented) + capability->set_implemented = true; + else + capability->set_implemented = false; + + if(get_implemented) + capability->get_implemented = true; + else + capability->get_implemented = false; + } + else if (status == SAI_STATUS_BUFFER_OVERFLOW) + { + // TODO on sai status overflow we should populate correct count on the list + + SWSS_LOG_ERROR("TODO need to handle SAI_STATUS_BUFFER_OVERFLOW, FIXME"); + } + + return status; +} + sai_status_t RedisRemoteSaiInterface::queryAattributeEnumValuesCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, diff --git a/lib/src/Sai.cpp b/lib/src/Sai.cpp index 483eccd41..ffedc3bed 100644 --- a/lib/src/Sai.cpp +++ b/lib/src/Sai.cpp @@ -576,6 +576,24 @@ sai_status_t Sai::objectTypeGetAvailability( count); } +sai_status_t Sai::queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) +{ + MUTEX(); + SWSS_LOG_ENTER(); + REDIS_CHECK_API_INITIALIZED(); + REDIS_CHECK_CONTEXT(switch_id); + + return context->m_meta->queryAattributeCapability( + switch_id, + object_type, + attr_id, + capability); +} + sai_status_t Sai::queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/lib/src/sai_redis_interfacequery.cpp b/lib/src/sai_redis_interfacequery.cpp index 1d8d9f4c2..c0b2c6b14 100644 --- a/lib/src/sai_redis_interfacequery.cpp +++ b/lib/src/sai_redis_interfacequery.cpp @@ -105,6 +105,21 @@ sai_status_t sai_api_query( return SAI_STATUS_INVALID_PARAMETER; } +sai_status_t sai_query_attribute_capability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) +{ + SWSS_LOG_ENTER(); + + return redis_sai->queryAattributeCapability( + switch_id, + object_type, + attr_id, + capability); +} + sai_status_t sai_query_attribute_enum_values_capability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/meta/DummySaiInterface.cpp b/meta/DummySaiInterface.cpp index da34ebe9a..4c02ca310 100644 --- a/meta/DummySaiInterface.cpp +++ b/meta/DummySaiInterface.cpp @@ -153,6 +153,17 @@ sai_status_t DummySaiInterface::objectTypeGetAvailability( return m_status; } +sai_status_t DummySaiInterface::queryAattributeCapability( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t objectType, + _In_ sai_attr_id_t attrId, + _Out_ sai_attr_capability_t *capability) +{ + SWSS_LOG_ENTER(); + + return m_status; +} + sai_status_t DummySaiInterface::queryAattributeEnumValuesCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, diff --git a/meta/DummySaiInterface.h b/meta/DummySaiInterface.h index 76007c8e1..7d60be35d 100644 --- a/meta/DummySaiInterface.h +++ b/meta/DummySaiInterface.h @@ -230,6 +230,12 @@ namespace saimeta _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; + virtual sai_status_t queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) override; + virtual sai_status_t queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/meta/Meta.cpp b/meta/Meta.cpp index 9a6c3dc64..61502c578 100644 --- a/meta/Meta.cpp +++ b/meta/Meta.cpp @@ -1856,6 +1856,37 @@ sai_status_t Meta::objectTypeGetAvailability( return status; } +sai_status_t Meta::queryAattributeCapability( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t objectType, + _In_ sai_attr_id_t attrId, + _Out_ sai_attr_capability_t *Capability) +{ + SWSS_LOG_ENTER(); + + PARAMETER_CHECK_OID_OBJECT_TYPE(switchId, SAI_OBJECT_TYPE_SWITCH); + PARAMETER_CHECK_OID_EXISTS(switchId, SAI_OBJECT_TYPE_SWITCH); + PARAMETER_CHECK_OBJECT_TYPE_VALID(objectType); + + auto mdp = sai_metadata_get_attr_metadata(objectType, attrId); + + if (!mdp) + { + SWSS_LOG_ERROR("unable to find attribute: %s:%d", + sai_serialize_object_type(objectType).c_str(), + attrId); + + return SAI_STATUS_INVALID_PARAMETER; + } + + PARAMETER_CHECK_IF_NOT_NULL(Capability); + + auto status = m_implementation->queryAattributeCapability(switchId, objectType, attrId, Capability); + + return status; +} + + sai_status_t Meta::queryAattributeEnumValuesCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, diff --git a/meta/Meta.h b/meta/Meta.h index 7860fcc8b..af11d3e18 100644 --- a/meta/Meta.h +++ b/meta/Meta.h @@ -235,6 +235,12 @@ namespace saimeta _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; + virtual sai_status_t queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) override; + virtual sai_status_t queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 598aec166..3cf9dfbc1 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -301,6 +301,9 @@ sai_status_t Syncd::processSingleEvent( if (op == REDIS_ASIC_STATE_COMMAND_FLUSH) return processFdbFlush(kco); + if (op == REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_QUERY) + return processAttrCapabilityQuery(kco); + if (op == REDIS_ASIC_STATE_COMMAND_ATTR_ENUM_VALUES_CAPABILITY_QUERY) return processAttrEnumValuesCapabilityQuery(kco); @@ -310,6 +313,59 @@ sai_status_t Syncd::processSingleEvent( SWSS_LOG_THROW("event op '%s' is not implemented, FIXME", op.c_str()); } +sai_status_t Syncd::processAttrCapabilityQuery( + _In_ const swss::KeyOpFieldsValuesTuple &kco) +{ + SWSS_LOG_ENTER(); + + auto& strSwitchVid = kfvKey(kco); + + sai_object_id_t switchVid; + sai_deserialize_object_id(strSwitchVid, switchVid); + + sai_object_id_t switchRid = m_translator->translateVidToRid(switchVid); + + auto& values = kfvFieldsValues(kco); + + if (values.size() != 2) + { + SWSS_LOG_ERROR("Invalid input: expected 2 arguments, received %zu", values.size()); + + m_getResponse->set(sai_serialize_status(SAI_STATUS_INVALID_PARAMETER), {}, REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_RESPONSE); + + return SAI_STATUS_INVALID_PARAMETER; + } + + sai_object_type_t objectType; + sai_deserialize_object_type(fvValue(values[0]), objectType); + + sai_attr_id_t attrId; + sai_deserialize_attr_id(fvValue(values[1]), attrId); + + sai_attr_capability_t capability; + + sai_status_t status = m_vendorSai->queryAattributeCapability(switchRid, objectType, attrId, &capability); + + std::vector entry; + + if (status == SAI_STATUS_SUCCESS) + { + entry = + { + swss::FieldValueTuple("CREATE_IMPLEMENTED", std::to_string(capability.create_implemented)), + swss::FieldValueTuple("SET_IMPLEMENTED", std::to_string(capability.set_implemented)), + swss::FieldValueTuple("GET_IMPLEMENTED", std::to_string(capability.get_implemented)) + }; + + SWSS_LOG_DEBUG("Sending response: create_implemented:%d, set_implemented:%d, get_implemented:%d", + capability.create_implemented, capability.set_implemented, capability.get_implemented); + } + + m_getResponse->set(sai_serialize_status(status), entry, REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_RESPONSE); + + return status; +} + sai_status_t Syncd::processAttrEnumValuesCapabilityQuery( _In_ const swss::KeyOpFieldsValuesTuple &kco) { diff --git a/syncd/Syncd.h b/syncd/Syncd.h index 92a31aee7..561f95797 100644 --- a/syncd/Syncd.h +++ b/syncd/Syncd.h @@ -120,6 +120,9 @@ namespace syncd sai_status_t processSingleEvent( _In_ const swss::KeyOpFieldsValuesTuple &kco); + sai_status_t processAttrCapabilityQuery( + _In_ const swss::KeyOpFieldsValuesTuple &kco); + sai_status_t processAttrEnumValuesCapabilityQuery( _In_ const swss::KeyOpFieldsValuesTuple &kco); diff --git a/syncd/VendorSai.cpp b/syncd/VendorSai.cpp index 0539ac4ca..0b407ad89 100644 --- a/syncd/VendorSai.cpp +++ b/syncd/VendorSai.cpp @@ -941,6 +941,27 @@ sai_status_t VendorSai::objectTypeGetAvailability( count); } +sai_status_t VendorSai::queryAattributeCapability( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t objectType, + _In_ sai_attr_id_t attrId, + _Out_ sai_attr_capability_t *capability) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + +#if 0 // Wait until BRCM fix their SAI issue then enable this + return sai_query_attribute_capability( + switchId, + objectType, + attrId, + capability); +#else + return SAI_STATUS_NOT_IMPLEMENTED; +#endif +} + sai_status_t VendorSai::queryAattributeEnumValuesCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, diff --git a/syncd/VendorSai.h b/syncd/VendorSai.h index 2f79aaf5d..f571f2a74 100644 --- a/syncd/VendorSai.h +++ b/syncd/VendorSai.h @@ -227,6 +227,12 @@ namespace syncd _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; + virtual sai_status_t queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) override; + virtual sai_status_t queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/vslib/inc/Sai.h b/vslib/inc/Sai.h index 7412bd928..8880ba8a0 100644 --- a/vslib/inc/Sai.h +++ b/vslib/inc/Sai.h @@ -232,6 +232,12 @@ namespace saivs _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; + virtual sai_status_t queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) override; + virtual sai_status_t queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/vslib/inc/VirtualSwitchSaiInterface.h b/vslib/inc/VirtualSwitchSaiInterface.h index 0891585bd..532f1dcfd 100644 --- a/vslib/inc/VirtualSwitchSaiInterface.h +++ b/vslib/inc/VirtualSwitchSaiInterface.h @@ -234,6 +234,12 @@ namespace saivs _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; + virtual sai_status_t queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) override; + virtual sai_status_t queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/vslib/src/Sai.cpp b/vslib/src/Sai.cpp index d2a0b1287..0a61dd22c 100644 --- a/vslib/src/Sai.cpp +++ b/vslib/src/Sai.cpp @@ -706,6 +706,23 @@ sai_status_t Sai::objectTypeGetAvailability( count); } +sai_status_t Sai::queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VS_CHECK_API_INITIALIZED(); + + return m_meta->queryAattributeCapability( + switch_id, + object_type, + attr_id, + capability); +} + sai_status_t Sai::queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/vslib/src/VirtualSwitchSaiInterface.cpp b/vslib/src/VirtualSwitchSaiInterface.cpp index e35a90547..0e052ef21 100644 --- a/vslib/src/VirtualSwitchSaiInterface.cpp +++ b/vslib/src/VirtualSwitchSaiInterface.cpp @@ -789,6 +789,37 @@ sai_status_t VirtualSwitchSaiInterface::objectTypeGetAvailability( return SAI_STATUS_NOT_SUPPORTED; } +sai_status_t VirtualSwitchSaiInterface::queryAattributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) +{ + SWSS_LOG_ENTER(); + + // TODO: We should generate this metadata for the virtual switch rather + // than hard-coding it here. + + if (object_type == SAI_OBJECT_TYPE_PORT && attr_id == SAI_PORT_ATTR_TPID) + { + capability->create_implemented = true; + capability->set_implemented = true; + capability->get_implemented = true; + + return SAI_STATUS_SUCCESS; + } + else if (object_type == SAI_OBJECT_TYPE_LAG && attr_id == SAI_LAG_ATTR_TPID) + { + capability->create_implemented = true; + capability->set_implemented = true; + capability->get_implemented = true; + + return SAI_STATUS_SUCCESS; + } + + return SAI_STATUS_NOT_SUPPORTED; +} + sai_status_t VirtualSwitchSaiInterface::queryAattributeEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/vslib/src/sai_vs_interfacequery.cpp b/vslib/src/sai_vs_interfacequery.cpp index dddddcd8a..322960f6e 100644 --- a/vslib/src/sai_vs_interfacequery.cpp +++ b/vslib/src/sai_vs_interfacequery.cpp @@ -104,6 +104,21 @@ sai_status_t sai_api_query( return SAI_STATUS_INVALID_PARAMETER; } +sai_status_t sai_query_attribute_capability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) +{ + SWSS_LOG_ENTER(); + + return vs_sai->queryAattributeCapability( + switch_id, + object_type, + attr_id, + capability); +} + sai_status_t sai_query_attribute_enum_values_capability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, From dc868975a893d25e5ac554ca3ecb1fe010685e2a Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Tue, 21 Jul 2020 19:41:50 +0000 Subject: [PATCH 2/5] Changes based on review comments for SAI API sai_query_attribute_capability() PR --- lib/inc/RedisRemoteSaiInterface.h | 4 ++-- lib/inc/Sai.h | 2 +- lib/inc/SaiInterface.h | 2 +- lib/src/RedisRemoteSaiInterface.cpp | 32 ++++++++++--------------- lib/src/Sai.cpp | 4 ++-- lib/src/sai_redis_interfacequery.cpp | 2 +- meta/DummySaiInterface.cpp | 2 +- meta/DummySaiInterface.h | 2 +- meta/Meta.cpp | 8 +++---- meta/Meta.h | 2 +- syncd/Syncd.cpp | 4 ++-- syncd/VendorSai.cpp | 2 +- syncd/VendorSai.h | 2 +- vslib/inc/Sai.h | 2 +- vslib/inc/VirtualSwitchSaiInterface.h | 2 +- vslib/src/Sai.cpp | 16 ++++++------- vslib/src/VirtualSwitchSaiInterface.cpp | 23 +++++------------- vslib/src/sai_vs_interfacequery.cpp | 2 +- 18 files changed, 48 insertions(+), 65 deletions(-) diff --git a/lib/inc/RedisRemoteSaiInterface.h b/lib/inc/RedisRemoteSaiInterface.h index 7f316d1ae..12d624ae2 100644 --- a/lib/inc/RedisRemoteSaiInterface.h +++ b/lib/inc/RedisRemoteSaiInterface.h @@ -240,7 +240,7 @@ namespace sairedis _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; - virtual sai_status_t queryAattributeCapability( + virtual sai_status_t queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, @@ -392,7 +392,7 @@ namespace sairedis private: // SAI API response - sai_status_t waitForQueryAattributeCapabilityResponse( + sai_status_t waitForQueryAttributeCapabilityResponse( _Out_ sai_attr_capability_t* capability); sai_status_t waitForQueryAattributeEnumValuesCapabilityResponse( diff --git a/lib/inc/Sai.h b/lib/inc/Sai.h index d48b59c49..1ea13bac0 100644 --- a/lib/inc/Sai.h +++ b/lib/inc/Sai.h @@ -231,7 +231,7 @@ namespace sairedis _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; - virtual sai_status_t queryAattributeCapability( + virtual sai_status_t queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, diff --git a/lib/inc/SaiInterface.h b/lib/inc/SaiInterface.h index a5f19d40b..5ce7f727f 100644 --- a/lib/inc/SaiInterface.h +++ b/lib/inc/SaiInterface.h @@ -242,7 +242,7 @@ namespace sairedis _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) = 0; - virtual sai_status_t queryAattributeCapability( + virtual sai_status_t queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, diff --git a/lib/src/RedisRemoteSaiInterface.cpp b/lib/src/RedisRemoteSaiInterface.cpp index 1dac7dcdb..4fda15190 100644 --- a/lib/src/RedisRemoteSaiInterface.cpp +++ b/lib/src/RedisRemoteSaiInterface.cpp @@ -827,7 +827,7 @@ sai_status_t RedisRemoteSaiInterface::waitForObjectTypeGetAvailabilityResponse( return status; } -sai_status_t RedisRemoteSaiInterface::queryAattributeCapability( +sai_status_t RedisRemoteSaiInterface::queryAttributeCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, _In_ sai_attr_id_t attrId, @@ -835,30 +835,30 @@ sai_status_t RedisRemoteSaiInterface::queryAattributeCapability( { SWSS_LOG_ENTER(); - auto switch_id_str = sai_serialize_object_id(switchId); - auto object_type_str = sai_serialize_object_type(objectType); + auto switchIdStr = sai_serialize_object_id(switchId); + auto objectTypeStr = sai_serialize_object_type(objectType); auto meta = sai_metadata_get_attr_metadata(objectType, attrId); if (meta == NULL) { - SWSS_LOG_ERROR("Failed to find attribute metadata: object type %s, attr id %d", object_type_str.c_str(), attrId); + SWSS_LOG_ERROR("Failed to find attribute metadata: object type %s, attr id %d", objectTypeStr.c_str(), attrId); return SAI_STATUS_INVALID_PARAMETER; } - const std::string attr_id_str = meta->attridname; + const std::string attrIdStr = meta->attridname; const std::vector entry = { - swss::FieldValueTuple("OBJECT_TYPE", object_type_str), - swss::FieldValueTuple("ATTR_ID", attr_id_str) + swss::FieldValueTuple("OBJECT_TYPE", objectTypeStr), + swss::FieldValueTuple("ATTR_ID", attrIdStr) }; SWSS_LOG_DEBUG( "Query arguments: switch %s, object type: %s, attribute: %s", - switch_id_str.c_str(), - object_type_str.c_str(), - attr_id_str.c_str() + switchIdStr.c_str(), + objectTypeStr.c_str(), + attrIdStr.c_str() ); // This query will not put any data into the ASIC view, just into the @@ -866,16 +866,16 @@ sai_status_t RedisRemoteSaiInterface::queryAattributeCapability( m_recorder->recordQueryAattributeCapability(switchId, objectType, attrId, capability); - m_redisChannel->set(switch_id_str, entry, REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_QUERY); + m_redisChannel->set(switchIdStr, entry, REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_QUERY); - auto status = waitForQueryAattributeCapabilityResponse(capability); + auto status = waitForQueryAttributeCapabilityResponse(capability); m_recorder->recordQueryAattributeCapabilityResponse(status, objectType, attrId, capability); return status; } -sai_status_t RedisRemoteSaiInterface::waitForQueryAattributeCapabilityResponse( +sai_status_t RedisRemoteSaiInterface::waitForQueryAttributeCapabilityResponse( _Out_ sai_attr_capability_t* capability) { SWSS_LOG_ENTER(); @@ -917,12 +917,6 @@ sai_status_t RedisRemoteSaiInterface::waitForQueryAattributeCapabilityResponse( else capability->get_implemented = false; } - else if (status == SAI_STATUS_BUFFER_OVERFLOW) - { - // TODO on sai status overflow we should populate correct count on the list - - SWSS_LOG_ERROR("TODO need to handle SAI_STATUS_BUFFER_OVERFLOW, FIXME"); - } return status; } diff --git a/lib/src/Sai.cpp b/lib/src/Sai.cpp index ffedc3bed..83f46f101 100644 --- a/lib/src/Sai.cpp +++ b/lib/src/Sai.cpp @@ -576,7 +576,7 @@ sai_status_t Sai::objectTypeGetAvailability( count); } -sai_status_t Sai::queryAattributeCapability( +sai_status_t Sai::queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, @@ -587,7 +587,7 @@ sai_status_t Sai::queryAattributeCapability( REDIS_CHECK_API_INITIALIZED(); REDIS_CHECK_CONTEXT(switch_id); - return context->m_meta->queryAattributeCapability( + return context->m_meta->queryAttributeCapability( switch_id, object_type, attr_id, diff --git a/lib/src/sai_redis_interfacequery.cpp b/lib/src/sai_redis_interfacequery.cpp index c0b2c6b14..b90161cee 100644 --- a/lib/src/sai_redis_interfacequery.cpp +++ b/lib/src/sai_redis_interfacequery.cpp @@ -113,7 +113,7 @@ sai_status_t sai_query_attribute_capability( { SWSS_LOG_ENTER(); - return redis_sai->queryAattributeCapability( + return redis_sai->queryAttributeCapability( switch_id, object_type, attr_id, diff --git a/meta/DummySaiInterface.cpp b/meta/DummySaiInterface.cpp index 4c02ca310..3f32f9229 100644 --- a/meta/DummySaiInterface.cpp +++ b/meta/DummySaiInterface.cpp @@ -153,7 +153,7 @@ sai_status_t DummySaiInterface::objectTypeGetAvailability( return m_status; } -sai_status_t DummySaiInterface::queryAattributeCapability( +sai_status_t DummySaiInterface::queryAttributeCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, _In_ sai_attr_id_t attrId, diff --git a/meta/DummySaiInterface.h b/meta/DummySaiInterface.h index 7d60be35d..b0d801233 100644 --- a/meta/DummySaiInterface.h +++ b/meta/DummySaiInterface.h @@ -230,7 +230,7 @@ namespace saimeta _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; - virtual sai_status_t queryAattributeCapability( + virtual sai_status_t queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, diff --git a/meta/Meta.cpp b/meta/Meta.cpp index 61502c578..cecec4e86 100644 --- a/meta/Meta.cpp +++ b/meta/Meta.cpp @@ -1856,11 +1856,11 @@ sai_status_t Meta::objectTypeGetAvailability( return status; } -sai_status_t Meta::queryAattributeCapability( +sai_status_t Meta::queryAttributeCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, _In_ sai_attr_id_t attrId, - _Out_ sai_attr_capability_t *Capability) + _Out_ sai_attr_capability_t *capability) { SWSS_LOG_ENTER(); @@ -1879,9 +1879,9 @@ sai_status_t Meta::queryAattributeCapability( return SAI_STATUS_INVALID_PARAMETER; } - PARAMETER_CHECK_IF_NOT_NULL(Capability); + PARAMETER_CHECK_IF_NOT_NULL(capability); - auto status = m_implementation->queryAattributeCapability(switchId, objectType, attrId, Capability); + auto status = m_implementation->queryAttributeCapability(switchId, objectType, attrId, capability); return status; } diff --git a/meta/Meta.h b/meta/Meta.h index af11d3e18..498ce11ea 100644 --- a/meta/Meta.h +++ b/meta/Meta.h @@ -235,7 +235,7 @@ namespace saimeta _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; - virtual sai_status_t queryAattributeCapability( + virtual sai_status_t queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 3cf9dfbc1..f1218f1f0 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -344,7 +344,7 @@ sai_status_t Syncd::processAttrCapabilityQuery( sai_attr_capability_t capability; - sai_status_t status = m_vendorSai->queryAattributeCapability(switchRid, objectType, attrId, &capability); + sai_status_t status = m_vendorSai->queryAttributeCapability(switchRid, objectType, attrId, &capability); std::vector entry; @@ -357,7 +357,7 @@ sai_status_t Syncd::processAttrCapabilityQuery( swss::FieldValueTuple("GET_IMPLEMENTED", std::to_string(capability.get_implemented)) }; - SWSS_LOG_DEBUG("Sending response: create_implemented:%d, set_implemented:%d, get_implemented:%d", + SWSS_LOG_INFO("Sending response: create_implemented:%d, set_implemented:%d, get_implemented:%d", capability.create_implemented, capability.set_implemented, capability.get_implemented); } diff --git a/syncd/VendorSai.cpp b/syncd/VendorSai.cpp index 0b407ad89..aec6765d6 100644 --- a/syncd/VendorSai.cpp +++ b/syncd/VendorSai.cpp @@ -941,7 +941,7 @@ sai_status_t VendorSai::objectTypeGetAvailability( count); } -sai_status_t VendorSai::queryAattributeCapability( +sai_status_t VendorSai::queryAttributeCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, _In_ sai_attr_id_t attrId, diff --git a/syncd/VendorSai.h b/syncd/VendorSai.h index f571f2a74..0c91eaf83 100644 --- a/syncd/VendorSai.h +++ b/syncd/VendorSai.h @@ -227,7 +227,7 @@ namespace syncd _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; - virtual sai_status_t queryAattributeCapability( + virtual sai_status_t queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, diff --git a/vslib/inc/Sai.h b/vslib/inc/Sai.h index 8880ba8a0..7a6ec637b 100644 --- a/vslib/inc/Sai.h +++ b/vslib/inc/Sai.h @@ -232,7 +232,7 @@ namespace saivs _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; - virtual sai_status_t queryAattributeCapability( + virtual sai_status_t queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, diff --git a/vslib/inc/VirtualSwitchSaiInterface.h b/vslib/inc/VirtualSwitchSaiInterface.h index 532f1dcfd..0e41302a6 100644 --- a/vslib/inc/VirtualSwitchSaiInterface.h +++ b/vslib/inc/VirtualSwitchSaiInterface.h @@ -234,7 +234,7 @@ namespace saivs _In_ const sai_attribute_t *attrList, _Out_ uint64_t *count) override; - virtual sai_status_t queryAattributeCapability( + virtual sai_status_t queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, diff --git a/vslib/src/Sai.cpp b/vslib/src/Sai.cpp index 0a61dd22c..1699fd1c8 100644 --- a/vslib/src/Sai.cpp +++ b/vslib/src/Sai.cpp @@ -706,20 +706,20 @@ sai_status_t Sai::objectTypeGetAvailability( count); } -sai_status_t Sai::queryAattributeCapability( - _In_ sai_object_id_t switch_id, - _In_ sai_object_type_t object_type, - _In_ sai_attr_id_t attr_id, +sai_status_t Sai::queryAttributeCapability( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t objectType, + _In_ sai_attr_id_t attrId, _Out_ sai_attr_capability_t *capability) { MUTEX(); SWSS_LOG_ENTER(); VS_CHECK_API_INITIALIZED(); - return m_meta->queryAattributeCapability( - switch_id, - object_type, - attr_id, + return m_meta->queryAttributeCapability( + switchId, + objectType, + attrId, capability); } diff --git a/vslib/src/VirtualSwitchSaiInterface.cpp b/vslib/src/VirtualSwitchSaiInterface.cpp index 0e052ef21..225c0e207 100644 --- a/vslib/src/VirtualSwitchSaiInterface.cpp +++ b/vslib/src/VirtualSwitchSaiInterface.cpp @@ -789,7 +789,7 @@ sai_status_t VirtualSwitchSaiInterface::objectTypeGetAvailability( return SAI_STATUS_NOT_SUPPORTED; } -sai_status_t VirtualSwitchSaiInterface::queryAattributeCapability( +sai_status_t VirtualSwitchSaiInterface::queryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, @@ -800,24 +800,13 @@ sai_status_t VirtualSwitchSaiInterface::queryAattributeCapability( // TODO: We should generate this metadata for the virtual switch rather // than hard-coding it here. - if (object_type == SAI_OBJECT_TYPE_PORT && attr_id == SAI_PORT_ATTR_TPID) - { - capability->create_implemented = true; - capability->set_implemented = true; - capability->get_implemented = true; + // in virtual switch by default all apis are implemented for all objects. SUCCESS for all attributes - return SAI_STATUS_SUCCESS; - } - else if (object_type == SAI_OBJECT_TYPE_LAG && attr_id == SAI_LAG_ATTR_TPID) - { - capability->create_implemented = true; - capability->set_implemented = true; - capability->get_implemented = true; + capability->create_implemented = true; + capability->set_implemented = true; + capability->get_implemented = true; - return SAI_STATUS_SUCCESS; - } - - return SAI_STATUS_NOT_SUPPORTED; + return SAI_STATUS_SUCCESS; } sai_status_t VirtualSwitchSaiInterface::queryAattributeEnumValuesCapability( diff --git a/vslib/src/sai_vs_interfacequery.cpp b/vslib/src/sai_vs_interfacequery.cpp index 322960f6e..0ea644e3f 100644 --- a/vslib/src/sai_vs_interfacequery.cpp +++ b/vslib/src/sai_vs_interfacequery.cpp @@ -112,7 +112,7 @@ sai_status_t sai_query_attribute_capability( { SWSS_LOG_ENTER(); - return vs_sai->queryAattributeCapability( + return vs_sai->queryAttributeCapability( switch_id, object_type, attr_id, From 139cf60e48c0f41a845526c95aca8c4e97c9bd84 Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Tue, 21 Jul 2020 20:21:08 +0000 Subject: [PATCH 3/5] Changes based on review comments for SAI API sai_query_attribute_capability() PR --- lib/inc/Recorder.h | 2 +- lib/src/Recorder.cpp | 2 +- lib/src/RedisRemoteSaiInterface.cpp | 15 +++++++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/inc/Recorder.h b/lib/inc/Recorder.h index d63551a84..773157d19 100644 --- a/lib/inc/Recorder.h +++ b/lib/inc/Recorder.h @@ -296,7 +296,7 @@ namespace sairedis _In_ sai_attr_id_t attr_id, _Out_ sai_attr_capability_t *capability); - void recordQueryAattributeCapabilityResponse( + void recordQueryAttributeCapabilityResponse( _In_ sai_status_t status, _In_ sai_object_type_t objectType, _In_ sai_attr_id_t attrId, diff --git a/lib/src/Recorder.cpp b/lib/src/Recorder.cpp index e2504c66c..ea9a42fb6 100644 --- a/lib/src/Recorder.cpp +++ b/lib/src/Recorder.cpp @@ -941,7 +941,7 @@ void Recorder::recordQueryAattributeCapability( } -void Recorder::recordQueryAattributeCapabilityResponse( +void Recorder::recordQueryAttributeCapabilityResponse( _In_ sai_status_t status, _In_ sai_object_type_t objectType, _In_ sai_attr_id_t attrId, diff --git a/lib/src/RedisRemoteSaiInterface.cpp b/lib/src/RedisRemoteSaiInterface.cpp index 4fda15190..13fee53ea 100644 --- a/lib/src/RedisRemoteSaiInterface.cpp +++ b/lib/src/RedisRemoteSaiInterface.cpp @@ -870,7 +870,7 @@ sai_status_t RedisRemoteSaiInterface::queryAttributeCapability( auto status = waitForQueryAttributeCapabilityResponse(capability); - m_recorder->recordQueryAattributeCapabilityResponse(status, objectType, attrId, capability); + m_recorder->recordQueryAttributeCapabilityResponse(status, objectType, attrId, capability); return status; } @@ -903,19 +903,30 @@ sai_status_t RedisRemoteSaiInterface::waitForQueryAttributeCapabilityResponse( create_implemented, set_implemented, get_implemented); if(create_implemented) + { capability->create_implemented = true; + } else + { capability->create_implemented = false; - + } if(set_implemented) + { capability->set_implemented = true; + } else + { capability->set_implemented = false; + } if(get_implemented) + { capability->get_implemented = true; + } else + { capability->get_implemented = false; + } } return status; From 05f32a231748928fb75d01391d543badb901263b Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Tue, 21 Jul 2020 20:28:57 +0000 Subject: [PATCH 4/5] Changes based on review comments for SAI API sai_query_attribute_capability() PR --- lib/inc/Recorder.h | 2 +- lib/src/Recorder.cpp | 2 +- lib/src/RedisRemoteSaiInterface.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/inc/Recorder.h b/lib/inc/Recorder.h index 773157d19..b747589ce 100644 --- a/lib/inc/Recorder.h +++ b/lib/inc/Recorder.h @@ -290,7 +290,7 @@ namespace sairedis _In_ sai_status_t status, _In_ const uint64_t *count); - void recordQueryAattributeCapability( + void recordQueryAttributeCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, _In_ sai_attr_id_t attr_id, diff --git a/lib/src/Recorder.cpp b/lib/src/Recorder.cpp index ea9a42fb6..448bd108f 100644 --- a/lib/src/Recorder.cpp +++ b/lib/src/Recorder.cpp @@ -905,7 +905,7 @@ void Recorder::recordObjectTypeGetAvailabilityResponse( recordObjectTypeGetAvailabilityResponse(status, values); } -void Recorder::recordQueryAattributeCapability( +void Recorder::recordQueryAttributeCapability( _In_ sai_object_id_t switchId, _In_ sai_object_type_t objectType, _In_ sai_attr_id_t attrId, diff --git a/lib/src/RedisRemoteSaiInterface.cpp b/lib/src/RedisRemoteSaiInterface.cpp index 13fee53ea..ebbbc7b41 100644 --- a/lib/src/RedisRemoteSaiInterface.cpp +++ b/lib/src/RedisRemoteSaiInterface.cpp @@ -864,7 +864,7 @@ sai_status_t RedisRemoteSaiInterface::queryAttributeCapability( // This query will not put any data into the ASIC view, just into the // message queue - m_recorder->recordQueryAattributeCapability(switchId, objectType, attrId, capability); + m_recorder->recordQueryAttributeCapability(switchId, objectType, attrId, capability); m_redisChannel->set(switchIdStr, entry, REDIS_ASIC_STATE_COMMAND_ATTR_CAPABILITY_QUERY); From 6a3150d4102ce62da10477528fddf4bcd245dfa5 Mon Sep 17 00:00:00 2001 From: Gen-Hwa Chiang Date: Wed, 22 Jul 2020 15:40:19 +0000 Subject: [PATCH 5/5] Changed to return capability in true/false string instead of number strings --- lib/src/RedisRemoteSaiInterface.cpp | 36 ++++------------------------- syncd/Syncd.cpp | 6 ++--- 2 files changed, 8 insertions(+), 34 deletions(-) diff --git a/lib/src/RedisRemoteSaiInterface.cpp b/lib/src/RedisRemoteSaiInterface.cpp index ebbbc7b41..ec045b54f 100644 --- a/lib/src/RedisRemoteSaiInterface.cpp +++ b/lib/src/RedisRemoteSaiInterface.cpp @@ -895,38 +895,12 @@ sai_status_t RedisRemoteSaiInterface::waitForQueryAttributeCapabilityResponse( return SAI_STATUS_FAILURE; } - const uint32_t create_implemented = std::stoi(fvValue(values[0])); - const uint32_t set_implemented = std::stoi(fvValue(values[1])); - const uint32_t get_implemented = std::stoi(fvValue(values[2])); + capability->create_implemented = (fvValue(values[0]) == "true" ? true : false); + capability->set_implemented = (fvValue(values[1]) == "true" ? true : false); + capability->get_implemented = (fvValue(values[2]) == "true" ? true : false); - SWSS_LOG_DEBUG("Received payload: create_implemented:%d, set_implemented:%d, get_implemented:%d", - create_implemented, set_implemented, get_implemented); - - if(create_implemented) - { - capability->create_implemented = true; - } - else - { - capability->create_implemented = false; - } - if(set_implemented) - { - capability->set_implemented = true; - } - else - { - capability->set_implemented = false; - } - - if(get_implemented) - { - capability->get_implemented = true; - } - else - { - capability->get_implemented = false; - } + SWSS_LOG_DEBUG("Received payload: create_implemented:%s, set_implemented:%s, get_implemented:%s", + (capability->create_implemented? "true":"false"), (capability->set_implemented? "true":"false"), (capability->get_implemented? "true":"false")); } return status; diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index f1218f1f0..b33a9af29 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -352,9 +352,9 @@ sai_status_t Syncd::processAttrCapabilityQuery( { entry = { - swss::FieldValueTuple("CREATE_IMPLEMENTED", std::to_string(capability.create_implemented)), - swss::FieldValueTuple("SET_IMPLEMENTED", std::to_string(capability.set_implemented)), - swss::FieldValueTuple("GET_IMPLEMENTED", std::to_string(capability.get_implemented)) + swss::FieldValueTuple("CREATE_IMPLEMENTED", (capability.create_implemented ? "true" : "false")), + swss::FieldValueTuple("SET_IMPLEMENTED", (capability.set_implemented ? "true" : "false")), + swss::FieldValueTuple("GET_IMPLEMENTED", (capability.get_implemented ? "true" : "false")) }; SWSS_LOG_INFO("Sending response: create_implemented:%d, set_implemented:%d, get_implemented:%d",