From 54cda1e58086bb5bb75b9d0c54a2741bbb7fc100 Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Tue, 5 Apr 2022 02:17:02 +0000 Subject: [PATCH 1/5] [BFD]Retry create BFD with different source UDP port on failure The create_bfd_session API can fail when a source port already used by a different application is passed to SAI create. To avoid failure, a retry logic is added to try with a different port number three times before failing --- orchagent/bfdorch.cpp | 35 ++++++++++++++++++++++++++++++++++- orchagent/bfdorch.h | 1 + 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/orchagent/bfdorch.cpp b/orchagent/bfdorch.cpp index 7bc5f4bfd3..3a4e792891 100644 --- a/orchagent/bfdorch.cpp +++ b/orchagent/bfdorch.cpp @@ -17,6 +17,7 @@ using namespace swss; #define BFD_SESSION_MILLISECOND_TO_MICROSECOND 1000 #define BFD_SRCPORTINIT 49152 #define BFD_SRCPORTMAX 65536 +#define NUM_BFD_SRCPORT_RETRIES 3 extern sai_bfd_api_t* sai_bfd_api; extern sai_object_id_t gSwitchId; @@ -239,6 +240,8 @@ bool BfdOrch::create_bfd_session(const string& key, const vector attrs; vector fvVector; + uint32_t attr_idx = 0; + uint32_t src_port_attr_idx = 0; for (auto i : data) { @@ -292,50 +295,62 @@ bool BfdOrch::create_bfd_session(const string& key, const vectorcreate_bfd_session(&bfd_session_id, gSwitchId, (uint32_t)attrs.size(), attrs.data()); + sai_status_t status = sai_bfd_api->create_bfd_session(&bfd_session_id, gSwitchId, attr_idx, attrs.data()); + + for (int retry = 0; (status != SAI_STATUS_SUCCESS) && (retry < NUM_BFD_SRCPORT_RETRIES); retry++) + { + auto old_num = attrs[src_port_attr_idx].value.u32; + attrs[src_port_attr_idx].value.u32 = bfd_src_port(); + SWSS_LOG_WARN("BFD create using port number %d failed. Retrying with port number %d", + old_num, attrs[src_port_attr_idx].value.u32); + status = sai_bfd_api->create_bfd_session(&bfd_session_id, gSwitchId, + attr_idx, attrs.data()); + } + if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to create bfd session %s, rv:%d", key.c_str(), status); @@ -487,3 +519,4 @@ uint32_t BfdOrch::bfd_src_port(void) return (port++); } + diff --git a/orchagent/bfdorch.h b/orchagent/bfdorch.h index 019af037b8..7774839c19 100644 --- a/orchagent/bfdorch.h +++ b/orchagent/bfdorch.h @@ -25,6 +25,7 @@ class BfdOrch: public Orch, public Subject uint32_t bfd_gen_id(void); uint32_t bfd_src_port(void); + void update_port_number(std::vector &attrs); bool register_bfd_state_change_notification(void); From 31ca1794652089ff015bcae8772d9fdb9e527115 Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Wed, 6 Apr 2022 22:35:09 +0000 Subject: [PATCH 2/5] Addressing code review comments --- orchagent/bfdorch.cpp | 64 +++++++++++++++++++++++-------------------- orchagent/bfdorch.h | 2 ++ 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/orchagent/bfdorch.cpp b/orchagent/bfdorch.cpp index 3a4e792891..f0848a78c5 100644 --- a/orchagent/bfdorch.cpp +++ b/orchagent/bfdorch.cpp @@ -240,8 +240,6 @@ bool BfdOrch::create_bfd_session(const string& key, const vector attrs; vector fvVector; - uint32_t attr_idx = 0; - uint32_t src_port_attr_idx = 0; for (auto i : data) { @@ -295,62 +293,50 @@ bool BfdOrch::create_bfd_session(const string& key, const vectorcreate_bfd_session(&bfd_session_id, gSwitchId, attr_idx, attrs.data()); + sai_status_t status = sai_bfd_api->create_bfd_session(&bfd_session_id, gSwitchId, (uint32_t)attrs.size(), attrs.data()); - for (int retry = 0; (status != SAI_STATUS_SUCCESS) && (retry < NUM_BFD_SRCPORT_RETRIES); retry++) + if (status != SAI_STATUS_SUCCESS) { - auto old_num = attrs[src_port_attr_idx].value.u32; - attrs[src_port_attr_idx].value.u32 = bfd_src_port(); - SWSS_LOG_WARN("BFD create using port number %d failed. Retrying with port number %d", - old_num, attrs[src_port_attr_idx].value.u32); - status = sai_bfd_api->create_bfd_session(&bfd_session_id, gSwitchId, - attr_idx, attrs.data()); + status = retry_create_bfd_session(bfd_session_id, attrs); } - if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to create bfd session %s, rv:%d", key.c_str(), status); @@ -471,6 +445,38 @@ bool BfdOrch::create_bfd_session(const string& key, const vector &attrs) +{ + for (uint32_t attr_idx = 0; attr_idx < (uint32_t)attrs.size(); attr_idx++) + { + if (attrs[attr_idx].id == SAI_BFD_SESSION_ATTR_UDP_SRC_PORT) + { + auto old_num = attrs[attr_idx].value.u32; + attrs[attr_idx].value.u32 = bfd_src_port(); + SWSS_LOG_INFO ("BFD create using port number %d failed. Retrying with port number %d", + old_num, attrs[attr_idx].value.u32); + return; + } + } +} + +sai_status_t BfdOrch::retry_create_bfd_session(sai_object_id_t &bfd_session_id, vector attrs) +{ + sai_status_t status = SAI_STATUS_FAILURE; + + for (int retry = 0; retry < NUM_BFD_SRCPORT_RETRIES; retry++) + { + update_port_number(attrs); + status = sai_bfd_api->create_bfd_session(&bfd_session_id, gSwitchId, + (uint32_t)attrs.size(), attrs.data()); + if(status == SAI_STATUS_SUCCESS) + { + return status; + } + } + return status; +} + bool BfdOrch::remove_bfd_session(const string& key) { if (bfd_session_map.find(key) == bfd_session_map.end()) diff --git a/orchagent/bfdorch.h b/orchagent/bfdorch.h index 7774839c19..4b86165c5b 100644 --- a/orchagent/bfdorch.h +++ b/orchagent/bfdorch.h @@ -28,6 +28,8 @@ class BfdOrch: public Orch, public Subject void update_port_number(std::vector &attrs); bool register_bfd_state_change_notification(void); + void update_port_number(std::vector &attrs); + sai_status_t retry_create_bfd_session(sai_object_id_t &bfd_session_id, vector attrs); std::map bfd_session_map; std::map bfd_session_lookup; From 0741287f873b36e0a21b9e35e8776fc5e47b1be8 Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Wed, 6 Apr 2022 22:53:46 +0000 Subject: [PATCH 3/5] Changing log level to warn --- orchagent/bfdorch.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/orchagent/bfdorch.cpp b/orchagent/bfdorch.cpp index f0848a78c5..2580e7fb2c 100644 --- a/orchagent/bfdorch.cpp +++ b/orchagent/bfdorch.cpp @@ -422,6 +422,7 @@ bool BfdOrch::create_bfd_session(const string& key, const vector &attrs) { auto old_num = attrs[attr_idx].value.u32; attrs[attr_idx].value.u32 = bfd_src_port(); - SWSS_LOG_INFO ("BFD create using port number %d failed. Retrying with port number %d", - old_num, attrs[attr_idx].value.u32); + SWSS_LOG_WARN("BFD create using port number %d failed. Retrying with port number %d", + old_num, attrs[attr_idx].value.u32); return; } } From a0b80e4b4d526d7e5b9796825af8eeb3827358d8 Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Wed, 6 Apr 2022 23:10:18 +0000 Subject: [PATCH 4/5] Fixing build issue --- orchagent/bfdorch.h | 1 - 1 file changed, 1 deletion(-) diff --git a/orchagent/bfdorch.h b/orchagent/bfdorch.h index 4b86165c5b..4a0cb9edfb 100644 --- a/orchagent/bfdorch.h +++ b/orchagent/bfdorch.h @@ -25,7 +25,6 @@ class BfdOrch: public Orch, public Subject uint32_t bfd_gen_id(void); uint32_t bfd_src_port(void); - void update_port_number(std::vector &attrs); bool register_bfd_state_change_notification(void); void update_port_number(std::vector &attrs); From f29c5db284bc8fee337b8ebccf4c21f89cec029d Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Thu, 7 Apr 2022 02:45:51 +0000 Subject: [PATCH 5/5] Addressing code review comments --- orchagent/bfdorch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/bfdorch.cpp b/orchagent/bfdorch.cpp index 2580e7fb2c..f40782ba2b 100644 --- a/orchagent/bfdorch.cpp +++ b/orchagent/bfdorch.cpp @@ -470,7 +470,7 @@ sai_status_t BfdOrch::retry_create_bfd_session(sai_object_id_t &bfd_session_id, update_port_number(attrs); status = sai_bfd_api->create_bfd_session(&bfd_session_id, gSwitchId, (uint32_t)attrs.size(), attrs.data()); - if(status == SAI_STATUS_SUCCESS) + if (status == SAI_STATUS_SUCCESS) { return status; }