Skewcy's Blog


First Patch Submission to the DPDK Open-Source Project

Let me first explain why I decided to submit a patch šŸ¤”ā€¦

Recently, I was working on a research project that utilized the DPDK framework. During the process, I realized that the frameworkā€™s driver did not implement all the functionalities specified by the network card. Since one particular functionality, the ā€œlaunchtime featureā€, was essential for my work, I spent some time modifying the driver for my project to use this feature.

After that, I decided to invest some time integrating this patch into the DPDK framework, basically for two main reasons. First, I believe this feature is essential for real-time community to use DPDK, and I had seen several discussions online about it, suggesting that its implementation could benefit many others. Secondly, I had never contributed to a large open-source project before and saw this as an opportunity to learn the basic process of submitting patches.

This blog has been selected as the developer highlight by DPDK community: https://www.dpdk.org/first-patch-submission-to-the-dpdk-open-source-project/

Preparation šŸ“š

Before submitting the patch, itā€™s crucial to understand the projectā€™s development process. DPDK is an open source project currently managed by Linux Foundation. The development of DPDK happens on mailing list (like Linux kernel) instead of on Github. This means we canā€™t simply use git commit to submit patches, then followed by a pull request. Instead, patches are submitted via email to maintainers. Therefore, some extra preparatory steps are needed before submission, including:

Mailing List: The process of registering and subscribing to the mailing list is relatively straightforward and can be done by following the steps in the DPDK official documentation. I wonā€™t repeat that here. I also recommend registering for Patchwork to easily track the code review process. From my experience, itā€™s best to use a separate email for subscription because DPDK generates dozens of patches daily, and mixing it with my regular email totally overflowed my inboxšŸ¤Æ.

Update 03/22/2024: Actually, there is an option to disable receiving all the emails from the mailing list (this way, you will still receive emails regarding your own patches). Alternatively, you can set the digest mode to only receive one summary per day. Simply toggle these options here. With these settings, thereā€™s no need to create a separate email for the mailing list.

MailingList -3quarterwidth

Email Configuration: Setting up the email is also quite simple. I used a Google email, configured it following this article to link git send-email. I didnā€™t encounter any issues during the process.

Finding Maintainers: DPDK is a large, modular open-source project, designed for collaborative work. Each module has a designated maintainer, and some even have specific maintenance branches. Therefore, itā€™s necessary to find the maintainer and the branch of the module you want to modify, and submit the patch to them for integration into the mainline repository. An important point to note is to look for maintainer information in the MAINTAINERS file in the GitHub root directory, not in the list provided in the documentation, as it may not be up-to-date. I made this mistake in my first submission, sending the patch to a previous maintainer.

Update 03/22/2024: You donā€™t need to find maintainers to Cc manually. There is a script for this: devtools/get-maintainer.sh. Just use it like this:

git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh

As for understanding the projectā€™s coding style, I suggest not spending too much time studying the official style guide. This is because you can ensure the consistency of the coding style with the automated review scripts mentioned later.

Writing the Patch šŸ”§

The process of writing the patch is essentially modifying the original code to fix bug or adding new functionalities, and after formatting it with diff, these modifications become the patch. An important suggestion here is to disable the auto-formatting feature of your IDE. Many projects have unique code formatting styles, and auto-formatting might totally mess up the format with Tab/Space changes everywhere šŸ« .

Testing

Testing is divided into three main parts:

Compilation and Functional Testing: Since my modifications were minor, I only compiled it on my local machine. The specific process is largely similar to installing DPDK, except the first step involves using meson setup --wipe build to clear the previous build directory. For functional testing, I recommend testing all DPDK built-in apps that might be affected, such as dpdk-testpmd.

Code Style Check: This part can be done following the Section 5.7 Checking the Patches in the DPDK official tutorial. Itā€™s important to note that you need to download checkpatch.pl yourself (You can get the source code by google searching, just a simple script). After generating codespell-dpdk.txt as instructed, run devtools/checkpatches.sh patch_name.patch in the same directory. The output log shows the results, where both errors and warnings need to be corrected, but some checks may not need changes if they are unreasonable.

ABI Policy: ABI Policy refers to the requirements for variable and function naming. It is recommended to self-check against the official ABI Guideline. But personally I feel if not new file created, there shouldnā€™t be many issues, just compare with the surrounding code to get a sense of the appropriate naming conventions.

Submitting the Patch

After testing, it is good to generate the patch. Before doing so, itā€™s recommended to double-check your git username and email, using git config --global user.name & git config --global user.email.

