Add host_tx_ready enhancements #2930

noaOrMlnx · 2023-10-12T09:40:33Z

Depending on sonic-net/sonic-swss-common#824
Depending on sonic-net/sonic-sairedis#1307

HLD: sonic-net/SONiC#1453

What I did
Add host_tx_ready enhancements to sonic-swss branch according to HLD mentioned above.

Why I did it
Module and Asic configurations must be synchronized in order to not configure an non-existing module.

How I verified it
Manually & unit tests.

Details if related

noaOrMlnx · 2023-11-29T08:33:04Z

/azpw run Azure.sonic-swss

mssonicbld · 2023-11-29T08:33:07Z

/AzurePipelines run Azure.sonic-swss

azure-pipelines · 2023-11-29T08:33:14Z

Azure Pipelines successfully started running 1 pipeline(s).

noaOrMlnx · 2023-12-03T12:56:39Z

/azpw run Azure.sonic-swss

mssonicbld · 2023-12-03T12:56:42Z

/AzurePipelines run Azure.sonic-swss

azure-pipelines · 2023-12-03T12:56:51Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2023-12-05T06:46:55Z

tests/test_warm_reboot.py

@@ -60,6 +60,19 @@ def check_port_oper_status(appl_db, port_name, state):
            break
    assert oper_status == state

+def check_port_host_tx_ready_status(state_db, port_name, status):


@noaOrMlnx have you tested in actual switch host_tx_ready is not deleted during warm-reboot? If its not delted, what is preventing it from deleted from STATE_DB's port table? Any code pointers?

The table is deleted.
The thing is, that after warm reboot, host_tx_ready value will be false, and we won't get any new notification from lower layers saying that host_tx_ready=true.
That's the reason we need to take the current value of host_tx_ready using the new API

prgeor · 2023-12-05T06:49:03Z

orchagent/portsorch.cpp

@@ -7345,6 +7521,18 @@ void PortsOrch::doTask(NotificationConsumer &consumer)

        sai_deserialize_free_port_oper_status_ntf(count, portoperstatus);
    }
+    else if (&consumer == m_portHostTxReadyNotificationConsumer && op == "port_host_tx_ready")


@noaOrMlnx Have you tested, when user does admin down, the host_tx_ready becomes false?

prgeor · 2023-12-05T06:51:32Z

orchagent/portsorch.cpp

@@ -4469,6 +4561,69 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer)
    }
 }

+void PortsOrch::doTransceiverInfoTableTask(Consumer &consumer)


@noaOrMlnx I would suggest to rename "doTransceiverPresenceCheck" since this routine is checking the presence of the module as notified in TRANSCEIVER_INFO SET/DEL

prgeor · 2023-12-05T06:53:14Z

orchagent/portsorch.cpp

+bool PortsOrch::setSaiHostTxSignal(const Port &port, bool enable)
+{
+    sai_attribute_t attr;
+    attr.id = SAI_PORT_ATTR_HOST_TX_SIGNAL_ENABLE;


@noaOrMlnx Will this crash for platform that doesn't support this new SAI attribute? See failure message?

A vendor which doesn't support this function should not get to this flow.
setSaiHostTxSignal is called only from doTransceiverPresenceCheck() function.

This function will be called when TRANSCEIVER_INFO table in STATE DB will change.
OA will listen to the table, only if the flow is supported:

if (saiHwTxSignalSupported && saiTxReadyNotifySupported) { SWSS_LOG_DEBUG("m_cmisModuleAsicSyncSupported is true"); m_cmisModuleAsicSyncSupported = true; Orch::addExecutor(new Consumer(new SubscriberStateTable(stateDb, STATE_TRANSCEIVER_INFO_TABLE_NAME, TableConsumable::DEFAULT_POP_BATCH_SIZE, 0), this, STATE_TRANSCEIVER_INFO_TABLE_NAME)); }

noaOrMlnx · 2023-12-07T10:41:07Z

/azpw run Azure.sonic-swss

mssonicbld · 2023-12-07T10:41:09Z

/AzurePipelines run Azure.sonic-swss

azure-pipelines · 2023-12-07T10:41:20Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2023-12-08T05:45:40Z

@noaOrMlnx please check if failure is due to some other PR dependency

prgeor · 2024-01-02T22:36:04Z

orchagent/portsorch.cpp

+        return;
+    }
+
+    SWSS_LOG_NOTICE("Setting host_tx_ready status = %s, port_id = 0x%" PRIx64, status.c_str(), portId);


