Bfd support for TSA state. #2926

siqbal1986 · 2023-10-12T01:36:35Z

These changes enable us to bring certain BFD sessions down when the device goes into TSA. when the device switches to TSB, these sessions are brought up.
What I did
Added a session cache in BFD orch which records sessions with the "tsa_shutdown" parameter and removes/adds these sessions when switching between TSA/TSB.
Why I did it
In TSA a device is not supposed to be forwarding data plane traffic but in the existing implementation, BFD sessions were kept up for Vxlan tunnels with BFD monitoring. This gives the Vxlan tunnel traffic receiver the impression that the device is active but not sending traffic.
How I verified it
The following tests were added
BFD Test - apply and remove TSA

Set up a BFD session with shutdown_bfd_during_tsa=true and checks state DB for session creation.
Apply TSA and verify removal of the said session from app db.
Apply TSB and verify reinstated session is app db.

BFD Test- remove active session in TSA

Set up a BFD session with shutdown_bfd_during_tsa=true and checks state DB for session creation.
Apply TSA and verify removal of the said session from app db.
Remove BFD session configuration form state db.
Apply TSB and verify that the session doesn't come up as it has already been removed.

Details if related

Example

under existing imlementation when a device goes into TSA the BFD sessions remain active
Total number of BFD sessions: 4
Peer Addr     Interface    Vrf      State    Type          Local Addr      TX Interval    RX Interval    Multiplier  Multihop      Local Discriminator
------------  -----------  -------  -------  ------------  ------------  -------------  -------------  ------------  ----------  ---------------------
200.251.17.3  default      default  Up       async_active  10.1.0.32              1000           1000            10  true                            3
200.251.17.1  default      default  Up       async_active  10.1.0.32              1000           1000            10  true                            1
200.251.17.4  default      default  Up       async_active  10.1.0.32              1000           1000            10  true                            4
200.251.17.2  default      default  Up       async_active  10.1.0.32              1000           1000            10  true                            2

With this implementation if a BFD sesion is configured with parameter "tsa_shutdown"= "true"
The sessions are brought down upon TSA.

Total number of BFD sessions: 0
Peer Addr     Interface    Vrf      State    Type          Local Addr      TX Interval    RX Interval    Multiplier  Multihop      Local Discriminator
------------  -----------  -------  -------  ------------  ------------  -------------  -------------  ------------  ----------  ---------------------
When the device is taken out of TSA i.e. TSB, the sessions are brought up again.
Total number of BFD sessions: 4
Peer Addr     Interface    Vrf      State    Type          Local Addr      TX Interval    RX Interval    Multiplier  Multihop      Local Discriminator
------------  -----------  -------  -------  ------------  ------------  -------------  -------------  ------------  ----------  ---------------------
200.251.17.3  default      default  Up       async_active  10.1.0.32              1000           1000            10  true                            3
200.251.17.1  default      default  Up       async_active  10.1.0.32              1000           1000            10  true                            1
200.251.17.4  default      default  Up       async_active  10.1.0.32              1000           1000            10  true                            4
200.251.17.2  default      default  Up       async_active  10.1.0.32              1000           1000            10  true                            2

prsunny

Please provide full description of the behavior as to when TSA is applied with some examples.

orchagent/bfdorch.cpp

…-swss into bfd_tsa_support

siqbal1986 · 2023-12-01T00:45:10Z

/azp run

azure-pipelines · 2023-12-01T00:45:17Z

Commenter does not have sufficient privileges for PR 2926 in repo sonic-net/sonic-swss

siqbal1986 · 2023-12-03T00:41:09Z

/azpw run

mssonicbld · 2023-12-03T00:41:11Z

/AzurePipelines run

azure-pipelines · 2023-12-03T00:41:21Z

Azure Pipelines successfully started running 1 pipeline(s).

prsunny · 2023-12-04T17:36:19Z

orchagent/bfdorch.cpp

