issue: 3898040 DOCA RX Data Path #174

AlexanderGrissik · 2024-07-02T07:21:05Z

Description

DOCA RX Data Path
Changes in epoll os events handling
Some cleanups

What

It implements DOCA RX Data Path regular RQ including notification mechanism.
It integrates CQ moderation and MSIX affinity DOCA features.
In addition, it removes internal thread involvement in OS events handling for epoll. This improves performance and avoids lock contentions. Now OS polling will be done the same as it was for poll/select. i.e Polling each every loops or epolls for OS events depending on the configuration.
Obsolete and unneeded flags cleanup.

Why ?

DOCA integration

Change type

What kind of change does this PR introduce?

Check list

Code follows the style de facto guidelines of this project
Comments have been inserted in hard to understand places
Documentation has been updated (if necessary)
Test has been added (if possible)

pasis

Few minor comments

pasis · 2024-08-03T21:03:32Z

src/core/dev/hw_queue_rx.cpp

+     */
+    if (safe_mce_sys().app.distribute_cq_interrupts && g_p_app->get_worker_id() >= 0 &&
+        m_p_ib_ctx_handler->is_notification_affinity_supported()) {
+        uint32_t core = g_p_app->get_worker_id() % std::thread::hardware_concurrency();


from the hardware_concurrency() description:

Returns the number of concurrent threads supported by the implementation. The value should be considered only a hint.

Is it reliable approach? What if available CPUs have non-continuous numbering? What if number of completion channels (IRQs) is lower than number of CPUs?

You are correct. This implementation is not good because CPU can be higher than comp channels. We need to ask from DOCA to give us the number of available channels.
I will change this section to use some temporary constant comp ch number just to test functionality.

pasis · 2024-08-03T21:22:48Z

src/core/dev/ring_simple.cpp

 {
    if (!safe_mce_sys().doca_flow) {
-        int ret = -1;
+        m_lock_ring_rx.lock();


[suggestion] replace manual locking with a lock_guard at the top

pasis · 2024-08-03T21:33:49Z

src/core/iomux/epoll_wait_call.cpp

@@ -405,13 +404,12 @@ int epoll_wait_call::ring_poll_and_process_element()
    return m_epfd_info->ring_poll_and_process_element(&m_poll_sn_rx, &m_poll_sn_tx, nullptr);


[question] do we still need "poll sn" logic?

Removed all leftovers of poll sn

pasis · 2024-08-03T21:34:14Z

src/core/dev/hw_queue_rx.cpp

+    if (safe_mce_sys().app.distribute_cq_interrupts && g_p_app->get_worker_id() >= 0 &&
+        m_p_ib_ctx_handler->is_notification_affinity_supported()) {
+        uint32_t core = g_p_app->get_worker_id() % std::thread::hardware_concurrency();
+        hwqrx_loginfo("Setting PE core affinity: %" PRIu32 ", pid: %d", core, getpid());


Replace info with debug log level.

pasis · 2024-08-03T21:38:34Z

src/core/iomux/epfd_info.cpp

@@ -89,10 +87,6 @@ epfd_info::epfd_info(int epfd, int size)

    xlio_stats_instance_create_epoll_block(m_epfd, &(m_stats->stats));

-    // Register this socket to read nonoffloaded data
-    g_p_event_handler_manager->update_epfd(m_epfd, EPOLL_CTL_ADD,


[question] Is "issue: 3502635 Avoid epoll OS polling through internal thread" generic enough to port to vNext?

Yes. Rebased and ported from vNext.

pasis · 2024-08-03T21:39:35Z

README

-Similar to XLIO_RX_SKIP_OS, but in select() or poll() this will force the XLIO
+In select(),poll() or epoll() this will force the XLIO
 to check the non offloaded fd even though an offloaded socket has ready
 packets found while polling.


make a pretty layout with up to 80 symbols in a row

Correct layout ported from vNext

pasis · 2024-08-03T21:42:09Z

src/core/iomux/io_mux_call.cpp

-            // (most likely in case of a wakeup and probably only under epoll_wait (Not
-            // select/poll))
-            ring_clear_rx_notification();
+            // DOCA WA: Unclearable manual triggered events generate false PE events.


[suggestion] Put "XXX" or "TODO" prefix. Some editors such as vim highlight it and this will raise attention to a temporary code.

Adding RX data-path with DOCA ETH RX. Utilizes DOCA regular RX queue. DOCA path is completely moved to hw_queue_rx without using cq_mgr_rx. DPCP path is mostly unchanged and is implemented with cq_mgr_rx. Signed-off-by: Alexander Grissik <[email protected]>

Adding support for RX PE notification moderation with RXQ. Signed-off-by: Alexander Grissik <[email protected]>

Adding interrupt distribution affinity for Nginx/Envoy use cases. Each worker uses different CPU core for PE interrupts. Signed-off-by: Alexander Grissik <[email protected]>

1. Remove global poll sn logic 2. Use lock instead of trylock for notifications 3. Differentiate between clear-notification and poll-and-process 4. Remove poll-and-process from clear-notification. We do check-all-offloaded-sockets after that anyway. This removes poll overhead. Signed-off-by: Alexander Grissik <[email protected]>

This flag logic is unneccessary for epolls without offloaded sockets. We must wait_os for such epolls, otherwise we break DOCA or other application logics. This flag is never changed in practice and changing it will only introduce overhead without additional functionality. In case we do not have offloaded sockets in epoll context there is no reason to go and try to do polling/blocking loops. Signed-off-by: Alexander Grissik <[email protected]>

If request notification called on non empty PE, DOCA will set manual triggered event. This event is not cleared as part of clear notification. Consequent calls to epoll_wait such as inside wait_os generate event again and again, until request notification is called again on that PE. We need to clear false PE events after wait_os to avoid redundant polling attempts. This is a temporary solution. Signed-off-by: Alexander Grissik <[email protected]>

Removing all global poll sn leftovers from all pathes. Global poll SN is an optimization attempt to skip request notification if another thread polled that CQ. This approach is not applicable for HPC application and may introduce polling overhead (It is likely that the second thread drains the CQ anyway). Signed-off-by: Alexander Grissik <[email protected]>

AlexanderGrissik requested a review from pasis July 2, 2024 07:21

AlexanderGrissik added the XLIO_DOCA label Jul 2, 2024

AlexanderGrissik force-pushed the doca_rx branch 2 times, most recently from 904bf19 to dcdcc3d Compare July 28, 2024 12:56

pasis approved these changes Aug 3, 2024

View reviewed changes

AlexanderGrissik added 5 commits August 4, 2024 12:09

issue: 3898040 Adding RX notification moderation

9c4884a

Adding support for RX PE notification moderation with RXQ. Signed-off-by: Alexander Grissik <[email protected]>

issue: 3898040 Adding RX distributed interrupt

d8ca3c0

Adding interrupt distribution affinity for Nginx/Envoy use cases. Each worker uses different CPU core for PE interrupts. Signed-off-by: Alexander Grissik <[email protected]>

AlexanderGrissik force-pushed the doca_rx branch from dcdcc3d to 33f87f8 Compare August 4, 2024 15:16

AlexanderGrissik changed the base branch from doca_xlio to doca_xlio_vNext August 4, 2024 15:17

AlexanderGrissik added 2 commits August 5, 2024 06:55

AlexanderGrissik force-pushed the doca_rx branch from 33f87f8 to 7f134d6 Compare August 5, 2024 06:56

AlexanderGrissik merged commit a8ab162 into Mellanox:doca_xlio_vNext Aug 5, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

issue: 3898040 DOCA RX Data Path #174

issue: 3898040 DOCA RX Data Path #174

AlexanderGrissik commented Jul 2, 2024 •

edited

Loading

pasis left a comment

pasis Aug 3, 2024

AlexanderGrissik Aug 4, 2024

pasis Aug 3, 2024

AlexanderGrissik Aug 4, 2024

pasis Aug 3, 2024

AlexanderGrissik Aug 4, 2024

pasis Aug 3, 2024

AlexanderGrissik Aug 4, 2024

pasis Aug 3, 2024

AlexanderGrissik Aug 4, 2024

pasis Aug 3, 2024

AlexanderGrissik Aug 4, 2024

pasis Aug 3, 2024

AlexanderGrissik Aug 4, 2024

		@@ -405,13 +404,12 @@ int epoll_wait_call::ring_poll_and_process_element()
		return m_epfd_info->ring_poll_and_process_element(&m_poll_sn_rx, &m_poll_sn_tx, nullptr);

issue: 3898040 DOCA RX Data Path #174

issue: 3898040 DOCA RX Data Path #174

Conversation

AlexanderGrissik commented Jul 2, 2024 • edited Loading

Description

What

Why ?

Change type

Check list

pasis left a 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

AlexanderGrissik commented Jul 2, 2024 •

edited

Loading