@noaOrMlnx please log the logical port name (port.m_alias) as well for easy identification

@noaOrMlnx please add the logical port name as done previously port.m_alias.c_Str()

@prgeor Done

prgeor · 2024-01-02T22:37:05Z

orchagent/portsorch.cpp

    {
        m_portStateTable.hset(port.m_alias, "host_tx_ready", "false");
        SWSS_LOG_NOTICE("Set host_tx_ready to false as gbstatus is false "
                        "for port %s", port.m_alias.c_str());
    }

    /* Update the state table for host_tx_ready*/
-    if (state && (gbstatus == true) && (status == SAI_STATUS_SUCCESS) )
+    if (state && (gbstatus == true) && (status == SAI_STATUS_SUCCESS) && !m_cmisModuleAsicSyncSupported)


@noaOrMlnx can we use the newly introduced API setHostTxReady() here as well instead of directly calling hset?

prgeor · 2024-01-02T22:37:12Z

orchagent/portsorch.cpp

+
+        if (!m_cmisModuleAsicSyncSupported)
+        {
+            m_portStateTable.hset(port.m_alias, "host_tx_ready", "false");


@noaOrMlnx can we use the newly introduced API setHostTxReady() here as well instead of directly calling hset?

prgeor · 2024-01-02T22:37:39Z

orchagent/portsorch.cpp

    /* Update the host_tx_ready to false before setting admin_state, when admin state is false */
-    if (!state)
+    if (!state && !m_cmisModuleAsicSyncSupported)


@noaOrMlnx can we use the newly introduced API setHostTxReady() here as well instead of directly calling hset?

and add log with port alias

What I did Add host_tx_ready enhancements to sonic-swss branch according to HLD Why I did it Module and Asic configurations must be synchronized in order to not configure an non-existing module.

mssonicbld · 2024-01-18T16:33:38Z

Cherry-pick PR to 202311: #3016

mssonicbld · 2024-01-21T23:01:10Z

@noaOrMlnx cherry pick PR didn't pass PR checker. Please check!!!
#3016

What I did Add host_tx_ready enhancements to sonic-swss branch according to HLD Why I did it Module and Asic configurations must be synchronized in order to not configure an non-existing module.

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch 2 times, most recently from 7c4f5c4 to 68ccc1d Compare October 22, 2023 12:41

noaOrMlnx mentioned this pull request Oct 30, 2023

CMIS Module Management Enhancement HLD sonic-net/SONiC#1453

Merged

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch from 68ccc1d to 62a59ae Compare November 12, 2023 20:32

noaOrMlnx requested a review from prgeor November 20, 2023 07:39

noaOrMlnx marked this pull request as ready for review November 20, 2023 07:40

noaOrMlnx requested a review from prsunny as a code owner November 20, 2023 07:40

Junchao-Mellanox approved these changes Nov 24, 2023

View reviewed changes

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch from 62a59ae to adaf4be Compare November 29, 2023 07:39

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch from adaf4be to 28fbbec Compare November 30, 2023 07:13

keboliu force-pushed the host_tx_ready_swss_enhancements branch from 28fbbec to 7efd62e Compare December 1, 2023 09:19

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch from 7efd62e to 45242f5 Compare December 3, 2023 12:57

prgeor reviewed Dec 5, 2023

View reviewed changes

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch from ab22f5c to f30fc35 Compare December 7, 2023 05:46

noaOrMlnx added the Request for 202311 Branch label Dec 7, 2023

prgeor previously approved these changes Dec 8, 2023

View reviewed changes

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch from f30fc35 to 3d1a6f3 Compare December 10, 2023 07:07

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch from c3301ba to 1c6a1fb Compare December 28, 2023 08:28

noaOrMlnx added 2 commits January 1, 2024 09:30

Update test_warm_reboot & test_admin_status

642ec47

Add new vs test

e20e649

prsunny previously approved these changes Jan 2, 2024

View reviewed changes

prgeor reviewed Jan 2, 2024

View reviewed changes

noaOrMlnx dismissed prsunny’s stale review via bab14da January 3, 2024 09:36

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch 3 times, most recently from 7a929f4 to 7a2ab85 Compare January 3, 2024 10:38

prsunny previously approved these changes Jan 4, 2024

View reviewed changes

Update direct calls to hset, to an API call

56eb52d

and add log with port alias

noaOrMlnx dismissed prsunny’s stale review via 56eb52d January 8, 2024 18:24

noaOrMlnx force-pushed the host_tx_ready_swss_enhancements branch from 7a2ab85 to 56eb52d Compare January 8, 2024 18:24

Merge branch 'master' into host_tx_ready_swss_enhancements

a5afd10

prgeor approved these changes Jan 9, 2024

View reviewed changes

prsunny approved these changes Jan 9, 2024

View reviewed changes

prsunny merged commit 7702b8a into sonic-net:master Jan 9, 2024
14 of 15 checks passed

yxieca added the Approved for 202311 Branch label Jan 18, 2024

mssonicbld mentioned this pull request Jan 18, 2024

[action] [PR:2930] Add host_tx_ready enhancements (#2930) #3016

Merged

mssonicbld added the Created PR to 202311 Branch label Jan 18, 2024

mssonicbld added Included in 202311 Branch and removed Created PR to 202311 Branch labels Jan 22, 2024

saksarav-nokia mentioned this pull request Mar 8, 2024

Orchagent crashes everytime in switch create with master image in BCM DNX platform sonic-net/sonic-buildimage#18300

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add host_tx_ready enhancements #2930

Add host_tx_ready enhancements #2930

noaOrMlnx commented Oct 12, 2023 •

edited

Loading

noaOrMlnx commented Nov 29, 2023

mssonicbld commented Nov 29, 2023

azure-pipelines bot commented Nov 29, 2023

noaOrMlnx commented Dec 3, 2023

mssonicbld commented Dec 3, 2023

azure-pipelines bot commented Dec 3, 2023

prgeor Dec 5, 2023

noaOrMlnx Dec 6, 2023

prgeor Dec 5, 2023 •

edited

Loading

noaOrMlnx Dec 5, 2023

prgeor Dec 5, 2023

noaOrMlnx Dec 6, 2023

prgeor Dec 5, 2023

noaOrMlnx Dec 6, 2023

noaOrMlnx commented Dec 7, 2023

mssonicbld commented Dec 7, 2023

azure-pipelines bot commented Dec 7, 2023

prgeor commented Dec 8, 2023

prgeor Jan 2, 2024

prgeor Jan 8, 2024

noaOrMlnx Jan 8, 2024

prgeor Jan 2, 2024

noaOrMlnx Jan 3, 2024

prgeor Jan 2, 2024

noaOrMlnx Jan 3, 2024

prgeor Jan 2, 2024

noaOrMlnx Jan 3, 2024

mssonicbld commented Jan 18, 2024

mssonicbld commented Jan 21, 2024

Add host_tx_ready enhancements #2930

Add host_tx_ready enhancements #2930

Conversation

noaOrMlnx commented Oct 12, 2023 • edited Loading

noaOrMlnx commented Nov 29, 2023

mssonicbld commented Nov 29, 2023

azure-pipelines bot commented Nov 29, 2023

noaOrMlnx 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

prgeor Dec 5, 2023 • edited Loading

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

noaOrMlnx commented Dec 7, 2023

mssonicbld commented Dec 7, 2023

azure-pipelines bot commented Dec 7, 2023

prgeor commented Dec 8, 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

mssonicbld commented Jan 18, 2024

mssonicbld commented Jan 21, 2024

noaOrMlnx commented Oct 12, 2023 •

edited

Loading

prgeor Dec 5, 2023 •

edited

Loading