Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

issue: 3623390 Implementing DOCA Flow steering creation #150

Merged
merged 4 commits into from
Jul 24, 2024
Merged

issue: 3623390 Implementing DOCA Flow steering creation #150

merged 4 commits into from
Jul 24, 2024

Conversation

AlexanderGrissik
Copy link
Collaborator

Description

Adds RX steering through DOCA and enables receiving first packets with multi process application.
This PR is based on PR 138.
This PR requires DOCA 2.7, commit <17ad5454c600b487bb5e7b63b5d406f196f48922 ([devtools]: fix error in binary compatibility script)> with the attached patch.
0001-TEMP-Enable-Multiprocess-DPDK.patch

What
  1. Adds Steering through DOCA
  2. Adds DOCA RXQ creation/destruction and initial usage
  3. Enables fork() to work with DOCA/DPDK for Nginx
Why ?

DOCA integration

Change type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

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)

@AlexanderGrissik AlexanderGrissik force-pushed the doca_tcp_steering branch 3 times, most recently from b78d0b5 to ea1de00 Compare May 30, 2024 13:17
iftahl
iftahl reviewed Jun 6, 2024
View reviewed changes
src/core/dev/hw_queue_rx.cpp Outdated
hwqrx_logerr("DOCA RXQ CPU-path REGULAR-type is not supported");
return false;
}
hwqrx_loginfo("RXQ caps MaxBurstSize %u, MaxPacketSize %u, Dev:%s", max_burst_size,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remember to make it log debug, and not info sometime in the future PRs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. We will revisit all the info logs one we have a more stable version. Now it is convenient for debugging on initial stages of DOCA integration.

src/core/util/sys_vars.cpp Outdated
@@ -2005,6 +2005,13 @@ void mce_sys_var::get_env_params()

void set_env_params()
{
#if defined(DEFINED_NGINX)
// We must not call setenv for Nginx master process.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it only for doca branch?
Is it permanent? or will be removed later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently it is only for DOCA. But looks like it is needed also for regular XLIO. Today it seems to work by luck in regular XLIO.

src/core/util/sys_vars.h
@@ -799,7 +799,7 @@ extern mce_sys_var &safe_mce_sys();
#define MCE_DEFAULT_CQ_AIM_MAX_PERIOD_USEC (250)
#define MCE_DEFAULT_CQ_AIM_INTERVAL_MSEC (250)
#define MCE_DEFAULT_CQ_AIM_INTERRUPTS_RATE_PER_SEC (5000)
#define MCE_DEFAULT_CQ_POLL_BATCH (16)
#define MCE_DEFAULT_CQ_POLL_BATCH (128)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to change it at this point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works well for now. And this is the nginx default anyway. We can do the tuning later.

@AlexanderGrissik AlexanderGrissik force-pushed the doca_tcp_steering branch 6 times, most recently from 3e3ff8b to 8ab1f7c Compare July 23, 2024 12:05
@AlexanderGrissik
issue: 3623390 DOCA Flow Port initialization
8ffb72b 
1. Start DOCA flow port on demand. Starting DOCA Flow port takes significant amount of time, so we prefer to avoid starting it always on all devices.
2. Handle DOCA flow termination. DOCA Flow creates DPDK threads and DPDK infinite internal thread which uses epoll which is offloaded by XLIO itself.
   For this, we need first termiante all DOCA flow related elements and then destroy the fd_collection while not allowing new epolls to go through XLIO.

Signed-off-by: Alexander Grissik <[email protected]>
@AlexanderGrissik
issue: 3623390 Creating DOCA root control pipe per device
0e1f824 
Creating root control pipe without any mask as entry point for adding rules per device.

Signed-off-by: Alexander Grissik <[email protected]>
@AlexanderGrissik
issue: 3623390 Implementing DOCA Flow steering creation
136e533 
1. Isolated mode DOCA Flow steering for multi-process
2. Start DOCA RXQ and attach with DOCA FLow

Signed-off-by: Alexander Grissik <[email protected]>
@AlexanderGrissik AlexanderGrissik force-pushed the doca_tcp_steering branch 4 times, most recently from b17195d to 86ad7dd Compare July 23, 2024 15:23
iftahl
iftahl reviewed Jul 24, 2024
View reviewed changes
src/core/dev/hw_queue_rx.cpp

err = doca_eth_rxq_set_type(m_doca_rxq.get(), DOCA_ETH_RXQ_TYPE_REGULAR);
if (DOCA_IS_ERROR(err)) {
PRINT_DOCA_ERR(hwqrx_logerr, err, "doca_eth_rxq_get_type_supported");
return false;
}

err = doca_eth_rxq_task_recv_set_conf(m_doca_rxq.get(), task_completion_cb, rx_task_error_cb,
safe_mce_sys().cq_poll_batch_max);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we use here "cq_poll_batch_max"?
if I understand it correctly - we should be able to post big number of tasks to fill the rq, but cq_poll_batch_max refers to how many wqes to poll we do for each polling.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just temporary. It will be changed.

src/core/dev/rfs_rule.cpp
@@ -88,6 +99,41 @@ bool rfs_rule::create(dpcp::match_params &match_value, dpcp::match_params &match
*reinterpret_cast<uint64_t *>(match_value.dst.ipv6),
*reinterpret_cast<uint64_t *>(match_value.dst_mac));

if (0) { // TEMP: Switch to 1 to enable DOCA Flow
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you had a param for that before, don't you want it here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This param will be added in RX path PR. Here it's just to be able to turn on for debugging. RX data path is not working yet. Only receiving a few packets.

@AlexanderGrissik AlexanderGrissik removed the request for review from pasis July 24, 2024 10:42
@AlexanderGrissik AlexanderGrissik merged commit 59a35cf into Mellanox:doca_xlio Jul 24, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iftahl iftahl iftahl approved these changes

Assignees
No one assigned
Labels
XLIO_DOCA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AlexanderGrissik @iftahl