@@ -298,6 +352,10 @@ bool BfdOrch::create_bfd_session(const string& key, const vector<FieldValueTuple
        {
            tos = to_uint<uint8_t>(value);
        }
+        else if (fvField(i) == "tsa_shutdown")


what is the purpose of this section?

I have added a comment in the code and I have changed the name of the field to shutdown_bfd_during_tsa. The BFD tsa shutdown is handled by the bfd cache in the caller function. This block is added to avoid getting the unknown parameter syslog.

prsunny · 2023-12-04T17:38:16Z

orchagent/vnetorch.cpp

@@ -1817,6 +1817,7 @@ void VNetRouteOrch::createBfdSession(const string& vnet, const NextHopKey& endpo
        FieldValueTuple fvTuple("local_addr", src_ip.to_string());
        data.push_back(fvTuple);
        data.emplace_back("multihop", "true");
+        data.emplace_back("tsa_shutdown", "true");


not clear of the meaning of this variable - 'tsa_shutdown' ?

renamed it already and added comments.

tests/test_bfd.py

orchagent/bfdorch.cpp

prsunny · 2023-12-07T21:29:33Z

orchagent/bfdorch.cpp

+                //specified session gets removed when the device goes into TSA state.
+                //if this parameter is not specified or set to false for a session, the
+                // corrosponding BFD session would be maintained even in TSA state.
+                if (fvField(i) == "shutdown_bfd_during_tsa" && value == "true" )


@arlakshm , fyi (This param can control the individual BFD session state during TSA)

prsunny · 2023-12-07T21:43:09Z

orchagent/bfdorch.cpp

+
+void BfdOrch::handleTsaStateChange(bool tsaState)
+{
+    SWSS_LOG_INFO("BfdOrch TSA state Changed to %d.\n", int(tsaState));


Can you please change this to NOTICE? , also log the existing state

done. now log is in the caller function.

prsunny · 2023-12-07T21:47:27Z

orchagent/bfdorch.cpp

+{
+    SWSS_LOG_ENTER();
+    tsa_enabled = false;
+    SWSS_LOG_ERROR("BgpGlobalStateOrch init complete\n");


What is this ERROR log, please remove

orchagent/bfdorch.cpp

prsunny · 2023-12-07T21:53:57Z

orchagent/bfdorch.cpp

@@ -74,6 +74,16 @@ BfdOrch::BfdOrch(DBConnector *db, string tableName, TableConnector stateDbBfdSes

    Orch::addExecutor(bfdStateNotificatier);
    register_state_change_notif = false;
+    BgpGlobalStateOrch* bgp_global_state_orch = gDirectory.get<BgpGlobalStateOrch*>();


I think you don't need this section. Just initialize tsa_enabled to false in class and handle TSA change in the normal update path.

prsunny · 2023-12-07T21:55:21Z

orchagent/bfdorch.cpp

+            notify_session_state_down(it.first);
+            if (!remove_bfd_session(it.first))
+            {
+                SWSS_LOG_ERROR("Failed to remove BFD session %s\n", it.first.c_str());


Please remove the error log as its handled in the remove function already

prsunny · 2023-12-07T21:56:15Z

orchagent/bfdorch.cpp

+        {
+            if (!create_bfd_session(it.first, it.second))
+            {
+                SWSS_LOG_ERROR("Failed to create BFD session %s\n", it.first.c_str());


please remove the error log

prsunny · 2023-12-12T18:21:11Z

orchagent/bfdorch.cpp

+        {
+            notify_session_state_down(it.first);
+            remove_bfd_session(it.first);
+        } else


else -> next line

orchagent/bfdorch.cpp

prsunny · 2023-12-12T18:24:00Z

Please check coverage of test.

siqbal1986 · 2024-01-26T19:29:50Z

tests/test_vnet.py

@@ -3428,6 +3443,83 @@ def test_vnet_orch_24(self, dvs, testlog):
        # delete vxlan tunnel
        delete_vxlan_tunnel(dvs, tunnel_name)

+    '''
+    Test 25 - Test for BFD TSA and TSB behaviour within overlay tunnel routes.


add description of test here

orchagent/bfdorch.cpp

Bfd session bringdown when entring TSA state. Tests to verify the logic.

02288e5

siqbal1986 requested a review from prsunny as a code owner October 12, 2023 01:36

prsunny requested changes Oct 12, 2023

View reviewed changes

orchagent/bfdorch.cpp Outdated Show resolved Hide resolved

orchagent/bfdorch.cpp Outdated Show resolved Hide resolved

siqbal1986 added 4 commits November 27, 2023 17:26

fixed typo and removed debugging trace.

e41fa6d

Merge branch 'master' into bfd_tsa_support

08dfd64

Merge branch 'master' into bfd_tsa_support

dc6b875

Merge branch 'bfd_tsa_support' of https://github.com/siqbal1986/sonic…

633ee13

…-swss into bfd_tsa_support

prsunny and others added 3 commits December 1, 2023 10:38

Merge branch 'master' into bfd_tsa_support

fad54cf

fixed a make check error

0fb802f

Merge branch 'master' into bfd_tsa_support

227e0d4

prsunny reviewed Dec 4, 2023

View reviewed changes

siqbal1986 added 2 commits December 4, 2023 09:50

Merge branch 'master' into bfd_tsa_support

560d927

fixes and updates in comments.

dd11002

prsunny reviewed Dec 7, 2023

View reviewed changes

siqbal1986 added 2 commits December 7, 2023 16:21

Merge branch 'master' into bfd_tsa_support

6cceac7

fixed changes suggested in review.

a1d2889

prsunny reviewed Dec 12, 2023

View reviewed changes

siqbal1986 commented Jan 26, 2024

View reviewed changes

orchagent/bfdorch.cpp Show resolved Hide resolved

siqbal1986 added 3 commits February 6, 2024 12:28

changes finalized in review.

fddfc5f

Merge branch 'master' into bfd_tsa_support

5363ed3

Merge branch 'master' into bfd_tsa_support

b673871

prsunny approved these changes Feb 7, 2024

View reviewed changes

prsunny merged commit 3bd0144 into sonic-net:master Feb 7, 2024
14 checks passed

siqbal1986 mentioned this pull request Feb 28, 2024

tests for BFD session bringdown during TSA. sonic-net/sonic-mgmt#11805

Merged

8 tasks

siqbal1986 mentioned this pull request May 13, 2024

Traffic Shift Away for BFD Session sonic-net/SONiC#1604

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bfd support for TSA state. #2926

Bfd support for TSA state. #2926

siqbal1986 commented Oct 12, 2023 •

edited

prsunny left a comment

siqbal1986 commented Dec 1, 2023

azure-pipelines bot commented Dec 1, 2023

siqbal1986 commented Dec 3, 2023

mssonicbld commented Dec 3, 2023

azure-pipelines bot commented Dec 3, 2023

prsunny Dec 4, 2023

siqbal1986 Dec 5, 2023

prsunny Dec 4, 2023

siqbal1986 Dec 5, 2023

prsunny Dec 7, 2023

prsunny Dec 7, 2023

siqbal1986 Dec 12, 2023

prsunny Dec 7, 2023

siqbal1986 Dec 12, 2023

prsunny Dec 7, 2023

siqbal1986 Dec 12, 2023

prsunny Dec 7, 2023

siqbal1986 Dec 12, 2023

prsunny Dec 7, 2023

siqbal1986 Dec 12, 2023

prsunny Dec 12, 2023

siqbal1986 Feb 6, 2024

prsunny commented Dec 12, 2023

siqbal1986 Jan 26, 2024

siqbal1986 Feb 6, 2024

Bfd support for TSA state. #2926

Bfd support for TSA state. #2926

Conversation

siqbal1986 commented Oct 12, 2023 • edited

prsunny left a comment

Choose a reason for hiding this comment

siqbal1986 commented Dec 1, 2023

azure-pipelines bot commented Dec 1, 2023

siqbal1986 commented Dec 3, 2023

mssonicbld commented Dec 3, 2023

azure-pipelines bot commented Dec 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prsunny commented Dec 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siqbal1986 commented Oct 12, 2023 •

edited