My method of generating patches might not be the best practice. I initially used git add & git commit for a simple commit, then git commit --amend to modify the commit message. Finally, I generated the patch with git format-patch -1 -o ~/patch/. Here -1 indicates the patch-set includes the last few commits, but I only needed to generate one patch.

Before submission, check if the patch contains the Signed-off-by: xxxxx <xxxxxx@xxx> line. Mine wasnā€™t generated automatically, so I added it manually.

Finally, the patch can be submitted via git send-email, typically --to the maintainer, and --cc to dev@dpdk.org.

git send-email \
--to="xxxxxxx@xxx.com" \
--to="xxxxxxx@xxx.com" \
--cc="dev@dpdk.org"\ 
./patch_name.patch 

Update 03/22/2024: Again, no need to find maintainers to Cc manually. There is a script for this: devtools/get-maintainer.sh.

Before sending, it is best to first send it to yourself to check if the email format is correct with git send-email --to=[your email] patch_name.patch.

If you have registered for Patchwork, youā€™ll receive the patch submission notification after a few hours.

Specific Content of My Patch

Next, Iā€™ll briefly introduce what my patch does. If youā€™re not interested in DPDK development or technical details, feel free to skip to the next section [Code Review].

My patch primarily adds a real-time scheduling feature to the Intel igb driver, allowing packets to be sent precisely at the times we set. According to the network card manual, the precision can be within 16 nanoseconds, which is pretty accurate. Here are the main details of the patch:

Firstly, in e1000_regs.h, I exposed the register addresses needed for the feature. Those are needed to configure NIC into the Qav mode to use its launch time offloading feature.

/* QAV Tx mode control register */
#define E1000_I210_TQAVCTRL    0x3570
#define E1000_I210_LAUNCH_OS0  0x3578

Then, in igb_ethdev.c, during device initialization, I set the register values based on the portā€™s parameters to enable the related feature. Determining the register addresses and values requires referring to the device manual, which can be time-consuming.

Here I set the register field for fetch policy as E1000_TQAVCTRL_FETCH_ARB instead of the E1000_FETCH_TIME_DELTA (0xFFFF << 16). This is because the the FETCH_ARB fetching prioritizes the most empty queue, regardless of the fetch time. (referring Sec 7.2.7.5). Compare with fetch time, arbitrary fetching seems more suitable as default as DPDK lacks an interface to set fetch delay, unlike in the kernel which can be configured (e.g., through ā€˜Deltaā€™ in ETF Qdisc).

if (igb_tx_timestamp_dynflag > 0) {
  tqavctrl = E1000_READ_REG(hw, E1000_I210_TQAVCTRL);
  tqavctrl |= E1000_TQAVCTRL_MODE; /* Enable Qav mode */
  tqavctrl |= E1000_TQAVCTRL_FETCH_ARB; /* ARB fetch, no Round Robin*/
  tqavctrl |= E1000_TQAVCTRL_LAUNCH_TIMER_ENABLE; /* Enable Tx launch time*/
  E1000_WRITE_REG(hw, E1000_I210_TQAVCTRL, tqavctrl);
  E1000_WRITE_REG(hw, E1000_I210_LAUNCH_OS0, igb_tx_offset(dev));
}

Next, in igb_rxtx.c, I set the transmission pathā€™s Tx descriptor for packet transmission. Essentially, this involves writing the schedule time to the dynamic descriptor for each packet. This slides of dynamic descriptors is a good material to learn the concept if you are interested in.

// In eth_igb_xmit_pkts, get the scheduled time through RTE_MBUF_DYNFIELD
if (igb_tx_timestamp_dynflag > 0) {
  ts = *RTE_MBUF_DYNFIELD(tx_pkt,
  igb_tx_timestamp_dynfield_offset, uint64_t *);
  igbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req, tx_offload, ts);
} else {
  igbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req, tx_offload, 0);
}

Then write the timestamp into the mbuf. It is clear to see here the granularity of scheduling time is 32 ns.

// Write the schedule time into mbuf in igbe_set_xmit_ctx
if (txtime) {
 launch_time = (txtime - IGB_I210_TX_OFFSET_BASE) % NSEC_PER_SEC;
 ctx_txd->u.launch_time = rte_cpu_to_le_32(launch_time / 32);
} else {
 ctx_txd->u.launch_time = 0;
}

From here, the NIC takes over, sending packets based on the scheduled time set by the user.

Challenges

