Skip to contentSkip to conversation

Bug analysis of regexdev library

DISCUSSION Stephen Hemminger 1 participant 1 message Mar 2

lore
Share & tools
lore
#0 lore raw

Since nobody uses it, thought I would test AI review of regexdev library.

It found many things, and not to mention there are no regression tests for the library. Is anyone using or maintaining this? Can it be deprecated in 26.07 and removed in 26.11?

Here's a thorough bug analysis of the DPDK regexdev library:

## Critical Bugs

**1. Wrong comparison operator in rte_regexdev_get_dev_id()** (rte_regexdev.c)

This is the most serious bug. The function uses strcmp() without negation, so it matches on names that are *different* rather than equal:

```c
if (strcmp(name, rte_regex_devices[i].data->dev_name)) {
    id = rte_regex_devices[i].data->dev_id;
    break;
}
```

strcmp() returns 0 (falsy) on match. This should be !strcmp(...) or strcmp(...) == 0, exactly as done correctly in regexdev_allocated(). As written, this returns the ID of the first device whose name *doesn't* match.

**2. Wrong memset value on re-registration** (rte_regexdev.c, rte_regexdev_register())

```c
if (dev->data == NULL)
    dev->data = &rte_regexdev_shared_data->data[dev_id];
else
    memset(dev->data, 1, sizeof(*dev->data));
```

The else branch fills the data structure with byte value 1 (0x01 repeating), not zero. This is almost certainly meant to be memset(dev->data, 0, ...). A fill value of 1 corrupts every field: dev_id becomes garbage, dev_name becomes non-NUL junk, dev_started becomes 1 (true), etc. The if branch also has a problem — newly assigned data from the shared array is never zeroed, so it relies on the one-time memset in regexdev_shared_data_prepare().

**3. rte_regexdev_stop() ignores driver return value** (rte_regexdev.c)

```c
dev->dev_ops->dev_stop(dev);
dev->data->dev_started = 0;
return 0;
```

The dev_stop callback returns int, but the return value is discarded. If the driver fails to stop, the library unconditionally marks the device as stopped and returns success. Compare with dev_start, which correctly checks the return value before updating state.

## Moderate Bugs

**4. rte_regexdev_close() doesn't check driver return** (rte_regexdev.c)

Same pattern as stop — dev_close() return is ignored, and the device is unconditionally marked UNUSED and dev_started = 0 regardless of whether the driver succeeded.

**5. Duplicate dev_started check in rte_regexdev_queue_pair_setup()** (rte_regexdev.c)

The dev->data->dev_started check appears twice, identically:

```c
if (dev->data->dev_started) {  // first check
    ...
    return -EBUSY;
}
if (queue_pair_id >= dev->data->dev_conf.nb_queue_pairs) {
    ...
}
if (dev->data->dev_started) {  // redundant duplicate
    ...
    return -EBUSY;
}
```

This is dead code — the second check is unreachable unless there's a concurrent modification, and the library explicitly states it's not thread-safe.

**6. rte_regexdev_configure() allows nb_max_matches == 0 despite doc** (rte_regexdev.h vs .c)

The header comment for rte_regexdev_config::nb_max_matches says "The value 0 is allowed, in which case, value 1 used." But the implementation explicitly rejects zero:

```c
if (cfg->nb_max_matches == 0) {
    RTE_REGEXDEV_LOG_LINE(ERR, "Dev %u num of matches must be > 0", dev_id);
    return -EINVAL;
}
```

Either the doc or the code is wrong.

**7. Similarly, nb_rules_per_group == 0 rejection contradicts doc**

The header says "The value 0 is allowed, in which case, max_rules_per_group used." But the code rejects it with -EINVAL.

## Design / Robustness Issues

**8. rte_regexdev_xstats_names_get() rejects NULL but API says NULL returns capacity**

The header states: "If set to NULL, function returns required capacity." But the implementation returns -EINVAL on NULL, making it impossible to query the required size.

**9. rte_regexdev_xstats_reset() rejects NULL ids but API says NULL resets all**

The header says: "When NULL, all statistics will be reset." The implementation returns -EINVAL on NULL.

**10. Missing #include <stdio.h> dependency** (rte_regexdev_core.h)

The regexdev_dump_t typedef uses FILE *f but the header doesn't include <stdio.h>. It works by accident through transitive includes, but it's technically a missing dependency.

**11. Type mismatch on nb_rules**

In rte_regexdev.h, rte_regexdev_rule_db_update() declares nb_rules as uint32_t. In rte_regexdev_core.h, the callback typedef regexdev_rule_db_update_t declares it as uint16_t. This silently truncates rule counts above 65535.

**12. regexdev_capa field is uint32_t but capability flags can exceed 32 bits**

Capability RTE_REGEXDEV_CAPA_QUEUE_PAIR_OOS_F is defined as (1ULL << 22) with ULL suffix suggesting 64-bit intent, yet struct rte_regexdev_info::regexdev_capa is uint32_t. Currently the highest bit is 22, so it fits, but the ULL suffix signals the original developers may have intended a 64-bit field, and any future flag at bit 32+ would be silently dropped.

---

The strcmp inversion (bug #1) and the memset(..., 1, ...) (bug #2) are the most impactful — one breaks device lookup by name entirely, the other corrupts device state on re-registration. The xstats NULL-handling mismatches (#8, #9) and the nb_rules type mismatch (#11) are likely to bite real driver consumers.