|WARNING| [v1] crypto/qat: fix IPsec MB header include for ARM

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Tue Jun 30 12:35:20 CEST 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/166406

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-30

# DPDK Patch Review

## Patch Summary
**Subject:** crypto/qat: fix IPsec MB header include for ARM  
**Type:** Bug fix  
**Files Modified:** drivers/crypto/qat/qat_sym_session.h

---

## Review Findings

### Errors

None found.

### Warnings

#### 1. Missing Cc: stable at dpdk.org tag

This is a bug fix patch (indicated by the "Fixes:" tag) that corrects a header include issue affecting ARM builds. Bug fixes should typically be backported to stable releases.

**Suggested fix:** Add the following tag to the commit message:
```
Cc: stable at dpdk.org
```

### Info

#### 1. Header include logic simplification

The patch changes the conditional inclusion logic from:
```c
#ifndef RTE_QAT_OPENSSL
#ifndef RTE_ARCH_ARM
#include <intel-ipsec-mb.h>
#endif
#endif
```

To:
```c
#ifdef RTE_ARCH_ARM
#include <ipsec-mb.h>
#else
#include <intel-ipsec-mb.h>
#endif
```

**Analysis:** The new logic removes the `RTE_QAT_OPENSSL` guard entirely, meaning the IPsec MB headers will now be included unconditionally. This changes behavior when `RTE_QAT_OPENSSL` is defined:

- **Before:** No IPsec MB header included when `RTE_QAT_OPENSSL` is defined
- **After:** IPsec MB header included regardless of `RTE_QAT_OPENSSL`

**Consideration:** Verify that this is the intended behavior. If `RTE_QAT_OPENSSL` mode requires excluding IPsec MB headers, the guard should be retained:

```c
#ifndef RTE_QAT_OPENSSL
#ifdef RTE_ARCH_ARM
#include <ipsec-mb.h>
#else
#include <intel-ipsec-mb.h>
#endif
#endif
```

If the commit message indicates that the OpenSSL path should also use IPsec MB (as suggested by the Fixes reference to "require IPsec MB for HMAC precomputes"), then the current patch is correct and the commit message should explain why the OpenSSL guard was removed.

---

## Summary

The patch correctly addresses the ARM build issue by including the appropriate header (`ipsec-mb.h` vs `intel-ipsec-mb.h`). However:

1. **Missing stable tag** (Warning) - bug fixes should include `Cc: stable at dpdk.org`
2. **Behavioral change** (Info) - removing the `RTE_QAT_OPENSSL` guard changes when headers are included; verify this is intentional and consider documenting the reason in the commit message

## Code Quality Assessment

- **Style compliance:** Good - follows DPDK header guard conventions
- **Correctness:** Appears correct for the ARM case; OpenSSL case needs clarification
- **Documentation:** Commit message could be more detailed about why `RTE_QAT_OPENSSL` guard was removed


More information about the test-report mailing list