Re: [PATCH net-next v2 2/3] tcp_hybla: fix divide by zero on rtt0 == 0

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In net/ipv4/tcp_hybla.c, the patch replaces `module_param(rtt0,
  int, 0644)` with `module_param_cb(rtt0, &rtt0_ops, &rtt0, 0644)`.

--

tcp_hybla: fix divide by zero on rtt0 == 0

This commit changes the rtt0 module parameter from int to unsigned int.
It also registers rtt0 through module_param_cb() with a setter that only
accepts values from 1 to U32_MAX / USEC_PER_MSEC. That stops
hybla_recalc_param() from dividing by zero, and stops rtt0 * USEC_PER_MSEC
from wrapping on 32-bit.

> diff --git a/net/ipv4/tcp_hybla.c b/net/ipv4/tcp_hybla.c
> index abd7d91807e54..b9b482180d30b 100644
> --- a/net/ipv4/tcp_hybla.c
> +++ b/net/ipv4/tcp_hybla.c
> @@ -26,8 +26,20 @@ struct hybla {

[ ... ]

> +static const struct kernel_param_ops rtt0_ops = {
> +	.set = rtt0_set,
> +	.get = param_get_uint,
> +};
> +
> +module_param_cb(rtt0, &rtt0_ops, &rtt0, 0644);
>  MODULE_PARM_DESC(rtt0, "reference rout trip time (ms)");

[Severity: Low]
Does this drop the parmtype entry for rtt0 from the .modinfo section of
tcp_hybla.ko?

The old module_param() line expanded to module_param_named(), which emits
the type tag:

include/linux/moduleparam.h:
#define module_param_named(name, value, type, perm)			   \
	param_check_##type(name, &(value));				   \
	module_param_cb(name, &param_ops_##type, &value, perm);		   \
	__MODULE_PARM_TYPE(name, #type)

module_param_cb() only expands to __module_param_call():

#define module_param_cb(name, ops, arg, perm)				      \
	__module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0)

MODULE_PARM_DESC(rtt0, ...) is still there, but nothing provides a type.
After this change, modinfo tcp_hybla would show a description for rtt0
with no type suffix.

Since the storage is now unsigned int and it is read back with
param_get_uint, could a line like this be added after module_param_cb()?

__MODULE_PARM_TYPE(rtt0, "uint");

Other module_param_cb() users already do this, for example
arch/x86/kvm/mmu/mmu.c, fs/fuse/inode.c and
drivers/scsi/fcoe/fcoe_transport.c.

>  
>  /* This is called to refresh values for hybla parameters */

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001112948.322463-1-jiayuan.chen%40linux.dev