linux-kernel - Re: [RFC PATCH v2 11/12] greybus: cpc: honour remote's RX window

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <DE8P9HDNEZJH.33TRDMVZI1L55@silabs.com>
Date: Fri, 14 Nov 2025 15:28:38 -0500
From: "Yacin Belmihoub-Martel" <yacin.belmihoub-martel@...abs.com>
To: Damien Riégel <damien.riegel@...abs.com>,
        <greybus-dev@...ts.linaro.org>, "Johan Hovold" <johan@...nel.org>,
        "Alex
 Elder" <elder@...nel.org>,
        "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>,
        <linux-kernel@...r.kernel.org>
Cc: "Silicon Labs Kernel Team" <linux-devel@...abs.com>
Subject: Re: [RFC PATCH v2 11/12] greybus: cpc: honour remote's RX window

On Fri Nov 14, 2025 at 10:07 AM EST, Damien Riégel wrote:
> +/**
> + * cpc_header_get_frames_acked_count() - Get frames to be acknowledged.
> + * @seq: Current sequence number of the endpoint.
> + * @ack: Acknowledge number of the received frame.
> + *
> + * Return: Frames to be acknowledged.
> + */
> +u8 cpc_header_get_frames_acked_count(u8 seq, u8 ack)
> +{
> +	u8 frames_acked_count;
> +
> +	/* Find number of frames acknowledged with ACK number. */
> +	if (ack > seq) {
> +		frames_acked_count = ack - seq;
> +	} else {
> +		frames_acked_count = 256 - seq;
> +		frames_acked_count += ack;
> +	}
> +
> +	return frames_acked_count;
> +}

There is no need to check whether `ack > seq` since the return value in
downcasted to a `u8`. For example, if `ack=0` and `seq=254`, we can
simply do `(u8)(0-254)=2`.

> +static void __cpc_protocol_receive_ack(struct cpc_cport *cport, u8 recv_wnd, u8 ack)
> +{
> +	struct gb_host_device *gb_hd = cport->cpc_hd->gb_hd;
> +	struct sk_buff *skb;
> +	u8 acked_frames;
> +
> +	cport->tcb.send_wnd = recv_wnd;

Little bit of a nitpick, but handling the RX window update in
`__cpc_protocol_receive_ack` seems a bit misleading to me, in the sense
that the ACK itself is not directly related to the window. 

I would either rename the function to indicate that we are handling all 
the CPort's metadata received from the CPC header, or I would move the 
RX window update to the caller.

Thanks,
-- 
Yacin Belmihoub-Martel
Silicon Laboratories