[PATCH v4 05/20] net/txgbe: fix inaccuracy in Tx rate limiting

[Zaiyu Wang]

> AI feedback on this one. Looks ok as is but worth looking at.
Thank you for the review. Based on the feedback, I have made the following adjustments:
> # DPDK Patch Review: net/txgbe: fix inaccuracy in Tx rate limiting
> ### **Errors**
> 
> None found.
> 
> ### **Warnings**
> 
> **1. TDM_RL_ADJ write placement causes per-queue reconfiguration**
No code change
`txgbe_set_queue_rate_limit()` is directly exposed as the
`eth_dev_ops.set_queue_rate_limit`  to upper-layer applications. We cannot expect
applications to perform any additional setup before calling this interface, so the driver
must reconfig.
Writing the same value to this register multiple times is harmless. Therefore I prefer to
keep the current approach.

> **2. Magic number 21 lacks explanation**
> 
> The value 21 is documented in the commit message but not in the code. Future
> maintainers will not see the breakdown (7B preamble + 1B SFD + 1B EFD + 12B
> IPG).
> 
> **Suggested fix:** Add a comment or define:
> 
> ```c
> /* Ethernet framing overhead: 7B preamble + 1B SFD + 1B EFD + 12B IPG */
> #define TXGBE_TDM_RL_ADJ_OVERHEAD  21 wr32(hw, TXGBE_TDM_RL_ADJ,
> TXGBE_TDM_RL_ADJ_OVERHEAD); ```
> 
I have added a macro `TXGBE_FRAME_OVERHEAD`.

> **3. Missing release notes update**
> 
> This is a fix that changes hardware behavior and improves accuracy. Users may
> observe different TX rates after upgrading. The change should be documented in
> release notes to inform users of the accuracy improvement.
> 
> **Suggested fix:** Add an entry to `doc/guides/rel_notes/release_26_11.rst` (or
> current release):
> 
> ```rst
> * **net/txgbe: Improved TX rate limiting accuracy for Amber-lite NIC.**
> 
>   Fixed TX rate limiting accuracy for 64-byte packets from ~20% deviation
>   to within ~5% by correcting hardware framing overhead configuration.
> ```
> 
> ### **Info**
> 
> **1. Consider validating tx_rate is non-zero**
> 
> The code computes `factor_int = link_speed / tx_rate`. If `tx_rate` is somehow zero
> (due to caller error or invalid input), this would cause division by zero. The existing
> code may have validation elsewhere, but defensive programming would check
> this.
> 
> ```c
> if (tx_rate == 0) {
>     PMD_DRV_LOG(ERR, "TX rate cannot be zero");
>     return -EINVAL;
> }
> ```
No code change
In this function, `tx_rate == 0` is a valid input meaning "remove rate limiting and
restore unlimited mode". No additional check is needed.> 
> **2. Variable scope: factor_fra could be declared at initialization**
> 
> The variable `factor_fra` is declared but not initialized, then assigned in both
> branches of the conditional. C99 style would allow:
> 
> ```c
> u32 factor_fra = (frac * 16384) / 10000; if (factor_fra >= 8192)
>     factor_fra = 0;
> ```
> 
> This is stylistic only; the existing code is correct.
> 
No code change
Just leave it as is.