DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH 4/7] app/test/mempool_perf: drop constant-values replay
From: Morten Brørup @ 2026-06-01 13:22 UTC (permalink / raw)
  To: Andrew Rybchenko, Stephen Hemminger, dev
In-Reply-To: <1047eaf3-f41e-49b9-a4cf-0eb7d46dc6c6@oktetlabs.ru>

> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
> Sent: Monday, 1 June 2026 10.35
> 
> On 5/29/26 8:10 PM, Stephen Hemminger wrote:
> > The second nested matrix replays each (n_get_bulk == n_put_bulk)
> > point with use_constant_values=1 to exercise the compile-time
> > constant bulk-size paths in test_loop().  This roughly doubles the
> > work for the get/put diagonal at every n_keep without adding new
> > signal: the cycles/op result for a constant bulk is interesting in
> > isolated inlining studies, not in routine regression sweeps.
> >
> > Drop the replay.  The use_constant_values switch and its branches
> > in test_loop() are retained for now since they are exercised by
> > hand in any local benchmarking.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> As far as I can see you delete the only place where use_constant_values
> is set to 1. It looks suspicious and basically preserves dead code.
> Since Morten added the code, the patch should wait for his approval.

Having used the mempool perf test extensively myself, I agree that it is painfully slow.
Mempools are very often used with constant request sizes, so testing their performance remains relevant for regression sweeps too.
NAK to this change.

Maybe the testing of constant values could be reduced by using a subset of the non-constant mix of values.

> 
> > ---
> >   app/test/test_mempool_perf.c | 8 --------
> >   1 file changed, 8 deletions(-)
> >
> > diff --git a/app/test/test_mempool_perf.c
> b/app/test/test_mempool_perf.c
> > index 19591ad0c9..dd2f0bbaca 100644
> > --- a/app/test/test_mempool_perf.c
> > +++ b/app/test/test_mempool_perf.c
> > @@ -423,14 +423,6 @@ do_one_mempool_test(struct rte_mempool *mp,
> unsigned int cores, int external_cac
> >   				ret = launch_cores(mp, cores);
> >   				if (ret < 0)
> >   					return -1;
> > -
> > -				/* replay test with constant values */
> > -				if (n_get_bulk == n_put_bulk) {
> > -					use_constant_values = 1;
> > -					ret = launch_cores(mp, cores);
> > -					if (ret < 0)
> > -						return -1;
> > -				}
> >   			}
> >   		}
> >   	}


^ permalink raw reply

* Re: [PATCH v2] mbuf: fix mbuf operations history recording
From: Thomas Monjalon @ 2026-06-01 13:31 UTC (permalink / raw)
  To: Morten Brørup; +Cc: dev, Shani Peretz, Konstantin Ananyev, stable
In-Reply-To: <20260511133952.65539-1-mb@smartsharesystems.com>

11/05/2026 15:39, Morten Brørup:
> This addresses two bugs in mbuf operations history recording.
> 
> 1. With mbuf operations history recording enabled, when allocating mbufs
> from a mempool failed, the array of fetched mbuf pointers was not set, but
> it was dereferenced for mbuf operations history recording anyway, which
> would trigger a segmentation fault or cause undefined behavior.
> 
> This was fixed by changing how the return value from the mempool
> allocation is checked, so the function returns early on failure, and only
> proceeds on success.
> 
> 2. When allocating a bulk of mbufs using rte_pktmbuf_alloc_bulk(), two
> mbuf library allocation operations were recorded on the mbuf, because the
> function calls rte_mbuf_raw_alloc_bulk() for allocation, and both
> functions record a mbuf library allocation operation.
> 
> This was fixed by not recording a mbuf library allocation operation in
> rte_pktmbuf_alloc_bulk().
> 
> 3. When freeing a bulk of segmented mbufs, the free operations were only
> recorded on the first segments.
> 
> This was fixed by freeing the pending bulks of segments using
> rte_mbuf_raw_free_bulk(), which records the free operation on the mbufs,
> instead of calling rte_mempool_put_bulk() directly.
> The bulk operation recording at the start of the function, which only
> affected the first segments of segmented packets, was removed.
> 
> Fixes: d265a24a32a4 ("mbuf: record mbuf operations history")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

Applied, thanks.




^ permalink raw reply

* Re: [PATCH v6] mempool: improve cache behaviour and performance
From: Thomas Monjalon @ 2026-06-01 13:36 UTC (permalink / raw)
  To: Morten Brørup
  Cc: dev, Andrew Rybchenko, Bruce Richardson, Jingjing Wu,
	Praveen Shetty, Hemant Agrawal, Sachin Saxena
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35F6589E@smartserver.smartshare.dk>

26/05/2026 18:00, Morten Brørup:
> > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > Sent: Tuesday, 26 May 2026 16.00
> > 
> > This patch refactors the mempool cache to eliminate some unexpected
> > behaviour and reduce the mempool cache miss rate.
> > 
> > 1.
> > The actual cache size was 1.5 times the cache size specified at run-
> > time
> > mempool creation.
> > This was obviously not expected by application developers.
> > 
> > 2.
> > In get operations, the check for when to use the cache as bounce buffer
> > did not respect the run-time configured cache size,
> > but compared to the build time maximum possible cache size
> > (RTE_MEMPOOL_CACHE_MAX_SIZE, default 512).
> > E.g. with a configured cache size of 32 objects, getting 256 objects
> > would first fetch 32 + 256 = 288 objects into the cache,
> > and then move the 256 objects from the cache to the destination memory,
> > instead of fetching the 256 objects directly to the destination memory.
> > This had a performance cost.
> > However, this is unlikely to occur in real applications, so it is not
> > important in itself.
> > 
> > 3.
> > When putting objects into a mempool, and the mempool cache did not have
> > free space for so many objects,
> > the cache was flushed completely, and the new objects were then put
> > into
> > the cache.
> > I.e. the cache drain level was zero.
> > This (complete cache flush) meant that a subsequent get operation (with
> > the same number of objects) completely emptied the cache,
> > so another subsequent get operation required replenishing the cache.
> > 
> > Similarly,
> > When getting objects from a mempool, and the mempool cache did not hold
> > so
> > many objects,
> > the cache was replenished to cache->size + remaining objects,
> > and then (the remaining part of) the requested objects were fetched via
> > the cache,
> > which left the cache filled (to cache->size) at completion.
> > I.e. the cache refill level was cache->size (plus some, depending on
> > request size).
> > 
> > (1) was improved by generally comparing to cache->size instead of
> > cache->flushthresh, when considering the capacity of the cache.
> > The cache->flushthresh field is kept for API/ABI compatibility
> > purposes,
> > and initialized to cache->size instead of cache->size * 1.5.
> > 
> > (2) was improved by generally comparing to cache->size / 2 instead of
> > RTE_MEMPOOL_CACHE_MAX_SIZE, when checking the bounce buffer limit.
> > 
> > (3) was improved by flushing and replenishing the cache by half its
> > size,
> > so a flush/refill can be followed randomly by get or put requests.
> > This also reduced the number of objects in each flush/refill operation.
> > 
> > As a consequence of these changes, the size of the array holding the
> > objects in the cache (cache->objs[]) no longer needs to be
> > 2 * RTE_MEMPOOL_CACHE_MAX_SIZE, and can be reduced to
> > RTE_MEMPOOL_CACHE_MAX_SIZE at an API/ABI breaking release.

I'm not sure why waiting?


