[None][fix] enable hmac in RPC

[Superjomn]

Summary by CodeRabbit

Release Notes

• New Features

  • Added automatic HMAC key generation and injection into RPC worker initialization parameters for distributed communication.

• Tests

  • Added test coverage for HMAC key generation and validation in RPC proxy initialization.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

• PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

• PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

• Test cases are provided for new code paths (see test instructions)

• Any new dependencies have been scanned for license and vulnerabilities

• CODEOWNERS updated if ownership changes

• Documentation updated as needed

• Update tava architecture diagram if there is a significant design change in PR.

• The reviewers assigned automatically/manually are appropriate for the PR.

• Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...

Provide a user friendly way for developers to interact with a Jenkins server.

Run /bot [-h|--help] to print this help message.

See details below for each supported subcommand.

Details

run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]

Launch build/test pipelines. All previously running jobs will be killed.

--reuse-test (optional)pipeline-id (OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.

--disable-reuse-test (OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.

--disable-fail-fast (OPTIONAL) : Disable fail fast on build/tests/infra failures.

--skip-test (OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.

--stage-list "A10-PyTorch-1, xxx" (OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.

--gpu-type "A30, H100_PCIe" (OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.

--test-backend "pytorch, cpp" (OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.

--only-multi-gpu-test (OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.

--disable-multi-gpu-test (OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.

--add-multi-gpu-test (OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.

--post-merge (OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.

--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" (OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".

--detailed-log (OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.

--debug (OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in the stage-list parameter to access the appropriate container environment. Note: Does NOT update GitHub check status.

For guidance on mapping tests to stage names, see docs/source/reference/ci-overview.md
and the scripts/test_to_stage_mapping.py helper.

kill

kill

Kill all running builds associated with pull request.

skip

skip --comment COMMENT

Skip testing for latest commit on pull request. --comment "Reason for skipping build/test" is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

reuse-pipeline

reuse-pipeline

Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

[Superjomn]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #27125 [ run ] triggered by Bot. Commit: ceebe9f

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes introduce HMAC-based authentication to the RPC communication infrastructure by generating a 32-byte HMAC key during executor initialization and threading it through RPC client/server constructors and worker initialization payloads to enable encrypted RPC channels.

Changes

Cohort / File(s)	Summary
HMAC Key Generation & Executor Initialization tensorrt_llm/executor/rpc_proxy_mixin.py, tensorrt_llm/executor/ray_executor.py	Generates 32-byte HMAC key via os.urandom(32), passes to RPCClient constructor, and injects into worker_kwargs. Adds import os.
Worker Initialization with HMAC tensorrt_llm/executor/ray_gpu_worker.py, tensorrt_llm/executor/rpc_proxy.py	Adds hmac_key: Optional[bytes] parameter to worker constructors; injects key into worker initialization payloads and forwards to init_rpc_worker.
RPC Client/Server Configuration tensorrt_llm/executor/rpc_client.py, tensorrt_llm/executor/rpc_server.py, tensorrt_llm/executor/rpc_worker.py	Updates RPC client/server instantiation to conditionally enable HMAC encryption based on presence of hmac_key parameter.
RPC Worker Mixin tensorrt_llm/executor/rpc_worker_mixin.py	Extends init_rpc_worker signature with hmac_key: Optional[bytes] parameter; stores key and passes to RPCServer during RPC server creation.
Test Coverage tests/unittest/executor/test_rpc_proxy.py	Adds test_hmac_key_generation to verify HMAC key generation, presence in worker_kwargs, object identity, and successful RPC generation with auto-generated key.

Sequence Diagram(s)

sequenceDiagram
    participant Executor as Executor/Mixin
    participant Worker as RayGPUWorker
    participant RPC as RPC Layer
    participant Server as RPCServer

    Executor->>Executor: Generate hmac_key = os.urandom(32)
    Executor->>Executor: Store self.hmac_key
    Executor->>Executor: Inject hmac_key into worker_kwargs
    
    Executor->>Worker: Create worker with hmac_key in kwargs
    Worker->>Worker: Store hmac_key parameter
    Worker->>RPC: Call init_rpc_worker(rank, rpc_addr, hmac_key)
    
    RPC->>RPC: Store hmac_key
    RPC->>Server: Create RPCServer with hmac_key
    Server->>Server: use_hmac_encryption = (hmac_key is not None)
    Server->>Server: Setup encrypted channel if key present

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Extra attention needed:
  • tensorrt_llm/executor/rpc_client.py and tensorrt_llm/executor/rpc_server.py: Verify the conditional HMAC encryption logic (use_hmac_encryption=hmac_key) is syntactically correct and handles None values appropriately. Summary mentions potential syntax errors with stray tokens.
  • tensorrt_llm/executor/rpc_worker_mixin.py: Confirm the parameter threading from init_rpc_worker to RPCServer is complete and properly typed throughout the call stack.
  • tests/unittest/executor/test_rpc_proxy.py: Verify the new test adequately covers both key generation and RPC functionality with the injected key.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is incomplete - it only contains the template structure without any actual implementation details, rationale, test coverage information, or checklist verification.	Fill in the Description section explaining what HMAC changes are being made and why, document relevant tests in Test Coverage, and verify all PR checklist items.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title '[None][fix] enable hmac in RPC' clearly summarizes the main change: enabling HMAC in RPC communication across multiple executor files.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (4)

tensorrt_llm/executor/rpc/rpc_client.py (1)

85-96: Add type annotation for hmac_key parameter.

The hmac_key parameter on line 87 lacks a type annotation, which is inconsistent with other parameters in the constructor. This aligns with the project's practice of using type hints for interface documentation.

     def __init__(self,
                  address: str,
-                 hmac_key=None,
+                 hmac_key: Optional[bytes] = None,
                  timeout: Optional[float] = None,
                  num_workers: int = 4):

tensorrt_llm/executor/rpc_worker_mixin.py (1)

1-10: Missing NVIDIA copyright header.

Per coding guidelines, all TensorRT-LLM Open Source Software code files should contain an NVIDIA copyright header that includes the current year at the top.

Add the copyright header at the top of the file:

+# SPDX-FileCopyrightText: Copyright (c) 2022-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
 import asyncio

tests/unittest/executor/test_rpc_proxy.py (1)

131-133: Remove extraneous f-string prefix.

The f-string on line 132 has no placeholders, making the f prefix unnecessary.

             logger_debug(
-                f"[Test] HMAC key test passed: RPC communication successful",
+                "[Test] HMAC key test passed: RPC communication successful",
                 color="green")

tensorrt_llm/executor/rpc_proxy_mixin.py (1)

1-16: Missing NVIDIA copyright header.

Per coding guidelines, all TensorRT-LLM Open Source Software code files should contain an NVIDIA copyright header that includes the current year at the top.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68253d9 and ceebe9f.

📒 Files selected for processing (9)

• tensorrt_llm/executor/ray_executor.py (1 hunks)
• tensorrt_llm/executor/ray_gpu_worker.py (2 hunks)
• tensorrt_llm/executor/rpc/rpc_client.py (1 hunks)
• tensorrt_llm/executor/rpc/rpc_server.py (1 hunks)
• tensorrt_llm/executor/rpc_proxy.py (1 hunks)
• tensorrt_llm/executor/rpc_proxy_mixin.py (2 hunks)
• tensorrt_llm/executor/rpc_worker.py (1 hunks)
• tensorrt_llm/executor/rpc_worker_mixin.py (2 hunks)
• tests/unittest/executor/test_rpc_proxy.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.py

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

**/*.py: The code developed for TensorRT-LLM should conform to Python 3.8+
Indent Python code with 4 spaces; do not use tabs
Always maintain the namespace when importing in Python, even if only one class or function from a module is used (e.g., use from package.subpackage import foo and then foo.SomeClass() instead of from package.subpackage.foo import SomeClass)
Python filenames should use snake_case (e.g., some_file.py)
Python class names should use PascalCase (e.g., class SomeClass)
Python function and method names should use snake_case (e.g., def my_awesome_function():)
Python local variable names should use snake_case, with prefix k for variable names that start with a number (e.g., k_99th_percentile = ...)
Python global variables should use upper snake_case with prefix G (e.g., G_MY_GLOBAL = ...)
Python constants should use upper snake_case (e.g., MY_CONSTANT = ...)
Avoid shadowing variables declared in an outer scope in Python
Initialize all externally visible members of a Python class in the constructor
For Python interfaces that may be used outside a file, prefer docstrings over comments
Python comments should be reserved for code within a function, or interfaces that are local to a file
Use Google style docstrings for Python classes and functions, which can be parsed by Sphinx
Python attributes and variables can be documented inline with type and description (e.g., self.x = 5 followed by """<type>: Description of 'x'""" )
Avoid using reflection in Python when functionality can be easily achieved without reflection
When using try-except blocks in Python, limit the except clause to the smallest set of specific errors possible instead of catching all exceptions
When using try-except blocks in Python to handle multiple possible variable types (duck-typing), keep the body of the try as small as possible and use the else block to implement the logic

Files:

• tests/unittest/executor/test_rpc_proxy.py
• tensorrt_llm/executor/rpc_proxy_mixin.py
• tensorrt_llm/executor/rpc_worker.py
• tensorrt_llm/executor/ray_executor.py
• tensorrt_llm/executor/rpc_proxy.py
• tensorrt_llm/executor/rpc_worker_mixin.py
• tensorrt_llm/executor/rpc/rpc_client.py
• tensorrt_llm/executor/rpc/rpc_server.py
• tensorrt_llm/executor/ray_gpu_worker.py

**/*.{cpp,h,cu,py}

📄 CodeRabbit inference engine (CODING_GUIDELINES.md)

All TensorRT-LLM Open Source Software code files should contain an NVIDIA copyright header that includes the current year at the top

Files:

• tests/unittest/executor/test_rpc_proxy.py
• tensorrt_llm/executor/rpc_proxy_mixin.py
• tensorrt_llm/executor/rpc_worker.py
• tensorrt_llm/executor/ray_executor.py
• tensorrt_llm/executor/rpc_proxy.py
• tensorrt_llm/executor/rpc_worker_mixin.py
• tensorrt_llm/executor/rpc/rpc_client.py
• tensorrt_llm/executor/rpc/rpc_server.py
• tensorrt_llm/executor/ray_gpu_worker.py

🧬 Code graph analysis (2)

tests/unittest/executor/test_rpc_proxy.py (2)

tests/unittest/executor/test_ipc.py (1)

• test_hmac_key_generation (262-278)

tensorrt_llm/llmapi/utils.py (1)

• logger_debug (106-120)

tensorrt_llm/executor/ray_gpu_worker.py (1)

tensorrt_llm/executor/rpc_worker_mixin.py (1)

• init_rpc_worker (28-39)

🪛 Ruff (0.14.7)

tests/unittest/executor/test_rpc_proxy.py

132-132: f-string without any placeholders

Remove extraneous f prefix

(F541)

tensorrt_llm/executor/rpc_worker_mixin.py

30-30: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Pre-commit Check

🔇 Additional comments (13)

tensorrt_llm/executor/rpc/rpc_server.py (1)

108-114: LGTM!

The conditional enabling of HMAC encryption based on self._hmac_key presence is correct. The key is properly passed in the address tuple and the use_hmac_encryption flag appropriately reflects whether encryption should be enabled.

tensorrt_llm/executor/rpc_worker.py (1)

158-161: LGTM!

The HMAC key is correctly extracted from kwargs and passed to RPCServer. Using kwargs.get("hmac_key") provides a safe default of None when the key is not present, maintaining backward compatibility.

tensorrt_llm/executor/ray_executor.py (1)

84-88: LGTM!

The HMAC key injection follows the same pattern as rpc_proxy.py, ensuring consistency across executor implementations. The key is properly injected after init_rpc_executor() generates it and before workers are created with create_workers().

tensorrt_llm/executor/rpc/rpc_client.py (1)

108-114: LGTM!

The conditional enabling of HMAC encryption mirrors the server-side implementation in rpc_server.py. The key is correctly passed in the address tuple and use_hmac_encryption is set based on key presence.

tensorrt_llm/executor/rpc_proxy.py (1)

51-52: Verify HMAC key generation and injection placement in init_rpc_executor().

The pattern of generating an HMAC key and injecting it into worker_kwargs before launching workers is architecturally sound for authenticated RPC communication. However, the original review contains structural issues and lacks verification. Confirm that:

1. init_rpc_executor() properly generates and stores self.hmac_key
2. Lines 51-52 correctly assign self.hmac_key to worker_kwargs['hmac_key']
3. The file includes the required NVIDIA copyright header with current year
4. All naming conventions follow coding guidelines (snake_case for variables, PascalCase for classes)

tensorrt_llm/executor/rpc_worker_mixin.py (2)

28-39: LGTM! HMAC key parameter and storage look correct.

The hmac_key parameter is properly typed as Optional[bytes] with a default of None, maintaining backward compatibility. The key is correctly stored as an instance attribute for later use in start_rpc_server.

---

41-47: LGTM! HMAC key correctly passed to RPCServer.

The hmac_key is properly forwarded to the RPCServer constructor, which aligns with the RPCServer.__init__ signature shown in the relevant code snippets.

tests/unittest/executor/test_rpc_proxy.py (2)

98-109: Good test coverage for HMAC key generation and validation.

The test properly verifies that the HMAC key is automatically generated and has the expected 32-byte length, which matches the os.urandom(32) call in rpc_proxy_mixin.py.

---

111-118: Good identity verification for key consistency.

Using is to verify that both references point to the same object is the correct approach here, ensuring no accidental key duplication occurs during propagation.

tensorrt_llm/executor/rpc_proxy_mixin.py (1)

31-34: LGTM! Secure HMAC key generation and client initialization.

Using os.urandom(32) is the correct approach for generating cryptographically secure random bytes for HMAC keys. The 32-byte (256-bit) key size is appropriate for HMAC-SHA256. The key is properly passed to RPCClient for establishing authenticated RPC channels.

tensorrt_llm/executor/ray_gpu_worker.py (3)

159-172: LGTM! HMAC key parameter correctly added to RayGPUWorker.

The hmac_key parameter is properly typed as Optional[bytes] with a default of None, maintaining backward compatibility with existing code that doesn't use HMAC authentication.

---

192-196: LGTM! HMAC key correctly forwarded to init_rpc_worker.

The hmac_key is properly passed to init_rpc_worker, which aligns with the signature in rpc_worker_mixin.py.

---

40-74: Verify HMAC key propagation in RayWorkerWrapper.

The review comment references an AI summary indicating that RayWorkerWrapper.__init__ should accept and propagate hmac_key, but the provided code snippet at lines 40-74 shows no hmac_key parameter. If RayWorkerWrapper is used to instantiate workers that need HMAC-enabled RPC, this key should be passed through worker_kwargs to the underlying worker.

[Superjomn]

/bot run

[tensorrt-cicd]

PR_Github #27181 [ run ] triggered by Bot. Commit: ceebe9f

[tensorrt-cicd]

PR_Github #27125 [ run ] completed with state ABORTED. Commit: ceebe9f
LLM/main/L0_MergeRequest_PR #20695 (Blue Ocean) completed with status: ABORTED

[tensorrt-cicd]

PR_Github #27181 [ run ] completed with state SUCCESS. Commit: ceebe9f
/LLM/main/L0_MergeRequest_PR pipeline #20743 completed with status: 'SUCCESS'