Re: [PATCH net-next 1/7] uapi: if_link: Add RMNET_FLAGS_INGRESS_COALESCE

[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] Retained for independent validation only; my baseline decision is
  FALSE.

--

uapi: if_link: Add RMNET_FLAGS_INGRESS_COALESCE

This commit adds bit 6 to the rmnet ingress flags in the uAPI header. The
bit lets rmnet process incoming packets in the MAPv5 coalescing header
format.

> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 245b362045253..349fcddc6622f 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -2022,6 +2022,7 @@ enum {
>  #define RMNET_FLAGS_EGRESS_MAP_CKSUMV4            (1U << 3)
>  #define RMNET_FLAGS_INGRESS_MAP_CKSUMV5           (1U << 4)
>  #define RMNET_FLAGS_EGRESS_MAP_CKSUMV5            (1U << 5)
> +#define RMNET_FLAGS_INGRESS_COALESCE              (1U << 6)

[Severity: Low]
This is a minor point about bisection. Nothing in the kernel reads the new
bit at this commit, so it is accepted without any effect.

rmnet_newlink() and rmnet_changelink() in
drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c merge the requested
flags without checking them:

rmnet_changelink() {
    ...
		data_format = old_data_format & ~flags->mask;
		data_format |= flags->flags & flags->mask;
		WRITE_ONCE(port->data_format, data_format);
    ...
}

rmnet_fill_info() then reports port->data_format back. The bit shows as
enabled, but __rmnet_map_ingress_handler() and
rmnet_map_process_next_hdr_packet() never check it at this point.

Later in the series, "net: qualcomm: rmnet: Add DL packet coalescing
support" adds the code that reads this bit in the ingress path and in
rmnet_map_data.c. It also adds rmnet_config_data_format_valid(), which
rejects RMNET_FLAGS_INGRESS_COALESCE when RMNET_FLAGS_INGRESS_MAP_CKSUMV5
is not set. So the gap only exists in the middle of the series.

Would it make sense to fold this define into that patch? The bit would
then never be accepted by a kernel that ignores it.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930051345.857443-1-subash.a.kasiviswanathan%40oss.qualcomm.com