One challenge I encountered was that the actual packet transmission time in the NIC was not same as the schedule time I set. After thoroughly reviewing the device manual and the kernel driver implementation, I found no explanations for this discrepancy. To address this issue, I decided to investigate whether the gap between the actual sending time and the expected time was consistent across all devices. Fortunately, I had access to several devices using this driver for testing purposes. After conducting a series of tests, I observed that the difference between packet transmission and scheduled times remained constant within each test, only varying with different port speeds. Based on these findings, I think compensating for this difference in the scheduling time, taking into account the current port speed, would be the most effective solution.

First, in e1000_ethdev.h, I hardcoded the delay values tested on two different NICs to ensure compatibility with different devices.

/*
 * Macros to compensate the constant latency observed in i210 for launch time
 *
 * launch time = (offset_speed - offset_base + txtime) * 32
 * offset_speed is speed dependent, set in E1000_I210_LAUNCH_OS0
 */
#define IGB_I210_TX_OFFSET_BASE                0xffe0
#define IGB_I210_TX_OFFSET_SPEED_10            0xc7a0
#define IGB_I210_TX_OFFSET_SPEED_100           0x86e0
#define IGB_I210_TX_OFFSET_SPEED_1000          0xbe00

Then, depending on the current network speed, I selected different compensation values.

static uint32_t igb_tx_offset(struct rte_eth_dev *dev)
{
  struct e1000_hw *hw =
    E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);

  uint16_t duplex, speed;
  hw->mac.ops.get_link_up_info(hw, &speed, &duplex);

  uint32_t launch_os0 = E1000_READ_REG(hw, E1000_I210_LAUNCH_OS0);
  if (hw->mac.type != e1000_i210) {
    /* Set launch offset to base, no compensation */
    launch_os0 |= IGB_I210_TX_OFFSET_BASE;
  } else {
    /* Set launch offset depend on link speeds */
    switch (speed) {
    case SPEED_10:
      launch_os0 |= IGB_I210_TX_OFFSET_SPEED_10;
      break;
    case SPEED_100:
      launch_os0 |= IGB_I210_TX_OFFSET_SPEED_100;
      break;
    case SPEED_1000:
      launch_os0 |= IGB_I210_TX_OFFSET_SPEED_1000;
      break;
    default:
      launch_os0 |= IGB_I210_TX_OFFSET_BASE;
      break;
    }
  }

  return launch_os0;
}

Lastly, it writes the compensation values into the E1000_I210_LAUNCH_OS0 register once initialize the device. This approach has some trade off for optimization consideration. Instead of the common approach of fetching the current port rate and modifying the schedule time during packet transmission, I chose to only read the network card speed during device startup and write this value into the register. This utilizes the register value in calculating the final schedule time, thereby avoiding the overhead of querying the port speed each packet. The drawback of this approach is that if the port speed changes during packet transmission, the compensation value canā€™t be adjusted in real time. But I think change port speed during runtime is relatively a rare case overall.

Code Review šŸ‘€

Compared to smaller open-source projects, code review in DPDK is quite stringent, involving both automated reviews from system and manual reviews from maintainers.

The results of the automated review are received via email within a few hours of submission. Fortunately, despite this being my first patch submission, no errors were detected with beginner luck šŸ’«.

Automated Review -3quarterwidth

In the subsequent manual review phase, maintainers pose questions about the content of the patch, which feels similar to the rebuttal phase of a research paper submission process. The main issues raised in my case focused on:

Responding directly to these questions is usually sufficient. A few minor details to note:

git send-email \ 
--in-reply-to=xxxxxxxx.namprd11.prod.outlook.com \ 
--to=xxxx@xxx.com \ 
--cc=xxxx@xxx.com \ 
--subject="RE: [PATCH] net/e1000: support launchtime featureā€ \
./reply.txt

Although the process seemed somewhat complicated to me initially, the friendly atmosphere made it smoother. From submission to the final review, it took me over ten days, which included three replies and one code update. Along the way, I encountered some technical issues and uncertainties. However, these were resolved with the help of the maintainers and other developers, who provided guidance and support throughout the process.

Conclusion šŸŽ‰

The biggest take-away for me from this experience is that contributing to a large open-source project is not as daunting as it might seem. Itā€™s often the intricate rules and the stereotypes about open-source community that dissuade people from getting involved. However, I found that the community was welcoming, and the process, while detailed, was manageable when you are willing to spent some time on it.

Although contributing to open-source community is not my focus, attempting to submit a patch was definitely a valuable experience šŸ¤—.


skewcy@gmail.com