> > Performance data:
> > With a real WAN Optimization application, where the number of allocated
> > packets varies (as they are held in e.g. shaper queues), the mempool
> > cache miss rate dropped from ca. 1/20 objects to ca. 1/48 objects.
> > This was deployed in production at an ISP, and using an effective cache
> > size of 384 objects.
> > 
> > Bugzilla ID: 1027
> > Fixes: ea5dd2744b90 ("mempool: cache optimisations")
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> 
> Forgot carrying an Ack over from v5:
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> > ---
> > Depends-on: patch-163181 ("net/intel: do not bypass mbuf lib for mbuf
> > fast-free")
> 
> This dependency seems to cause CI apply failures.
> The dependency is based on an older snapshot of main,
> and this patch is based on a new snapshot of main.

The dependency should be resolved now.
Please could you send a v7?



^ permalink raw reply

* Re: [PATCH v11] eal/x86: optimize memcpy of small sizes
From: Thomas Monjalon @ 2026-06-01 13:38 UTC (permalink / raw)
  To: Morten Brørup
  Cc: dev, Bruce Richardson, Konstantin Ananyev, Vipin Varghese,
	Liangxing Wang, Thiyagarajan P, Bala Murali Krishna,
	Anatoly Burakov, Vladimir Medvedkin, Konstantin Ananyev,
	Stephen Hemminger
In-Reply-To: <20260521154214.1c171a74@phoenix.local>

22/05/2026 00:42, Stephen Hemminger:
> On Thu, 21 May 2026 18:56:31 +0000
> Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> > The implementation for copying up to 64 bytes does not depend on address
> > alignment with the size of the CPU's vector registers. Nonetheless, the
> > exact same code for copying up to 64 bytes was present in both the aligned
> > copy function and all the CPU vector register size specific variants of
> > the unaligned copy functions.
> > With this patch, the implementation for copying up to 64 bytes was
> > consolidated into one instance, located in the common copy function,
> > before checking alignment requirements.
> > This provides three benefits:
> > 1. No copy-paste in the source code.
> > 2. A performance gain for copying up to 64 bytes, because the
> > address alignment check is avoided in this case.
> > 3. Reduced instruction memory footprint, because the compiler only
> > generates one instance of the function for copying up to 64 bytes, instead
> > of two instances (one in the unaligned copy function, and one in the
> > aligned copy function).
> > 
> > Furthermore, __rte_restrict was added to source and destination addresses.
> > 
> > Also, the missing implementation of rte_mov48() was added.
> > 
> > Until recently, some drivers required disabling stringop-overflow warnings
> > when using rte_memcpy().
> > For some strange reason, these warnings were disabled in the rte_memcpy
> > header file, instead of in the problematic drivers.
> > With series-38174 ("remove use of rte_memcpy from net/intel"), the
> > problematic drivers were updated to use memcpy() instead of rte_memcpy(),
> > so disabling these warnings is no longer required, and was removed.
> > 
> > Regarding performance...
> > The memcpy performance test (cache-to-cache copy) shows:
> > Copying up to 15 bytes takes ca. 4.5 cycles, versus ca. 6.5 cycles before.
> > Copying 8 bytes takes 4 cycles, versus 7 cycles before.
> > Copying 16 bytes takes 2 cycles, versus 4 cycles before.
> > Copying 64 bytes takes 4 cycles, versus 7 cycles before.
> > 
> > Depends-on: series-38174 ("remove use of rte_memcpy from net/intel")
> > 
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> 
> Here is the full wordy all providers reviews.
[...]
> Summary across 4 provider(s): clean=0 warnings=1 errors=3 failed=0

What is the followup?
Do we target DPDK 26.07?



^ permalink raw reply

* [PATCH] devtools: add Vertex AI to review scripts
From: David Marchand @ 2026-06-01 13:24 UTC (permalink / raw)
  To: dev; +Cc: thomas, Stephen Hemminger, Aaron Conole

Add support for Google Vertex AI authentication as an alternative to
direct API key authentication. All four providers (Anthropic, Google,
OpenAI, xAI) can now use Vertex AI with Application Default Credentials.

This requires a python dependency google-auth but it is left as
optional.

Key features:
- Auto-detection of authentication method based on environment
- Manual override via --auth flag (auto, direct, vertex)
- Automatic model name translation for Vertex format
- Support for both global and regional Vertex endpoints
- Proper error handling for Vertex API responses

Provider-specific implementations:
- Anthropic: Uses /publishers/anthropic/models/{model}:rawPredict
  with model name format claude-sonnet-4-5@20250929
- Google: Uses /publishers/google/models/{model}:generateContent
- OpenAI/xAI: Use /endpoints/openapi/chat/completions
  with publisher prefix (e.g., openai/gpt-oss-120b-maas)

Authentication detection logic:
- Vertex: Requires google-auth library and ADC configured
- Direct: Falls back to API key from environment variables

Available models on Vertex AI:
- Anthropic: All Claude models
- Google: All Gemini models
- OpenAI: gpt-oss-120b-maas, gpt-oss-20b-maas (open-weight only)
- xAI: grok-4.20-*, grok-4.1-fast-* variants

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Note: I only tested Vertex work.
I have no API key to double check the "direct" method is still working.

---
 devtools/ai/_common.py      | 166 ++++++++++++++++++++++++++++++++----
 devtools/ai/review-doc.py   |  35 ++++++--
 devtools/ai/review-patch.py |  39 ++++++---
 3 files changed, 205 insertions(+), 35 deletions(-)

diff --git a/devtools/ai/_common.py b/devtools/ai/_common.py
index 69982cbda5..0c1257842f 100644
--- a/devtools/ai/_common.py
+++ b/devtools/ai/_common.py
@@ -6,6 +6,7 @@
 
 import argparse
 import json
+import os
 import subprocess
 import sys
 from dataclasses import dataclass
@@ -13,6 +14,14 @@
 from urllib.error import HTTPError, URLError
 from urllib.request import Request, urlopen
 
+# Optional dependency for Vertex AI
+try:
+    from google.auth import default as google_auth_default
+    from google.auth.transport.requests import Request as GoogleAuthRequest
+    VERTEX_AI_AVAILABLE = True
+except ImportError:
+    VERTEX_AI_AVAILABLE = False
+
 # Provider configurations (model defaults; override with --model).
 PROVIDERS: dict[str, dict[str, str]] = {
     "anthropic": {
@@ -128,25 +137,137 @@ def print_token_summary(
     print(format_token_summary(usage, provider, model), file=sys.stderr)
 
 
+def get_vertex_credentials() -> tuple[str, str]:
+    """Get Google Cloud access token and project for Vertex AI.
+
+    Uses Application Default Credentials (ADC).
+    Requires: gcloud auth application-default login
+
+    Returns: (access_token, project_id)
+    """
+    credentials, project = google_auth_default()
+
+    # Refresh credentials to get access token
+    auth_request = GoogleAuthRequest()
+    credentials.refresh(auth_request)
+
+    if not project:
+        error("Could not detect GCP project. Set GOOGLE_CLOUD_PROJECT environment variable or run: gcloud config set project PROJECT_ID")
+
+    return credentials.token, project
+
+
+def model_to_vertex(model: str, provider: str) -> str:
+    """Convert model name to Vertex AI format.
+
+    Anthropic models use @ for version dates:
+    - API format: claude-sonnet-4-5-20250929
+    - Vertex format: claude-sonnet-4-5@20250929
+
+    OpenAI/xAI models need publisher prefix:
+    - Vertex requires: openai/gpt-oss-120b-maas
+
+    Other providers use the same format for both.
+    """
+    if provider == "anthropic":
+        # Match pattern: ends with -YYYYMMDD (8 digits)
+        if model.count('-') >= 3:
+            parts = model.rsplit('-', 1)
+            if len(parts) == 2 and len(parts[1]) == 8 and parts[1].isdigit():
+                return f"{parts[0]}@{parts[1]}"
+    elif provider in ("openai", "xai"):
+        # Add publisher prefix if not already present
+        if "/" not in model:
+            return f"{provider}/{model}"
+    return model
+
+
+def detect_auth_method(provider: str) -> str:
+    """Detect authentication method for a provider.
+
+    Args:
+        provider: The provider name (e.g., "anthropic", "openai")
+
+    Returns:
+        "direct" or "vertex"
+    """
+    env_var = PROVIDERS[provider]["env_var"]
+    if os.environ.get(env_var):
+        return "direct"
+    if VERTEX_AI_AVAILABLE:
+        try:
+            credentials, project = google_auth_default()
+            if credentials and project:
+                return "vertex"
+        except Exception:
+            pass
+    return "direct"
+
+
 def _build_request_meta(
-    provider: str, api_key: str, model: str
-) -> tuple[str, dict[str, str]]:
-    """Return (url, headers) for a provider request."""
+    provider: str, auth: str, model: str, request_data: dict[str, Any]
+) -> tuple[str, dict[str, str], dict[str, Any]]:
+    """Return (url, headers, request_data) for a provider request.
+
+    Args:
+        provider: Provider name
+        auth: Authentication string - either "direct:<api_key>" or "vertex"
+        model: Model identifier
+        request_data: The request payload (may be modified for Vertex)
+
+    Returns:
+        Tuple of (url, headers, modified_request_data)
+    """
     config = PROVIDERS[provider]
-    if provider == "anthropic":
+
+    if auth.startswith("direct:"):
+        api_key = auth[7:]
+        if provider == "anthropic":
+            request_data["model"] = model
+            return config["endpoint"], {
+                "Content-Type": "application/json",
+                "x-api-key": api_key,
+                "anthropic-version": "2023-06-01",
+            }, request_data
+        if provider == "google":
+            url = f"{config['endpoint']}/{model}:generateContent?key={api_key}"
+            return url, {"Content-Type": "application/json"}, request_data
+        # openai, xai
+        request_data["model"] = model
         return config["endpoint"], {
             "Content-Type": "application/json",
-            "x-api-key": api_key,
-            "anthropic-version": "2023-06-01",
-        }
-    if provider == "google":
-        url = f"{config['endpoint']}/{model}:generateContent?key={api_key}"
-        return url, {"Content-Type": "application/json"}
-    # openai, xai
-    return config["endpoint"], {
+            "Authorization": f"Bearer {api_key}",
+        }, request_data
+
+    # Vertex AI authentication
+    if auth != "vertex":
+        error(f"Invalid auth format: {auth}")
+
+    access_token, project_id = get_vertex_credentials()
+    project_id = os.environ.get("GOOGLE_CLOUD_PROJECT") or os.environ.get("GCP_PROJECT") or project_id
+    location = os.environ.get("CLOUD_ML_REGION", "global")
+
+    if location == "global":
+        vertex_base = f"https://aiplatform.googleapis.com/v1/projects/{project_id}/locations/{location}"
+    else:
+        vertex_base = f"https://{location}-aiplatform.googleapis.com/v1/projects/{project_id}/locations/{location}"
+
+    headers = {
         "Content-Type": "application/json",
-        "Authorization": f"Bearer {api_key}",
+        "Authorization": f"Bearer {access_token}",
     }
+    vertex_model = model_to_vertex(model, provider)
+
+    if provider == "anthropic":
+        request_data["anthropic_version"] = "vertex-2023-10-16"
+        url = f"{vertex_base}/publishers/anthropic/models/{vertex_model}:rawPredict"
+    elif provider == "google":
+        url = f"{vertex_base}/publishers/google/models/{vertex_model}:generateContent"
+    else:  # openai, xai
+        request_data["model"] = vertex_model
+        url = f"{vertex_base}/endpoints/openapi/chat/completions"
+
+    return url, headers, request_data
 
 
 def _extract_usage(provider: str, result: dict[str, Any]) -> TokenUsage:
@@ -208,7 +329,7 @@ def _print_verbose_usage(usage: TokenUsage) -> None:
 
 def send_request(
     provider: str,
-    api_key: str,
+    auth: str,
     model: str,
     request_data: dict[str, Any],
     *,
@@ -220,8 +341,19 @@ def send_request(
     The caller assembles the provider-specific request body via its own
     build_*_request helpers (the prompts differ per script). This function
     handles transport, error reporting, and token-usage extraction.
+
+    Args:
+        provider: Provider name (anthropic, openai, xai, google)
+        auth: Authentication string - either "direct:<api_key>" or "vertex"
+        model: Model identifier
+        request_data: Provider-specific request payload
+        timeout: Request timeout in seconds
+        verbose: Show detailed token usage
+
+    Returns:
+        Tuple of (response_text, token_usage)
     """
-    url, headers = _build_request_meta(provider, api_key, model)
+    url, headers, request_data = _build_request_meta(provider, auth, model, request_data)
     body = json.dumps(request_data).encode("utf-8")
     req = Request(url, data=body, headers=headers)
 
@@ -232,6 +364,8 @@ def send_request(
         error_body = e.read().decode("utf-8")
         try:
             error_data = json.loads(error_body)
+            if isinstance(error_data, list) and error_data:
+                error_data = error_data[0]
             error(f"API error: {error_data.get('error', error_body)}")
         except json.JSONDecodeError:
             error(f"API error ({e.code}): {error_body}")
@@ -239,6 +373,8 @@ def send_request(
         if isinstance(e.reason, TimeoutError):
             error(f"Request timed out after {timeout} seconds")
         error(f"Connection error: {e.reason}")
+    except TimeoutError:
+        error(f"Request timed out after {timeout} seconds")
 
     usage = _extract_usage(provider, result)
     if verbose:
diff --git a/devtools/ai/review-doc.py b/devtools/ai/review-doc.py
index 24e70ae06b..ee02c7ee40 100755
--- a/devtools/ai/review-doc.py
+++ b/devtools/ai/review-doc.py
@@ -24,8 +24,10 @@
 
 from _common import (
     PROVIDERS,
+    VERTEX_AI_AVAILABLE,
     TokenUsage,
     add_token_args,
+    detect_auth_method,
     error,
     get_git_config,
     list_providers,
@@ -273,7 +275,6 @@ def build_anthropic_request(
         doc_file, commit_prefix, output_format, include_diff_markers
     )
     return {
-        "model": model,
         "max_tokens": max_tokens,
         "system": [
             {"type": "text", "text": SYSTEM_PROMPT},
@@ -307,7 +308,6 @@ def build_openai_request(
         doc_file, commit_prefix, output_format, include_diff_markers
     )
     return {
-        "model": model,
         "max_tokens": max_tokens,
         "messages": [
             {"role": "system", "content": SYSTEM_PROMPT},
@@ -352,7 +352,7 @@ def build_google_request(
 
 def call_api(
     provider: str,
-    api_key: str,
+    auth: str,
     model: str,
     max_tokens: int,
     agents_content: str,
@@ -399,7 +399,7 @@ def call_api(
         )
     return send_request(
         provider,
-        api_key,
+        auth,
         model,
         request_data,
         timeout=timeout,
@@ -631,6 +631,12 @@ def main() -> None:
         help="Show API request details",
     )
     add_token_args(parser)
+    parser.add_argument(
+        "--auth",
+        choices=["auto", "direct", "vertex"],
+        default="auto",
+        help="Authentication method: auto (default), direct (API key), vertex (Google Cloud)",
+    )
     parser.add_argument(
         "-q",
         "--quiet",
@@ -709,10 +715,20 @@ def main() -> None:
     config = PROVIDERS[args.provider]
     model = args.model or config["default_model"]
 
-    # Get API key
-    api_key = os.environ.get(config["env_var"])
-    if not api_key:
-        error(f"{config['env_var']} environment variable not set")
+    if args.auth == "auto":
+        auth_method = detect_auth_method(args.provider)
+    else:
+        auth_method = args.auth
+
+    if auth_method == "vertex":
+        if not VERTEX_AI_AVAILABLE:
+            error("Vertex AI support requires 'google-auth' library. Install with: pip install google-auth")
+        auth = "vertex"
+    else:
+        api_key = os.environ.get(config["env_var"])
+        if not api_key:
+            error(f"{config['env_var']} environment variable not set")
+        auth = f"direct:{api_key}"
 
     # Validate files
     agents_path = Path(args.agents)
@@ -783,6 +799,7 @@ def main() -> None:
         if args.verbose:
             print("=== Request ===", file=sys.stderr)
             print(f"Provider: {args.provider}", file=sys.stderr)
+            print(f"Auth method: {auth_method}", file=sys.stderr)
             print(f"Model: {model}", file=sys.stderr)
             print(f"Output format: {args.output_format}", file=sys.stderr)
             print(f"AGENTS file: {args.agents}", file=sys.stderr)
@@ -800,7 +817,7 @@ def main() -> None:
         # Call API
         review_text, call_usage = call_api(
             args.provider,
-            api_key,
+            auth,
             model,
             args.tokens,
             agents_content,
diff --git a/devtools/ai/review-patch.py b/devtools/ai/review-patch.py
index 52601ac156..8f2ce85a12 100755
--- a/devtools/ai/review-patch.py
+++ b/devtools/ai/review-patch.py
@@ -22,8 +22,10 @@
 
 from _common import (
     PROVIDERS,
+    VERTEX_AI_AVAILABLE,
     TokenUsage,
     add_token_args,
+    detect_auth_method,
     error,
     get_git_config,
     list_providers,
@@ -474,7 +476,6 @@ def build_anthropic_request(
         patch_name=patch_name, format_instruction=format_instruction
     )
     return {
-        "model": model,
         "max_tokens": max_tokens,
         "system": [
             {"type": "text", "text": system_prompt},
@@ -508,7 +509,6 @@ def build_openai_request(
         patch_name=patch_name, format_instruction=format_instruction
     )
     return {
-        "model": model,
         "max_tokens": max_tokens,
         "messages": [
             {"role": "system", "content": system_prompt},
@@ -553,7 +553,7 @@ def build_google_request(
 
 def call_api(
     provider: str,
-    api_key: str,
+    auth: str,
     model: str,
     max_tokens: int,
     system_prompt: str,
@@ -596,7 +596,7 @@ def call_api(
         )
     return send_request(
         provider,
-        api_key,
+        auth,
         model,
         request_data,
         timeout=timeout,
@@ -813,6 +813,12 @@ def main() -> None:
         help="Show API request details",
     )
     add_token_args(parser)
+    parser.add_argument(
+        "--auth",
+        choices=["auto", "direct", "vertex"],
+        default="auto",
+        help="Authentication method: auto (default), direct (API key), vertex (Google Cloud)",
+    )
     parser.add_argument(
         "-f",
         "--format",
@@ -930,10 +936,20 @@ def main() -> None:
     config = PROVIDERS[args.provider]
     model = args.model or config["default_model"]
 
-    # Get API key
-    api_key = os.environ.get(config["env_var"])
-    if not api_key:
-        error(f"{config['env_var']} environment variable not set")
+    if args.auth == "auto":
+        auth_method = detect_auth_method(args.provider)
+    else:
+        auth_method = args.auth
+
+    if auth_method == "vertex":
+        if not VERTEX_AI_AVAILABLE:
+            error("Vertex AI support requires 'google-auth' library. Install with: pip install google-auth")
+        auth = "vertex"
+    else:
+        api_key = os.environ.get(config["env_var"])
+        if not api_key:
+            error(f"{config['env_var']} environment variable not set")
+        auth = f"direct:{api_key}"
 
     # Validate files
     agents_path = Path(args.agents)
@@ -1041,7 +1057,7 @@ def main() -> None:
 
                 review_text, call_usage = call_api(
                     args.provider,
-                    api_key,
+                    auth,
                     model,
                     args.tokens,
                     system_prompt,
@@ -1111,7 +1127,7 @@ def main() -> None:
 
                 review_text, call_usage = call_api(
                     args.provider,
-                    api_key,
+                    auth,
                     model,
                     args.tokens,
                     system_prompt,
@@ -1136,6 +1152,7 @@ def main() -> None:
     if args.verbose:
         print("=== Request ===", file=sys.stderr)
         print(f"Provider: {args.provider}", file=sys.stderr)
+        print(f"Auth method: {auth_method}", file=sys.stderr)
         print(f"Model: {model}", file=sys.stderr)
         print(f"Review date: {review_date}", file=sys.stderr)
         if args.release:
@@ -1164,7 +1181,7 @@ def main() -> None:
     if estimated_tokens > 0:  # Not already processed
         review_text, call_usage = call_api(
             args.provider,
-            api_key,
+            auth,
             model,
             args.tokens,
             system_prompt,
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v2] ring: add cache guard after ring elements table
From: Thomas Monjalon @ 2026-06-01 13:49 UTC (permalink / raw)
  To: Morten Brørup
  Cc: dev, Konstantin Ananyev, Wathsala Vithanage, Konstantin Ananyev
In-Reply-To: <20260505161329.258182-1-mb@smartsharesystems.com>

05/05/2026 18:13, Morten Brørup:
> Added cache guard after the table holding the ring elements, to avoid
> false sharing conflicts caused by next-line hardware prefetchers when
> accessing elements at the end of the ring table.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Wathsala Vithanage <wathsala.vithanage@arm.com>

Applied, thanks.



^ permalink raw reply

* RE: [PATCH v6] mempool: improve cache behaviour and performance
From: Morten Brørup @ 2026-06-01 13:51 UTC (permalink / raw)
  To: Thomas Monjalon, Bruce Richardson
  Cc: dev, Andrew Rybchenko, Jingjing Wu, Praveen Shetty,
	Hemant Agrawal, Sachin Saxena
In-Reply-To: <0b8SMRhASVKQeU9e_kqJHQ@monjalon.net>

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, 1 June 2026 15.36
> 
> 26/05/2026 18:00, Morten Brørup:
> > > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > > Sent: Tuesday, 26 May 2026 16.00
> > >
> > > This patch refactors the mempool cache to eliminate some unexpected
> > > behaviour and reduce the mempool cache miss rate.
> > >
> > > 1.
> > > The actual cache size was 1.5 times the cache size specified at
> run-
> > > time
> > > mempool creation.
> > > This was obviously not expected by application developers.
> > >
> > > 2.
> > > In get operations, the check for when to use the cache as bounce
> buffer
> > > did not respect the run-time configured cache size,
> > > but compared to the build time maximum possible cache size
> > > (RTE_MEMPOOL_CACHE_MAX_SIZE, default 512).
> > > E.g. with a configured cache size of 32 objects, getting 256
> objects
> > > would first fetch 32 + 256 = 288 objects into the cache,
> > > and then move the 256 objects from the cache to the destination
> memory,
> > > instead of fetching the 256 objects directly to the destination
> memory.
> > > This had a performance cost.
> > > However, this is unlikely to occur in real applications, so it is
> not
> > > important in itself.
> > >
> > > 3.
> > > When putting objects into a mempool, and the mempool cache did not
> have
> > > free space for so many objects,
> > > the cache was flushed completely, and the new objects were then put
> > > into
> > > the cache.
> > > I.e. the cache drain level was zero.
> > > This (complete cache flush) meant that a subsequent get operation
> (with
> > > the same number of objects) completely emptied the cache,
> > > so another subsequent get operation required replenishing the
> cache.
> > >
> > > Similarly,
> > > When getting objects from a mempool, and the mempool cache did not
> hold
> > > so
> > > many objects,
> > > the cache was replenished to cache->size + remaining objects,
> > > and then (the remaining part of) the requested objects were fetched
> via
> > > the cache,
> > > which left the cache filled (to cache->size) at completion.
> > > I.e. the cache refill level was cache->size (plus some, depending
> on
> > > request size).
> > >
> > > (1) was improved by generally comparing to cache->size instead of
> > > cache->flushthresh, when considering the capacity of the cache.
> > > The cache->flushthresh field is kept for API/ABI compatibility
> > > purposes,
> > > and initialized to cache->size instead of cache->size * 1.5.
> > >
> > > (2) was improved by generally comparing to cache->size / 2 instead
> of
> > > RTE_MEMPOOL_CACHE_MAX_SIZE, when checking the bounce buffer limit.
> > >
> > > (3) was improved by flushing and replenishing the cache by half its
> > > size,
> > > so a flush/refill can be followed randomly by get or put requests.
> > > This also reduced the number of objects in each flush/refill
> operation.
> > >
> > > As a consequence of these changes, the size of the array holding
> the
> > > objects in the cache (cache->objs[]) no longer needs to be
> > > 2 * RTE_MEMPOOL_CACHE_MAX_SIZE, and can be reduced to
> > > RTE_MEMPOOL_CACHE_MAX_SIZE at an API/ABI breaking release.
> 
> I'm not sure why waiting?

Because the rte_mempool_cache structure holding the array is part of the public API:
https://elixir.bootlin.com/dpdk/v26.03/source/lib/mempool/rte_mempool.h#L113

abidiff complained about it in v1, so I reverted the array size reduction in v2.

> 
> 
> > > Performance data:
> > > With a real WAN Optimization application, where the number of
> allocated
> > > packets varies (as they are held in e.g. shaper queues), the
> mempool
> > > cache miss rate dropped from ca. 1/20 objects to ca. 1/48 objects.
> > > This was deployed in production at an ISP, and using an effective
> cache
> > > size of 384 objects.
> > >
> > > Bugzilla ID: 1027
> > > Fixes: ea5dd2744b90 ("mempool: cache optimisations")
> > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> >
> > Forgot carrying an Ack over from v5:
> > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >
> > > ---
> > > Depends-on: patch-163181 ("net/intel: do not bypass mbuf lib for
> mbuf
> > > fast-free")
> >
> > This dependency seems to cause CI apply failures.
> > The dependency is based on an older snapshot of main,
> > and this patch is based on a new snapshot of main.
> 
> The dependency should be resolved now.
> Please could you send a v7?
> 

I'm not 100 % sure, but I think the problem is CI only...

This patch is based on main, so it should apply as-is.

Bruce has already applied the other patch (that this one depends on) to the intel-next-net tree.
The other patch is based on an older snapshot of main, so when using "Depends-on", I guess the CI bases its series on the other patch; and then the CI fails to apply this patch (because it's based on a newer snapshot of main).


^ permalink raw reply

* RE: [PATCH v11] eal/x86: optimize memcpy of small sizes
From: Morten Brørup @ 2026-06-01 14:19 UTC (permalink / raw)
  To: Thomas Monjalon, Stephen Hemminger, Bruce Richardson,
	Konstantin Ananyev
  Cc: dev, Konstantin Ananyev, Vipin Varghese, Liangxing Wang,
	Thiyagarajan P, Bala Murali Krishna, Anatoly Burakov,
	Vladimir Medvedkin
In-Reply-To: <JAYuDo5qT4OWJH9lfTRFKA@monjalon.net>

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, 1 June 2026 15.38
> 
> 22/05/2026 00:42, Stephen Hemminger:
> > On Thu, 21 May 2026 18:56:31 +0000
> > Morten Brørup <mb@smartsharesystems.com> wrote:
> >
> > > The implementation for copying up to 64 bytes does not depend on
> address
> > > alignment with the size of the CPU's vector registers. Nonetheless,
> the
> > > exact same code for copying up to 64 bytes was present in both the
> aligned
> > > copy function and all the CPU vector register size specific
> variants of
> > > the unaligned copy functions.
> > > With this patch, the implementation for copying up to 64 bytes was
> > > consolidated into one instance, located in the common copy
> function,
> > > before checking alignment requirements.
> > > This provides three benefits:
> > > 1. No copy-paste in the source code.
> > > 2. A performance gain for copying up to 64 bytes, because the
> > > address alignment check is avoided in this case.
> > > 3. Reduced instruction memory footprint, because the compiler only
> > > generates one instance of the function for copying up to 64 bytes,
> instead
> > > of two instances (one in the unaligned copy function, and one in
> the
> > > aligned copy function).
> > >
> > > Furthermore, __rte_restrict was added to source and destination
> addresses.
> > >
> > > Also, the missing implementation of rte_mov48() was added.
> > >
> > > Until recently, some drivers required disabling stringop-overflow
> warnings
> > > when using rte_memcpy().
> > > For some strange reason, these warnings were disabled in the
> rte_memcpy
> > > header file, instead of in the problematic drivers.
> > > With series-38174 ("remove use of rte_memcpy from net/intel"), the
> > > problematic drivers were updated to use memcpy() instead of
> rte_memcpy(),
> > > so disabling these warnings is no longer required, and was removed.
> > >
> > > Regarding performance...
> > > The memcpy performance test (cache-to-cache copy) shows:
> > > Copying up to 15 bytes takes ca. 4.5 cycles, versus ca. 6.5 cycles
> before.
> > > Copying 8 bytes takes 4 cycles, versus 7 cycles before.
> > > Copying 16 bytes takes 2 cycles, versus 4 cycles before.
> > > Copying 64 bytes takes 4 cycles, versus 7 cycles before.
> > >
> > > Depends-on: series-38174 ("remove use of rte_memcpy from
> net/intel")
> > >
> > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > > Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> >
> > Here is the full wordy all providers reviews.
> [...]
> > Summary across 4 provider(s): clean=0 warnings=1 errors=3 failed=0
> 
> What is the followup?

AI wants me to fix existing code.
I had chosen to stick to the file's existing coding style etc., including some unnecessary type casts.
So AI also complains about my code doing things the same way existing code in the file does it.
Fixing existing code is out of scope for this patch. And using a different style for my changes would be confusing.

The patch description mentions that stringop-overflow warnings are no longer disabled for rte_mempcy().
AI wants this to go into the release notes (although it is x86 architecture only).
But IMO, this is far below the threshold for what should go into the release notes.

> Do we target DPDK 26.07?

IMO, yes, this v11 patch is good.


^ permalink raw reply

* Re: [PATCH v6] mempool: improve cache behaviour and performance
From: Thomas Monjalon @ 2026-06-01 14:19 UTC (permalink / raw)
  To: Morten Brørup
  Cc: Bruce Richardson, dev, Andrew Rybchenko, Jingjing Wu,
	Praveen Shetty, Hemant Agrawal, Sachin Saxena
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35F658CC@smartserver.smartshare.dk>

01/06/2026 15:51, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Monday, 1 June 2026 15.36
> > 
> > 26/05/2026 18:00, Morten Brørup:
> > > > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > > > Sent: Tuesday, 26 May 2026 16.00
> > > >
> > > > This patch refactors the mempool cache to eliminate some unexpected
> > > > behaviour and reduce the mempool cache miss rate.
> > > >
> > > > 1.
> > > > The actual cache size was 1.5 times the cache size specified at
> > run-
> > > > time
> > > > mempool creation.
> > > > This was obviously not expected by application developers.
> > > >
> > > > 2.
> > > > In get operations, the check for when to use the cache as bounce
> > buffer
> > > > did not respect the run-time configured cache size,
> > > > but compared to the build time maximum possible cache size
> > > > (RTE_MEMPOOL_CACHE_MAX_SIZE, default 512).
> > > > E.g. with a configured cache size of 32 objects, getting 256
> > objects
> > > > would first fetch 32 + 256 = 288 objects into the cache,
> > > > and then move the 256 objects from the cache to the destination
> > memory,
> > > > instead of fetching the 256 objects directly to the destination
> > memory.
> > > > This had a performance cost.
> > > > However, this is unlikely to occur in real applications, so it is
> > not
> > > > important in itself.
> > > >
> > > > 3.
> > > > When putting objects into a mempool, and the mempool cache did not
> > have
> > > > free space for so many objects,
> > > > the cache was flushed completely, and the new objects were then put
> > > > into
> > > > the cache.
> > > > I.e. the cache drain level was zero.
> > > > This (complete cache flush) meant that a subsequent get operation
> > (with
> > > > the same number of objects) completely emptied the cache,
> > > > so another subsequent get operation required replenishing the
> > cache.
> > > >
> > > > Similarly,
> > > > When getting objects from a mempool, and the mempool cache did not
> > hold
> > > > so
> > > > many objects,
> > > > the cache was replenished to cache->size + remaining objects,
> > > > and then (the remaining part of) the requested objects were fetched
> > via
> > > > the cache,
> > > > which left the cache filled (to cache->size) at completion.
> > > > I.e. the cache refill level was cache->size (plus some, depending
> > on
> > > > request size).
> > > >
> > > > (1) was improved by generally comparing to cache->size instead of
> > > > cache->flushthresh, when considering the capacity of the cache.
> > > > The cache->flushthresh field is kept for API/ABI compatibility
> > > > purposes,
> > > > and initialized to cache->size instead of cache->size * 1.5.
> > > >
> > > > (2) was improved by generally comparing to cache->size / 2 instead
> > of
> > > > RTE_MEMPOOL_CACHE_MAX_SIZE, when checking the bounce buffer limit.
> > > >
> > > > (3) was improved by flushing and replenishing the cache by half its
> > > > size,
> > > > so a flush/refill can be followed randomly by get or put requests.
> > > > This also reduced the number of objects in each flush/refill
> > operation.
> > > >
> > > > As a consequence of these changes, the size of the array holding
> > the
> > > > objects in the cache (cache->objs[]) no longer needs to be
> > > > 2 * RTE_MEMPOOL_CACHE_MAX_SIZE, and can be reduced to
> > > > RTE_MEMPOOL_CACHE_MAX_SIZE at an API/ABI breaking release.
> > 
> > I'm not sure why waiting?
> 
> Because the rte_mempool_cache structure holding the array is part of the public API:
> https://elixir.bootlin.com/dpdk/v26.03/source/lib/mempool/rte_mempool.h#L113
> 
> abidiff complained about it in v1, so I reverted the array size reduction in v2.
> 
> > 
> > 
> > > > Performance data:
> > > > With a real WAN Optimization application, where the number of
> > allocated
> > > > packets varies (as they are held in e.g. shaper queues), the
> > mempool
> > > > cache miss rate dropped from ca. 1/20 objects to ca. 1/48 objects.
> > > > This was deployed in production at an ISP, and using an effective
> > cache
> > > > size of 384 objects.
> > > >
> > > > Bugzilla ID: 1027
> > > > Fixes: ea5dd2744b90 ("mempool: cache optimisations")
> > > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > >
> > > Forgot carrying an Ack over from v5:
> > > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > >
> > > > ---
> > > > Depends-on: patch-163181 ("net/intel: do not bypass mbuf lib for
> > mbuf
> > > > fast-free")
> > >
> > > This dependency seems to cause CI apply failures.
> > > The dependency is based on an older snapshot of main,
> > > and this patch is based on a new snapshot of main.
> > 
> > The dependency should be resolved now.
> > Please could you send a v7?
> > 
> 
> I'm not 100 % sure, but I think the problem is CI only...
> 
> This patch is based on main, so it should apply as-is.
> 
> Bruce has already applied the other patch (that this one depends on) to the intel-next-net tree.
> The other patch is based on an older snapshot of main, so when using "Depends-on", I guess the CI bases its series on the other patch; and then the CI fails to apply this patch (because it's based on a newer snapshot of main).

The patch is in main since last week.

So you could send a new version with the ack and it will run in CI.



^ permalink raw reply

* Re: [PATCH] devtools: add Vertex AI to review scripts
From: Thomas Monjalon @ 2026-06-01 14:21 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Stephen Hemminger, Aaron Conole
In-Reply-To: <20260601132402.1125588-1-david.marchand@redhat.com>

01/06/2026 15:24, David Marchand:
> +    if args.auth == "auto":
> +        auth_method = detect_auth_method(args.provider)
> +    else:
> +        auth_method = args.auth
> +
> +    if auth_method == "vertex":
> +        if not VERTEX_AI_AVAILABLE:
> +            error("Vertex AI support requires 'google-auth' library. Install with: pip install google-auth")
> +        auth = "vertex"
> +    else:
> +        api_key = os.environ.get(config["env_var"])
> +        if not api_key:
> +            error(f"{config['env_var']} environment variable not set")
> +        auth = f"direct:{api_key}"

Could we have such code in the common file?




^ permalink raw reply

* RE: [PATCH v6] mempool: improve cache behaviour and performance
From: Morten Brørup @ 2026-06-01 14:27 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Bruce Richardson, dev
In-Reply-To: <6w4sxDwSRYyFMvKExI5ztQ@monjalon.net>

> > > > > Depends-on: patch-163181 ("net/intel: do not bypass mbuf lib
> for
> > > mbuf
> > > > > fast-free")
> > > >
> > > > This dependency seems to cause CI apply failures.
> > > > The dependency is based on an older snapshot of main,
> > > > and this patch is based on a new snapshot of main.
> > >
> > > The dependency should be resolved now.
> > > Please could you send a v7?
> > >
> >
> > I'm not 100 % sure, but I think the problem is CI only...
> >
> > This patch is based on main, so it should apply as-is.
> >
> > Bruce has already applied the other patch (that this one depends on)
> to the intel-next-net tree.
> > The other patch is based on an older snapshot of main, so when using
> "Depends-on", I guess the CI bases its series on the other patch; and
> then the CI fails to apply this patch (because it's based on a newer
> snapshot of main).
> 
> The patch is in main since last week.
> 
> So you could send a new version with the ack and it will run in CI.
> 

OK. Will do.


^ permalink raw reply

* Re: [PATCH] net: fix GTP Tunnel parse out-of-bounds read
From: Andrew Rybchenko @ 2026-06-01 14:27 UTC (permalink / raw)
  To: Thomas Monjalon, dev, Bruce Richardson, Jerin Jacob,
	Hemant Agrawal
  Cc: Stephen Hemminger, stable, Jie Hai
In-Reply-To: <-yUhN_HpTAq8QmssYJx54A@monjalon.net>

On 6/1/26 3:59 PM, Thomas Monjalon wrote:
> Any comment about this fix?
> 
> 
> 09/04/2026 18:15, Stephen Hemminger:
>> If packet is fragmented across multiple mbufs or the packet
>> has only GTP header the code would reference outside
>> the incoming mbuf.
>>
>> Send GTP packet:
>> - Valid GTP header (8 bytes)
>> - msg_type = 0xff
>> - e=1, s=1, pn=1 (sets gtp_len = 12)
>> - Total packet size = 10 bytes
>>
>> Read at gh + 12 accesses 2 bytes beyond packet end.
>>
>> The fix is to use rte_pktmbuf_read in a manner similar
>> to the read of the GTP header.
>>
>> Fixes: 64ed7f854cf4 ("net: add tunnel packet type parsing")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

The overall idea LGTM. However one nit in the code.

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

>> ---
>>   lib/net/rte_net.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
>> index 458b4814a9..da4018437b 100644
>> --- a/lib/net/rte_net.c
>> +++ b/lib/net/rte_net.c
>> @@ -219,8 +219,7 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
>>   	case RTE_GTPU_UDP_PORT: {
>>   		const struct rte_gtp_hdr *gh;
>>   		struct rte_gtp_hdr gh_copy;
>> -		uint8_t gtp_len;
>> -		uint8_t ip_ver;
>> +		uint32_t gtp_len;
>>   		gh = rte_pktmbuf_read(m, *off, sizeof(*gh), &gh_copy);
>>   		if (unlikely(gh == NULL))
>>   			return 0;
>> @@ -231,9 +230,16 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
>>   		 * Check message type. If message type is 0xff, it is
>>   		 * a GTP data packet. If not, it is a GTP control packet
>>   		 */
>> +		*off += gtp_len;
>>   		if (gh->msg_type == 0xff) {
>> -			ip_ver = *(const uint8_t *)((const char *)gh + gtp_len);
>> -			ip_ver = (ip_ver) & 0xf0;
>> +			const uint8_t *l3_hdr;

l3_hdr is confusing here. It sounds like a complete layer 3 header, but
it is just one first byte. May be it would be useful to highlight that
it is an inner Layer 3 header.

E.g. inner_l3_hdr_byte (I'm not fully happy with the name too, but
I hope idea is clear)

>> +			uint8_t l3_copy, ip_ver;
>> +
>> +			l3_hdr = rte_pktmbuf_read(m, *off, sizeof(*l3_hdr), &l3_copy);

sizeof(l3_copy) would be safer here.

>> +			if (unlikely(l3_hdr == NULL))
>> +				return 0;
>> +
>> +			ip_ver = *l3_hdr & 0xf0;
>>   			if (ip_ver == RTE_GTP_TYPE_IPV4)
>>   				*proto = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
>>   			else if (ip_ver == RTE_GTP_TYPE_IPV6)
>> @@ -243,7 +249,6 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
>>   		} else {
>>   			*proto = 0;
>>   		}
>> -		*off += gtp_len;
>>   		hdr_lens->inner_l2_len = gtp_len + sizeof(struct rte_udp_hdr);
>>   		hdr_lens->tunnel_len = gtp_len;
>>   		if (port_no == RTE_GTPC_UDP_PORT)
>>
> 
> 
> 
> 
> 


^ permalink raw reply

* Re: [PATCH] devtools: add Vertex AI to review scripts
From: David Marchand @ 2026-06-01 14:39 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Stephen Hemminger, Aaron Conole
In-Reply-To: <bUYoD1-ARAaefqQHkOtV4Q@monjalon.net>

On Mon, 1 Jun 2026 at 16:21, Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 01/06/2026 15:24, David Marchand:
> > +    if args.auth == "auto":
> > +        auth_method = detect_auth_method(args.provider)
> > +    else:
> > +        auth_method = args.auth
> > +
> > +    if auth_method == "vertex":
> > +        if not VERTEX_AI_AVAILABLE:
> > +            error("Vertex AI support requires 'google-auth' library. Install with: pip install google-auth")
> > +        auth = "vertex"
> > +    else:
> > +        api_key = os.environ.get(config["env_var"])
> > +        if not api_key:
> > +            error(f"{config['env_var']} environment variable not set")
> > +        auth = f"direct:{api_key}"
>
> Could we have such code in the common file?

Indeed, I'll wait a bit for more comments before sending a v2.


-- 
David Marchand


^ permalink raw reply

* Re: [PATCH] devtools: add Vertex AI to review scripts
From: Stephen Hemminger @ 2026-06-01 15:11 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: David Marchand, dev, Aaron Conole
In-Reply-To: <bUYoD1-ARAaefqQHkOtV4Q@monjalon.net>

On Mon, 01 Jun 2026 16:21:25 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 01/06/2026 15:24, David Marchand:
> > +    if args.auth == "auto":
> > +        auth_method = detect_auth_method(args.provider)
> > +    else:
> > +        auth_method = args.auth
> > +
> > +    if auth_method == "vertex":
> > +        if not VERTEX_AI_AVAILABLE:
> > +            error("Vertex AI support requires 'google-auth' library. Install with: pip install google-auth")
> > +        auth = "vertex"
> > +    else:
> > +        api_key = os.environ.get(config["env_var"])
> > +        if not api_key:
> > +            error(f"{config['env_var']} environment variable not set")
> > +        auth = f"direct:{api_key}"  
> 
> Could we have such code in the common file?

Yes please add to devtools/ai/_common.py used by both review-doc and review-patch



^ permalink raw reply

* Re: [PATCH] ethdev: promote experimental API's to stable
From: Stephen Hemminger @ 2026-06-01 15:26 UTC (permalink / raw)
  To: David Marchand; +Cc: Andrew Rybchenko, dev, Thomas Monjalon
In-Reply-To: <CAJFAV8ztviupv0rcjvL9MkBQSzY+yfak0RK8GzNbe-8s5JiyEw@mail.gmail.com>

On Mon, 1 Jun 2026 13:55:13 +0200
David Marchand <david.marchand@redhat.com> wrote:

> On Wed, 27 May 2026 at 16:44, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > +  * ``rte_eth_macaddrs_get``  
> 
> I am not enthousiastic on marking this stable.
> It more or less sets in stone that the mac addresses are stored in an array.

So either we kill it or make it stable, having experimental API for so long
seems like a todo list that never gets done.

^ permalink raw reply

* Re: [PATCH] ethdev: promote experimental API's to stable
From: Stephen Hemminger @ 2026-06-01 15:30 UTC (permalink / raw)
  To: David Marchand; +Cc: Andrew Rybchenko, dev, Thomas Monjalon
In-Reply-To: <CAJFAV8ztviupv0rcjvL9MkBQSzY+yfak0RK8GzNbe-8s5JiyEw@mail.gmail.com>

On Mon, 1 Jun 2026 13:55:13 +0200
David Marchand <david.marchand@redhat.com> wrote:

> On Wed, 27 May 2026 at 16:44, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > +  * ``rte_eth_macaddrs_get``  
> 
> I am not enthousiastic on marking this stable.
> It more or less sets in stone that the mac addresses are stored in an array.

The device could store the mac addresses in any form.
Your right that right now the implementation of the API depends on dev->data->mac_addrs.
But if that changed, having API to walk the macs would be good.

Maybe rte_eth_macaddrs_get should scrub out empty zero slots now?

^ permalink raw reply

* Re: [PATCH v3 00/20]net/sxe2: added Linkdata sxe2 ethernet driver
From: Stephen Hemminger @ 2026-06-01 15:40 UTC (permalink / raw)
  To: liujie5; +Cc: dev
In-Reply-To: <20260601084950.269887-1-liujie5@linkdatatechnology.com>

On Mon,  1 Jun 2026 16:49:30 +0800
liujie5@linkdatatechnology.com wrote:

> From: Jie Liu <liujie5@linkdatatechnology.com>
> 
> This patch set implements core functionality for the SXE PMD,
> including basic driver framework, data path setup, and advanced
> offload features (VLAN, RSS, DCB, PTP etc.).
> 
> V3:
>  - Addressed AI comments
> 
> Jie Liu (20):
>   net/sxe2: support AVX512 vectorized path for Rx and Tx
>   net/sxe2: add AVX2 vector data path for Rx and Tx
>   drivers: add supported packet types get callback
>   net/sxe2: support L2 filtering and MAC config
>   drivers: support RSS feature
>   net/sxe2: support TM hierarchy and shaping
>   net/sxe2: support IPsec inline protocol offload
>   net/sxe2: support statistics and multi-process
>   drivers: interrupt handling
>   net/sxe2: add NEON vec Rx/Tx burst functions
>   drivers: add support for VF representors
>   net/sxe2: add support for custom UDP tunnel ports
>   net/sxe2: support firmware version reading
>   net/sxe2: implement get monitor address
>   common/sxe2: add shared SFP module definitions
>   net/sxe2: support SFP module info and EEPROM access
>   net/sxe2: implement private dump info
>   net/sxe2: add mbuf validation in Tx debug mode
>   drivers: add testpmd commands for private features
>   net/sxe2: update sxe2 feature matrix docs
> 
>  doc/guides/nics/features/sxe2.ini          |   66 +
>  drivers/common/sxe2/sxe2_common.c          |  156 ++
>  drivers/common/sxe2/sxe2_common.h          |    4 +
>  drivers/common/sxe2/sxe2_flow_public.h     |  633 +++++++
>  drivers/common/sxe2/sxe2_ioctl_chnl.c      |  178 +-
>  drivers/common/sxe2/sxe2_ioctl_chnl_func.h |   18 +
>  drivers/common/sxe2/sxe2_msg.h             |  118 ++
>  drivers/common/sxe2/sxe2_ptype.h           | 1793 ++++++++++++++++++
>  drivers/net/sxe2/meson.build               |   56 +-
>  drivers/net/sxe2/sxe2_cmd_chnl.c           | 1587 +++++++++++++++-
>  drivers/net/sxe2/sxe2_cmd_chnl.h           |  139 ++
>  drivers/net/sxe2/sxe2_drv_cmd.h            |  521 +++++-
>  drivers/net/sxe2/sxe2_dump.c               |  304 +++
>  drivers/net/sxe2/sxe2_dump.h               |   12 +
>  drivers/net/sxe2/sxe2_ethdev.c             | 1531 +++++++++++++++-
>  drivers/net/sxe2/sxe2_ethdev.h             |  112 +-
>  drivers/net/sxe2/sxe2_ethdev_repr.c        |  610 ++++++
>  drivers/net/sxe2/sxe2_ethdev_repr.h        |   32 +
>  drivers/net/sxe2/sxe2_filter.c             |  897 +++++++++
>  drivers/net/sxe2/sxe2_filter.h             |  100 +
>  drivers/net/sxe2/sxe2_flow.c               | 1391 ++++++++++++++
>  drivers/net/sxe2/sxe2_flow.h               |   30 +
>  drivers/net/sxe2/sxe2_flow_define.h        |  144 ++
>  drivers/net/sxe2/sxe2_flow_parse_action.c  | 1182 ++++++++++++
>  drivers/net/sxe2/sxe2_flow_parse_action.h  |   23 +
>  drivers/net/sxe2/sxe2_flow_parse_engine.c  |  106 ++
>  drivers/net/sxe2/sxe2_flow_parse_engine.h  |   13 +
>  drivers/net/sxe2/sxe2_flow_parse_pattern.c | 1935 ++++++++++++++++++++
>  drivers/net/sxe2/sxe2_flow_parse_pattern.h |   46 +
>  drivers/net/sxe2/sxe2_ipsec.c              | 1565 ++++++++++++++++
>  drivers/net/sxe2/sxe2_ipsec.h              |  254 +++
>  drivers/net/sxe2/sxe2_irq.c                | 1025 +++++++++++
>  drivers/net/sxe2/sxe2_irq.h                |   25 +
>  drivers/net/sxe2/sxe2_mac.c                |  535 ++++++
>  drivers/net/sxe2/sxe2_mac.h                |   84 +
>  drivers/net/sxe2/sxe2_mp.c                 |  414 +++++
>  drivers/net/sxe2/sxe2_mp.h                 |   67 +
>  drivers/net/sxe2/sxe2_queue.c              |   17 +-
>  drivers/net/sxe2/sxe2_rss.c                |  584 ++++++
>  drivers/net/sxe2/sxe2_rss.h                |   81 +
>  drivers/net/sxe2/sxe2_rx.c                 |   38 +
>  drivers/net/sxe2/sxe2_rx.h                 |    2 +
>  drivers/net/sxe2/sxe2_security.c           |  335 ++++
>  drivers/net/sxe2/sxe2_security.h           |   77 +
>  drivers/net/sxe2/sxe2_stats.c              |  591 ++++++
>  drivers/net/sxe2/sxe2_stats.h              |   39 +
>  drivers/net/sxe2/sxe2_switchdev.c          |  332 ++++
>  drivers/net/sxe2/sxe2_switchdev.h          |   33 +
>  drivers/net/sxe2/sxe2_testpmd.c            |  733 ++++++++
>  drivers/net/sxe2/sxe2_testpmd_lib.c        |  969 ++++++++++
>  drivers/net/sxe2/sxe2_testpmd_lib.h        |  142 ++
>  drivers/net/sxe2/sxe2_tm.c                 | 1169 ++++++++++++
>  drivers/net/sxe2/sxe2_tm.h                 |   78 +
>  drivers/net/sxe2/sxe2_tx.c                 |    7 +
>  drivers/net/sxe2/sxe2_txrx.c               |  176 +-
>  drivers/net/sxe2/sxe2_txrx.h               |    4 +
>  drivers/net/sxe2/sxe2_txrx_check_mbuf.c    |  595 ++++++
>  drivers/net/sxe2/sxe2_txrx_check_mbuf.h    |   38 +
>  drivers/net/sxe2/sxe2_txrx_poll.c          |  243 ++-
>  drivers/net/sxe2/sxe2_txrx_vec.c           |   46 +-
>  drivers/net/sxe2/sxe2_txrx_vec.h           |   38 +-
>  drivers/net/sxe2/sxe2_txrx_vec_avx2.c      |  777 ++++++++
>  drivers/net/sxe2/sxe2_txrx_vec_avx512.c    |  897 +++++++++
>  drivers/net/sxe2/sxe2_txrx_vec_common.h    |    1 +
>  drivers/net/sxe2/sxe2_txrx_vec_neon.c      |  707 +++++++
>  drivers/net/sxe2/sxe2_vsi.c                |  146 ++
>  drivers/net/sxe2/sxe2_vsi.h                |   12 +-
>  drivers/net/sxe2/sxe2vf_regs.h             |   82 +
>  68 files changed, 26571 insertions(+), 124 deletions(-)
>  create mode 100644 drivers/common/sxe2/sxe2_flow_public.h
>  create mode 100644 drivers/common/sxe2/sxe2_msg.h
>  create mode 100644 drivers/common/sxe2/sxe2_ptype.h
>  create mode 100644 drivers/net/sxe2/sxe2_dump.c
>  create mode 100644 drivers/net/sxe2/sxe2_dump.h
>  create mode 100644 drivers/net/sxe2/sxe2_ethdev_repr.c
>  create mode 100644 drivers/net/sxe2/sxe2_ethdev_repr.h
>  create mode 100644 drivers/net/sxe2/sxe2_filter.c
>  create mode 100644 drivers/net/sxe2/sxe2_filter.h
>  create mode 100644 drivers/net/sxe2/sxe2_flow.c
>  create mode 100644 drivers/net/sxe2/sxe2_flow.h
>  create mode 100644 drivers/net/sxe2/sxe2_flow_define.h
>  create mode 100644 drivers/net/sxe2/sxe2_flow_parse_action.c
>  create mode 100644 drivers/net/sxe2/sxe2_flow_parse_action.h
>  create mode 100644 drivers/net/sxe2/sxe2_flow_parse_engine.c
>  create mode 100644 drivers/net/sxe2/sxe2_flow_parse_engine.h
>  create mode 100644 drivers/net/sxe2/sxe2_flow_parse_pattern.c
>  create mode 100644 drivers/net/sxe2/sxe2_flow_parse_pattern.h
>  create mode 100644 drivers/net/sxe2/sxe2_ipsec.c
>  create mode 100644 drivers/net/sxe2/sxe2_ipsec.h
>  create mode 100644 drivers/net/sxe2/sxe2_irq.c
>  create mode 100644 drivers/net/sxe2/sxe2_mac.c
>  create mode 100644 drivers/net/sxe2/sxe2_mac.h
>  create mode 100644 drivers/net/sxe2/sxe2_mp.c
>  create mode 100644 drivers/net/sxe2/sxe2_mp.h
>  create mode 100644 drivers/net/sxe2/sxe2_rss.c
>  create mode 100644 drivers/net/sxe2/sxe2_rss.h
>  create mode 100644 drivers/net/sxe2/sxe2_security.c
>  create mode 100644 drivers/net/sxe2/sxe2_security.h
>  create mode 100644 drivers/net/sxe2/sxe2_stats.c
>  create mode 100644 drivers/net/sxe2/sxe2_stats.h
>  create mode 100644 drivers/net/sxe2/sxe2_switchdev.c
>  create mode 100644 drivers/net/sxe2/sxe2_switchdev.h
>  create mode 100644 drivers/net/sxe2/sxe2_testpmd.c
>  create mode 100644 drivers/net/sxe2/sxe2_testpmd_lib.c
>  create mode 100644 drivers/net/sxe2/sxe2_testpmd_lib.h
>  create mode 100644 drivers/net/sxe2/sxe2_tm.c
>  create mode 100644 drivers/net/sxe2/sxe2_tm.h
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_check_mbuf.c
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_check_mbuf.h
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_vec_avx2.c
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_vec_avx512.c
>  create mode 100644 drivers/net/sxe2/sxe2_txrx_vec_neon.c
>  create mode 100644 drivers/net/sxe2/sxe2vf_regs.h
> 

Thanks for fixing everything from review.

But CI builds are failing with clang

FAILED: [code=1] drivers/libtmp_rte_net_sxe2.a.p/net_sxe2_sxe2_filter.c.o 
ccache clang -Idrivers/libtmp_rte_net_sxe2.a.p -Idrivers -I../drivers -Idrivers/net/sxe2 -I../drivers/net/sxe2 -Idrivers/common/sxe2 -I../drivers/common/sxe2 -Ilib/ethdev -I../lib/ethdev -Ilib/eal/common -I../lib/eal/common -I. -I.. -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include -I../kernel/linux -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/metrics -I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Ilib/argparse -I../lib/argparse -Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/ring -I../lib/ring -Ilib/meter -I../lib/meter -Idrivers/bus/pci -I../drivers/bus/pci -I../drivers/bus/pci/linux -Ilib/pci -I../lib/pci -Idrivers/bus/vdev -I../drivers/bus/vdev -Ilib/hash -I../lib/hash -Ilib/rcu -I../lib/rcu -Ilib/cryptodev -I../lib/cryptodev -Ilib/security -I../lib/security -Ilib/cmdline -I../lib/cmdline -Xclang -fcolor-diagnostics -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c11 -O2 -g -include rte_config.h -Wvla -Wcast-qual -Wcomma -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wshadow -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=corei7 -mrtm -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-address-of-packed-member -g -DCC_AVX512_SUPPORT -DRTE_LOG_DEFAULT_LOGTYPE=pmd.net.sxe2 -DRTE_ANNOTATE_LOCKS -Wthread-safety -MD -MQ drivers/libtmp_rte_net_sxe2.a.p/net_sxe2_sxe2_filter.c.o -MF drivers/libtmp_rte_net_sxe2.a.p/net_sxe2_sxe2_filter.c.o.d -o drivers/libtmp_rte_net_sxe2.a.p/net_sxe2_sxe2_filter.c.o -c ../drivers/net/sxe2/sxe2_filter.c
../drivers/net/sxe2/sxe2_filter.c:401:1: error: expected statement
}
^
1 error generated.

^ permalink raw reply

* Re: [PATCH v5 0/2] few improvemnts for SORING lib
From: Thomas Monjalon @ 2026-06-01 15:41 UTC (permalink / raw)
  To: Konstantin Ananyev; +Cc: dev, wathsala.vithanage, mb
In-Reply-To: <20260505154728.82235-1-konstantin.ananyev@huawei.com>

> Konstantin Ananyev (2):
>   ring: make soring to always finalize its own stage
>   ring: introduce peek API for soring

Applied, thanks.



^ permalink raw reply

* Re: [PATCH v4] ring: fix zero-copy burst API documentation
From: Thomas Monjalon @ 2026-06-01 15:44 UTC (permalink / raw)
  To: Zhiguang Jin
  Cc: Konstantin Ananyev, Wathsala Vithanage, Dharmik Thakkar,
	Honnappa Nagarahalli, dev, stable
In-Reply-To: <20260521030009.23491-1-jinzhiguang@kylinos.cn>

21/05/2026 05:00, Zhiguang Jin:
> These are burst APIs relying on RTE_RING_QUEUE_VARIABLE behavior, they
> operate on a best-effort basis and return the actual number of
> objects processed (between 0 and n).
> 
> Update description to match implementation.
> 
> Fixes: 47bec9a5ca9f ("ring: add zero copy API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zhiguang Jin <jinzhiguang@kylinos.cn>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

Applied, thanks.




^ permalink raw reply

* Re: [PATCH] stack: rightsize cache guard
From: Thomas Monjalon @ 2026-06-01 15:58 UTC (permalink / raw)
  To: Morten Brørup; +Cc: dev
In-Reply-To: <20260504071539.252926-1-mb@smartsharesystems.com>

04/05/2026 09:15, Morten Brørup:
> Using 2 cache lines as cache guard after the table holding the stack
> elements may be too many or too few. Instead, use the number of cache
> lines specified in the build configuration.
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>

Applied, thanks.




^ permalink raw reply

* [PATCH v2] eal: fix data race in hugepage prefault
From: Stephen Hemminger @ 2026-06-01 16:00 UTC (permalink / raw)
  To: dev
  Cc: Stephen Hemminger, stable, Michal Sieron, Thomas Monjalon,
	Anatoly Burakov, Bruce Richardson
In-Reply-To: <20260520170812.759638-1-stephen@networkplumber.org>

The prefault step in alloc_seg() reads a value from the hugepage and
writes it back unchanged to force the kernel to commit the backing
page. The read and write were not atomic, which races with concurrent
access to the same physical page from a secondary process attaching
to the hugetlbfs-backed mapping during rte_eal_init().

Replace the non-atomic load+store with a single atomic fetch-or of
zero. This touches the page with an atomic read-modify-write without
changing its contents, eliminating the race while preserving the
original intent of forcing a write fault.

Fixes: 0f1631be24bd ("mem: fix page fault trigger")
Cc: stable@dpdk.org

Reported-by: Michal Sieron <michal.sieron@nokia.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 .mailmap                     | 1 +
 lib/eal/linux/eal_memalloc.c | 8 +++++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/.mailmap b/.mailmap
index 43febb9030..3c45e365d3 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1094,6 +1094,7 @@ Michal Mazurek <maz@semihalf.com>
 Michal Michalik <michal.michalik@intel.com>
 Michal Nowak <michal2.nowak@intel.com>
 Michal Schmidt <mschmidt@redhat.com>
+Michal Sieron <michal.sieron@nokia.com>
 Michal Swiatkowski <michal.swiatkowski@intel.com>
 Michal Wilczynski <michal.wilczynski@intel.com>
 Michał Mirosław <michal.miroslaw@atendesoftware.pl> <mirq-linux@rere.qmqm.pl>
diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index a39bc31c7b..7359a41d3f 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -25,6 +25,7 @@
 #include <linux/falloc.h>
 #include <linux/mman.h> /* for hugetlb-related mmap flags */
 
+#include <rte_atomic.h>
 #include <rte_common.h>
 #include <rte_log.h>
 #include <rte_eal.h>
@@ -597,10 +598,11 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 
 	/* we need to trigger a write to the page to enforce page fault and
 	 * ensure that page is accessible to us, but we can't overwrite value
-	 * that is already there, so read the old value, and write itback.
-	 * kernel populates the page with zeroes initially.
+	 * that is already there.
+	 * Use an atomic OR with zero to touch the page without changing its contents.
 	 */
-	*(volatile int *)addr = *(volatile int *)addr;
+	(void)rte_atomic_fetch_or_explicit((__rte_atomic uint64_t *)addr, 0,
+					   rte_memory_order_relaxed);
 
 	iova = rte_mem_virt2iova(addr);
 	if (iova == RTE_BAD_PHYS_ADDR) {
-- 
2.53.0


^ permalink raw reply related

* [PATCH v2] net: fix GTP Tunnel parse out-of-bounds read
From: Stephen Hemminger @ 2026-06-01 16:17 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, stable, Andrew Rybchenko, Jie Hai
In-Reply-To: <20260409161556.141251-1-stephen@networkplumber.org>

If packet is fragmented across multiple mbufs or the packet
has only GTP header the code would reference outside
the incoming mbuf.

Send GTP packet:
- Valid GTP header (8 bytes)
- msg_type = 0xff
- e=1, s=1, pn=1 (sets gtp_len = 12)
- Total packet size = 10 bytes

Read at gh + 12 accesses 2 bytes beyond packet end.

The fix is to use rte_pktmbuf_read in a manner similar
to the read of the GTP header.

Fixes: 64ed7f854cf4 ("net: add tunnel packet type parsing")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
v2 - change variable name of l3 header byte

 lib/net/rte_net.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
index 458b4814a9..0a91e92ba0 100644
--- a/lib/net/rte_net.c
+++ b/lib/net/rte_net.c
@@ -219,8 +219,7 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
 	case RTE_GTPU_UDP_PORT: {
 		const struct rte_gtp_hdr *gh;
 		struct rte_gtp_hdr gh_copy;
-		uint8_t gtp_len;
-		uint8_t ip_ver;
+		uint32_t gtp_len;
 		gh = rte_pktmbuf_read(m, *off, sizeof(*gh), &gh_copy);
 		if (unlikely(gh == NULL))
 			return 0;
@@ -231,9 +230,17 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
 		 * Check message type. If message type is 0xff, it is
 		 * a GTP data packet. If not, it is a GTP control packet
 		 */
+		*off += gtp_len;
 		if (gh->msg_type == 0xff) {
-			ip_ver = *(const uint8_t *)((const char *)gh + gtp_len);
-			ip_ver = (ip_ver) & 0xf0;
+			const uint8_t *l3_byte;
+			uint8_t l3_copy, ip_ver;
+
+			/* read first byte of l3 header */
+			l3_byte = rte_pktmbuf_read(m, *off, sizeof(uint8_t), &l3_copy);
+			if (unlikely(l3_byte == NULL))
+				return 0;
+
+			ip_ver = *l3_byte & 0xf0;
 			if (ip_ver == RTE_GTP_TYPE_IPV4)
 				*proto = rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4);
 			else if (ip_ver == RTE_GTP_TYPE_IPV6)
@@ -243,7 +250,6 @@ ptype_tunnel_with_udp(uint16_t *proto, const struct rte_mbuf *m,
 		} else {
 			*proto = 0;
 		}
-		*off += gtp_len;
 		hdr_lens->inner_l2_len = gtp_len + sizeof(struct rte_udp_hdr);
 		hdr_lens->tunnel_len = gtp_len;
 		if (port_no == RTE_GTPC_UDP_PORT)
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v5 1/1] net/mana: add device reset support
From: Stephen Hemminger @ 2026-06-01 16:31 UTC (permalink / raw)
  To: Wei Hu; +Cc: dev, longli, weh
In-Reply-To: <20260529142648.148407-2-weh@linux.microsoft.com>

On Fri, 29 May 2026 07:26:48 -0700
Wei Hu <weh@linux.microsoft.com> wrote:

> +#define MANA_OPS_1_LOCK(_func)						\
> +static int								\
> +_func##_lock(struct rte_eth_dev *dev)					\
> +{									\
> +	struct mana_priv *priv = dev->data->dev_private;		\
> +	int ret;							\
> +	if (!pthread_mutex_trylock(&priv->reset_ops_lock)) {		\
> +		if (rte_atomic_load_explicit(&priv->dev_state,		\
> +		    rte_memory_order_acquire) !=			\
> +		    MANA_DEV_ACTIVE) {					\
> +			pthread_mutex_unlock(&priv->reset_ops_lock);	\
> +			return -EBUSY;					\
> +		}							\
> +		ret = _func(dev);					\
> +		pthread_mutex_unlock(&priv->reset_ops_lock);		\
> +	} else {							\
> +		ret = -EBUSY;						\
> +	}								\
> +	return ret;							\
> +}
> +
> +MANA_OPS_1_LOCK(mana_dev_configure)
> +
> +MANA_OPS_1_LOCK(mana_dev_start)

I strongly dislike wrapping locking in macros.
Macros make code harder to analyze and hide things.

Also, using pthread_mutex here needs seems like a bigger hammer than needed.
It looks like reset is just an atomic flag and as long as it was in shared
memory simple atomic operators would suffice.

In Linux there are many ways to express the same thing
but in general if most drivers follow the same patterns it helps
when there is an issue to be able to fix it globally.

Also any code which turns off thread safety analysis is a red flag
for me. It needs strong justification.

Overall this reset logic looks more complex than other drivers.

^ permalink raw reply

* [PATCH v7] mempool: improve cache behaviour and performance
From: Morten Brørup @ 2026-06-01 16:40 UTC (permalink / raw)
  To: dev, Andrew Rybchenko, Bruce Richardson, Jingjing Wu,
	Praveen Shetty, Hemant Agrawal, Sachin Saxena, Thomas Monjalon
  Cc: Morten Brørup
In-Reply-To: <20260408141315.904381-1-mb@smartsharesystems.com>

This patch refactors the mempool cache to eliminate some unexpected
behaviour and reduce the mempool cache miss rate.

1.
The actual cache size was 1.5 times the cache size specified at run-time
mempool creation.
This was obviously not expected by application developers.

2.
In get operations, the check for when to use the cache as bounce buffer
did not respect the run-time configured cache size,
but compared to the build time maximum possible cache size
(RTE_MEMPOOL_CACHE_MAX_SIZE, default 512).
E.g. with a configured cache size of 32 objects, getting 256 objects
would first fetch 32 + 256 = 288 objects into the cache,
and then move the 256 objects from the cache to the destination memory,
instead of fetching the 256 objects directly to the destination memory.
This had a performance cost.
However, this is unlikely to occur in real applications, so it is not
important in itself.

3.
When putting objects into a mempool, and the mempool cache did not have
free space for so many objects,
the cache was flushed completely, and the new objects were then put into
the cache.
I.e. the cache drain level was zero.
This (complete cache flush) meant that a subsequent get operation (with
the same number of objects) completely emptied the cache,
so another subsequent get operation required replenishing the cache.

Similarly,
When getting objects from a mempool, and the mempool cache did not hold so
many objects,
the cache was replenished to cache->size + remaining objects,
and then (the remaining part of) the requested objects were fetched via
the cache,
which left the cache filled (to cache->size) at completion.
I.e. the cache refill level was cache->size (plus some, depending on
request size).

(1) was improved by generally comparing to cache->size instead of
cache->flushthresh, when considering the capacity of the cache.
The cache->flushthresh field is kept for API/ABI compatibility purposes,
and initialized to cache->size instead of cache->size * 1.5.

(2) was improved by generally comparing to cache->size / 2 instead of
RTE_MEMPOOL_CACHE_MAX_SIZE, when checking the bounce buffer limit.

(3) was improved by flushing and replenishing the cache by half its size,
so a flush/refill can be followed randomly by get or put requests.
This also reduced the number of objects in each flush/refill operation.

As a consequence of these changes, the size of the array holding the
objects in the cache (cache->objs[]) no longer needs to be
2 * RTE_MEMPOOL_CACHE_MAX_SIZE, and can be reduced to
RTE_MEMPOOL_CACHE_MAX_SIZE at an API/ABI breaking release.

Performance data:
With a real WAN Optimization application, where the number of allocated
packets varies (as they are held in e.g. shaper queues), the mempool
cache miss rate dropped from ca. 1/20 objects to ca. 1/48 objects.
This was deployed in production at an ISP, and using an effective cache
size of 384 objects.

Bugzilla ID: 1027
Fixes: ea5dd2744b90 ("mempool: cache optimisations")
Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
v7:
* Rebased. Dependency no longer required. (Thomas)
v6:
* Moved driver changes out as separate patches, for easier review. (Bruce)
  Tests using the Intel idpf PMD in AVX512 mode may fail with this patch.
* Reverted a small code comment change. The original was better. (Bruce)
* Reverted rte_mempool_create() description requiring the cache_size to be
  an even number. There is no such requirement.
v5:
* Flush the cache from the bottom, where objects are colder, and move down
  the remaining objects, which are hotter.
* In the Intel idpf PMD, move up the hot objects in the cache and refill
  with cold objects at the bottom.
v4:
* Added Bugzilla ID.
* Added Fixes tag. For reference only.
* Moved fast-free related update of Intel common driver out as a separate
  patch, and depend on that patch.
* Omitted unrelated changes to the Intel idpf AVX512 driver, specifically
  fixing an indentation and adding mbuf instrumentation.
* Omitted unrelated changes to the mempool library, specifically adding
  __rte_restrict and changing a couple of comments to proper sentences.
* Please checkpatches by swapping operators in a couple of comparisons.
v3:
* Fixed my copy-paste bug in idpf_splitq_rearm().
v2:
* Fixed issue found by abidiff:
  Reverted cache objects array size reduction. Added a note instead.
* Added missing mbuf instrumentation to the Intel idpf AVX512 driver.
* Updated idpf_splitq_rearm() like idpf_singleq_rearm().
* Added a few more __rte_assume(). (Inspired by AI review)
* Updated NXP dpaa and dpaa2 mempool drivers to not set mempool cache
  flush threshold.
* Added release notes.
* Added deprecation notes.
---
 doc/guides/rel_notes/deprecation.rst   |  7 +++
 doc/guides/rel_notes/release_26_07.rst | 11 +++++
 lib/mempool/rte_mempool.c              | 14 +-----
 lib/mempool/rte_mempool.h              | 66 ++++++++++++++++----------
 4 files changed, 61 insertions(+), 37 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 17f90a6352..1b6cc181fb 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -158,3 +158,10 @@ Deprecation Notices
 * net/iavf: The dynamic mbuf field used to detect LLDP packets on the
   transmit path in the iavf PMD will be removed in a future release.
   After removal, only packet type-based detection will be supported.
+
+* mempool: The ``flushthresh`` field in ``struct rte_mempool_cache``
+  is obsolete, and will be removed in DPDK 26.11.
+
+* mempool: The object array in ``struct rte_mempool_cache`` is oversize by
+  factor two, and will be reduced to ``RTE_MEMPOOL_CACHE_MAX_SIZE`` in
+  DPDK 26.11.
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 8b4f8401e2..1b15c878f6 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -63,6 +63,17 @@ New Features
     ``rte_eal_init`` and the application is responsible for probing each device,
   * ``--auto-probing`` enables the initial bus probing, which is the current default behavior.
 
+* **Changed effective size of mempool cache.**
+
+  * The effective size of a mempool cache was changed to match the specified size at mempool creation; the effective size was previously 50 % larger than requested.
+  * The ``flushthresh`` field of the ``struct rte_mempool_cache`` became obsolete, but was kept for API/ABI compatibility purposes.
+  * The effective size of the ``objs`` array in the ``struct rte_mempool_cache`` was reduced to ``RTE_MEMPOOL_CACHE_MAX_SIZE``, but its size was kept for API/ABI compatibility purposes.
+
+* **Improved mempool cache flush/refill algorithm.**
+
+  The mempool cache flush/refill algorithm was improved, to reduce the mempool cache miss rate for most application types.
+  Applications where each lcore only puts or gets to a mempool, e.g. pipelined applications where ethdev Rx and Tx run on separate lcores, should adapt to the new algorithm by doubling their configured mempool cache size, to avoid doubling their mempool cache miss rate.
+
 * **Added LinkData sxe2 ethernet driver.**
 
   Added network driver for the LinkData network adapters.
diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 3042d94c14..805b52cc58 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -52,11 +52,6 @@ static void
 mempool_event_callback_invoke(enum rte_mempool_event event,
 			      struct rte_mempool *mp);
 
-/* Note: avoid using floating point since that compiler
- * may not think that is constant.
- */
-#define CALC_CACHE_FLUSHTHRESH(c) (((c) * 3) / 2)
-
 #if defined(RTE_ARCH_X86)
 /*
  * return the greatest common divisor between a and b (fast algorithm)
@@ -757,13 +752,8 @@ rte_mempool_free(struct rte_mempool *mp)
 static void
 mempool_cache_init(struct rte_mempool_cache *cache, uint32_t size)
 {
-	/* Check that cache have enough space for flush threshold */
-	RTE_BUILD_BUG_ON(CALC_CACHE_FLUSHTHRESH(RTE_MEMPOOL_CACHE_MAX_SIZE) >
-			 RTE_SIZEOF_FIELD(struct rte_mempool_cache, objs) /
-			 RTE_SIZEOF_FIELD(struct rte_mempool_cache, objs[0]));
-
 	cache->size = size;
-	cache->flushthresh = CALC_CACHE_FLUSHTHRESH(size);
+	cache->flushthresh = size; /* Obsolete; for API/ABI compatibility purposes only */
 	cache->len = 0;
 }
 
@@ -850,7 +840,7 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 
 	/* asked cache too big */
 	if (cache_size > RTE_MEMPOOL_CACHE_MAX_SIZE ||
-	    CALC_CACHE_FLUSHTHRESH(cache_size) > n) {
+	    cache_size > n) {
 		rte_errno = EINVAL;
 		return NULL;
 	}
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 8c384d3453..26f47bf258 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -89,7 +89,7 @@ struct __rte_cache_aligned rte_mempool_debug_stats {
  */
 struct __rte_cache_aligned rte_mempool_cache {
 	uint32_t size;	      /**< Size of the cache */
-	uint32_t flushthresh; /**< Threshold before we flush excess elements */
+	uint32_t flushthresh; /**< Obsolete; for API/ABI compatibility purposes only */
 	uint32_t len;	      /**< Current cache count */
 #ifdef RTE_LIBRTE_MEMPOOL_STATS
 	uint32_t unused;
@@ -107,8 +107,10 @@ struct __rte_cache_aligned rte_mempool_cache {
 	/**
 	 * Cache objects
 	 *
-	 * Cache is allocated to this size to allow it to overflow in certain
-	 * cases to avoid needless emptying of cache.
+	 * Note:
+	 * Cache is allocated at double size for API/ABI compatibility purposes only.
+	 * When reducing its size at an API/ABI breaking release,
+	 * remember to add a cache guard after it.
 	 */
 	alignas(RTE_CACHE_LINE_SIZE) void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2];
 };
@@ -1047,11 +1049,16 @@ rte_mempool_free(struct rte_mempool *mp);
  *   If cache_size is non-zero, the rte_mempool library will try to
  *   limit the accesses to the common lockless pool, by maintaining a
  *   per-lcore object cache. This argument must be lower or equal to
- *   RTE_MEMPOOL_CACHE_MAX_SIZE and n / 1.5.
+ *   RTE_MEMPOOL_CACHE_MAX_SIZE and n.
  *   The access to the per-lcore table is of course
  *   faster than the multi-producer/consumer pool. The cache can be
  *   disabled if the cache_size argument is set to 0; it can be useful to
  *   avoid losing objects in cache.
+ *   Note:
+ *   Mempool put/get requests of more than cache_size / 2 objects may be
+ *   partially or fully served directly by the multi-producer/consumer
+ *   pool, to avoid the overhead of copying the objects twice (instead of
+ *   once) when using the cache as a bounce buffer.
  * @param private_data_size
  *   The size of the private data appended after the mempool
  *   structure. This is useful for storing some private data after the
@@ -1390,22 +1397,30 @@ rte_mempool_do_generic_put(struct rte_mempool *mp, void * const *obj_table,
 	RTE_MEMPOOL_CACHE_STAT_ADD(cache, put_bulk, 1);
 	RTE_MEMPOOL_CACHE_STAT_ADD(cache, put_objs, n);
 
-	__rte_assume(cache->flushthresh <= RTE_MEMPOOL_CACHE_MAX_SIZE * 2);
-	__rte_assume(cache->len <= RTE_MEMPOOL_CACHE_MAX_SIZE * 2);
-	__rte_assume(cache->len <= cache->flushthresh);
-	if (likely(cache->len + n <= cache->flushthresh)) {
+	__rte_assume(cache->size <= RTE_MEMPOOL_CACHE_MAX_SIZE);
+	__rte_assume(cache->size / 2 <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
+	__rte_assume(cache->len <= RTE_MEMPOOL_CACHE_MAX_SIZE);
+	__rte_assume(cache->len <= cache->size);
+	if (likely(cache->len + n <= cache->size)) {
 		/* Sufficient room in the cache for the objects. */
 		cache_objs = &cache->objs[cache->len];
 		cache->len += n;
-	} else if (n <= cache->flushthresh) {
+	} else if (n <= cache->size / 2) {
 		/*
-		 * The cache is big enough for the objects, but - as detected by
-		 * the comparison above - has insufficient room for them.
-		 * Flush the cache to make room for the objects.
+		 * The number of objects is within the cache bounce buffer limit,
+		 * but - as detected by the comparison above - the cache has
+		 * insufficient room for them.
+		 * Flush the cache to the backend to make room for the objects;
+		 * flush (size / 2) objects from the bottom of the cache, where
+		 * objects are less hot, and move down the remaining objects, which
+		 * are more hot, from the upper half of the cache.
 		 */
-		cache_objs = &cache->objs[0];
-		rte_mempool_ops_enqueue_bulk(mp, cache_objs, cache->len);
-		cache->len = n;
+		__rte_assume(cache->len > cache->size / 2);
+		rte_mempool_ops_enqueue_bulk(mp, &cache->objs[0], cache->size / 2);
+		rte_memcpy(&cache->objs[0], &cache->objs[cache->size / 2],
+				sizeof(void *) * (cache->len - cache->size / 2));
+		cache_objs = &cache->objs[cache->len - cache->size / 2];
+		cache->len = cache->len - cache->size / 2 + n;
 	} else {
 		/* The request itself is too big for the cache. */
 		goto driver_enqueue_stats_incremented;
@@ -1524,7 +1539,7 @@ rte_mempool_do_generic_get(struct rte_mempool *mp, void **obj_table,
 	/* The cache is a stack, so copy will be in reverse order. */
 	cache_objs = &cache->objs[cache->len];
 
-	__rte_assume(cache->len <= RTE_MEMPOOL_CACHE_MAX_SIZE * 2);
+	__rte_assume(cache->len <= RTE_MEMPOOL_CACHE_MAX_SIZE);
 	if (likely(n <= cache->len)) {
 		/* The entire request can be satisfied from the cache. */
 		RTE_MEMPOOL_CACHE_STAT_ADD(cache, get_success_bulk, 1);
@@ -1548,13 +1563,13 @@ rte_mempool_do_generic_get(struct rte_mempool *mp, void **obj_table,
 	for (index = 0; index < len; index++)
 		*obj_table++ = *--cache_objs;
 
-	/* Dequeue below would overflow mem allocated for cache? */
-	if (unlikely(remaining > RTE_MEMPOOL_CACHE_MAX_SIZE))
+	/* Dequeue below would exceed the cache bounce buffer limit? */
+	__rte_assume(cache->size / 2 <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
+	if (unlikely(remaining > cache->size / 2))
 		goto driver_dequeue;
 
-	/* Fill the cache from the backend; fetch size + remaining objects. */
-	ret = rte_mempool_ops_dequeue_bulk(mp, cache->objs,
-			cache->size + remaining);
+	/* Fill the cache from the backend; fetch (size / 2) objects. */
+	ret = rte_mempool_ops_dequeue_bulk(mp, cache->objs, cache->size / 2);
 	if (unlikely(ret < 0)) {
 		/*
 		 * We are buffer constrained, and not able to fetch all that.
@@ -1568,10 +1583,11 @@ rte_mempool_do_generic_get(struct rte_mempool *mp, void **obj_table,
 	RTE_MEMPOOL_CACHE_STAT_ADD(cache, get_success_bulk, 1);
 	RTE_MEMPOOL_CACHE_STAT_ADD(cache, get_success_objs, n);
 
-	__rte_assume(cache->size <= RTE_MEMPOOL_CACHE_MAX_SIZE);
-	__rte_assume(remaining <= RTE_MEMPOOL_CACHE_MAX_SIZE);
-	cache_objs = &cache->objs[cache->size + remaining];
-	cache->len = cache->size;
+	__rte_assume(cache->size / 2 <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
+	__rte_assume(remaining <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
+	__rte_assume(remaining <= cache->size / 2);
+	cache_objs = &cache->objs[cache->size / 2];
+	cache->len = cache->size / 2 - remaining;
 	for (index = 0; index < remaining; index++)
 		*obj_table++ = *--cache_objs;
 
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v5 1/1] net/mana: add device reset support
From: Stephen Hemminger @ 2026-06-01 16:58 UTC (permalink / raw)
  To: Wei Hu; +Cc: dev, longli, weh
In-Reply-To: <20260529142648.148407-2-weh@linux.microsoft.com>

On Fri, 29 May 2026 07:26:48 -0700
Wei Hu <weh@linux.microsoft.com> wrote:

> From: Wei Hu <weh@microsoft.com>
> 
> Add support for handling hardware reset events in the MANA driver.
> When the MANA kernel driver receives a hardware service event, it
> initiates a device reset and notifies userspace via
> IBV_EVENT_DEVICE_FATAL. The DPDK driver handles this by performing
> an automatic teardown and recovery sequence.
> 
> The reset flow has two phases. In the enter phase, running on the
> EAL interrupt thread, the driver transitions the device state,
> waits for data path threads to reach a quiescent state using RCU,
> stops queues, tears down IB resources, and frees per-queue MR
> caches. A control thread is then spawned to handle the exit phase:
> it waits for the hardware to recover, unregisters the interrupt
> handler, re-probes the PCI device, reinitializes MR caches, and
> restarts queues.
> 
> A per-device mutex serializes the reset path with ethdev
> operations. The mutex uses PTHREAD_PROCESS_SHARED for multi-process
> support and is held across blocking IB verbs calls. Operations that
> cannot wait (configure, queue setup) return -EBUSY during reset,
> while dev_stop and dev_close join the reset thread before acquiring
> the lock to ensure proper sequencing. A CAS-based helper prevents
> double-join of the reset thread.
> 
> Multi-process support is included: secondary processes unmap and
> remap doorbell pages via IPC during the reset enter and exit
> phases. Data path functions in both primary and secondary
> processes check the device state atomically and return early when
> the device is not active. RCU quiescent state tracking uses
> per-queue thread IDs in shared hugepage memory, covering both
> primary and secondary process data path threads.
> 
> The driver uses ethdev recovery events to notify upper layers
> (e.g. netvsc) of the reset lifecycle: RTE_ETH_EVENT_ERR_RECOVERING
> on entry, RTE_ETH_EVENT_RECOVERY_SUCCESS or
> RTE_ETH_EVENT_RECOVERY_FAILED on completion. A PCI device removal
> event callback distinguishes hot-remove from service reset.
> 
> Documentation for the device reset feature is added in the MANA
> NIC guide and the 26.07 release notes.
> 
> Signed-off-by: Wei Hu <weh@microsoft.com>
> ---

I went a deeper with AI and tried to figure out a good way
to do what the driver is trying to do without reinventing so much.



This reset logic is considerably more complex than other DPDK drivers,
and most of the complexity looks self-inflicted. A few specific things.

The RCU use is not really RCU. thread_online/offline are called on every
rx/tx burst, and the "thread" token is the queue index, not a thread. So
it is a per-queue in-use flag paid for on the hottest path, plus a new
library dependency, to express "wait until no queue is mid-burst" -- which
the driver already half does by swapping the burst function to
mana_*_burst_removed and checking dev_state. Please drop the rcu use. If
you must drain readers, a per-queue atomic flag is lighter and local; the
fast path already has the dev_state acquire-load it needs.

The data path only needs the atomic, and that part is fine. The lock is
legitimate for serializing the teardown/rebuild against control ops, but
the way it is used is the problem: it is acquired in mana_intr_handler,
released in mana_reset_enter, re-acquired in mana_reset_thread, and
released in mana_reset_exit_delay. That cross-function, cross-thread
handoff is exactly why every function needs __rte_no_thread_safety_analysis.
Acquire and release the lock in the same function and the annotations all
go away. Turning off thread-safety analysis needs strong justification and
this does not have it.

Wrapping the ops in MANA_OPS_*_LOCK macros hides the lock/state protocol.
A single explicit helper at the top of each op is just as terse and stays
analyzable.

Two real bugs:

- Recovery and INTR_RMV events are sent via rte_eth_dev_callback_process
  while reset_ops_lock is held. An app handling INTR_RMV or RECOVERY_FAILED
  by calling dev_stop/dev_close will re-enter the lock (non-recursive) and
  deadlock; on the recovery path the callback runs on the reset thread so
  it also tries to join itself. Emit these events after dropping the lock,
  as ERR_RECOVERING already does.

- thread_online is taken at the top of the burst functions but
  thread_offline is only on some return paths. Any early return that
  misses it leaves a token non-quiescent and rte_rcu_qsbr_check() in
  mana_reset_enter spins forever. This is the kind of breakage the per-burst
  bracketing invites -- another reason to drop it.

Also: reset_ops_lock is held across ibv_close_device and the PCI re-probe
(blocking under a sleeping mutex), and rte_alarm.h is included but no
longer used.

Please look at how hns3 or mlx5 structure reset/recovery. Matching the
common pattern means fixes can be made across drivers at once.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox