Thank you for your contribution! Sashiko AI review found 3 potential issue(s)
to consider:
- [High] The proposed design violates kernel architecture by bypassing the
standard networking stack and misusing the compute accelerator (accel/DRM)
subsystem for network packet processing and ACL offloads.
- [Low] Usage of deprecated, non-inclusive terminology ('blacklisted').
- [Low] Grammatical error in the documentation regarding command pluralization.
--
commit 76a459f24739ffcc1e2f6fc248380fee0d5af75f
Author: Jaspinder Budhal <[email protected]>
Documentation: accel: Add NPAC client driver documentation
This commit adds documentation for the Texas Instruments NPAC (Network Packet
ACcelerator) client driver. It details the accelerator's purpose of offloading
network packet processing tasks like ACL rule enforcement. The documentation
also outlines its userspace interface and RPMsg communication model.
> diff --git a/Documentation/accel/npac.rst b/Documentation/accel/npac.rst
> new file mode 100644
> index 0000000000000..fba6ddd0cfd2c
> --- /dev/null
> +++ b/Documentation/accel/npac.rst
[ ... ]
> +When the NPAC host driver announces the NPAC RPMsg service endpoint, the
> +RPMsg core matches the announced service against the registered NPAC client
> +driver and invokes the driver probe callback.
> +
> +Offload command received from userspace are formatted into RPMsgs and sent
[Severity: Low]
This isn't a bug, but there is a grammatical error here. Should "command" be
pluralized to "commands" to match the plural verb "are"?
> +to the NPAC Host Driver.
> +
> +NPAC Host Driver responses are received through RPMsg callbacks.
> +
> +ACL Rule Offload
> +----------------
> +
> +ACL (Access Control List) rules are packet filtering policies that define how
> +network traffic should be handled — for example, dropping packets from a
> +blacklisted source IP or allowing traffic only from trusted MAC addresses.
[Severity: Low]
This isn't a bug, but the Linux kernel coding style guidelines recommend using
inclusive terminology. Could "blacklisted" be replaced with "blocklisted" or
"denylisted"?
[ ... ]
> +IOCTL Commands
> +~~~~~~~~~~~~~~
> +
> +DRM_IOCTL_NPAC_ACL_ADD_RULE
> +
> + This IOCTL allows userspace to add an ACL rule to the accelerator. The
[Severity: High]
Does this UAPI design bypass the established Linux networking subsystem APIs
for packet filtering?
The accel subsystem is generally intended for generic compute, ML, and AI
workloads. Introducing a custom UAPI for network packet filtering via
/dev/accel/accel* and DRM_IOCTL_NPAC_ACL_ADD_RULE bypasses standard
interfaces like tc-flower (via ndo_setup_tc) or netfilter hardware offload.
This custom interface would prevent standard networking utilities such as tc
or nftables from managing these ACLs, and it removes the standard software
fallback path. Should this driver integrate with the standard networking stack
instead of implementing custom DRM IOCTLs?
> + call will forward the rule to the NPAC host driver for hardware
> programming.
> + The NPAC Host Driver is responsible for validating the rule and applying
> it to
> + the NPAC hardware.
--
Sashiko AI review ·
https://sashiko.dev/#/patchset/[email protected]?part=1
