netdev - Re: [PATCH net v3 1/6] usbnet: ipheth: break up NCM header size computation

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: <5be4f8f1-af40-4114-963e-76f645380081@pen.gy>
Date: Sun, 1 Dec 2024 23:08:04 +0100
From: Foster Snowhill <forst@....gy>
To: Paolo Abeni <pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>
Cc: Georgi Valkov <gvalkov@...il.com>, Simon Horman <horms@...nel.org>,
 Oliver Neukum <oneukum@...e.com>, netdev@...r.kernel.org,
 linux-usb@...r.kernel.org
Subject: Re: [PATCH net v3 1/6] usbnet: ipheth: break up NCM header size
 computation

On 2024-11-28 10:16, Paolo Abeni wrote:
> On 11/24/24 00:54, Foster Snowhill wrote:
>> Originally, the total NCM header size was computed as the sum of two
>> vaguely labelled constants. While accurate, it's not particularly clear
>> where they're coming from.
>>
>> Use sizes of existing NCM structs where available. Define the total
>> NDP16 size based on the maximum amount of DPEs that can fit into the
>> iOS-specific fixed-size header.
>>
>> Fixes: a2d274c62e44 ("usbnet: ipheth: add CDC NCM support")
>> Signed-off-by: Foster Snowhill <forst@....gy>
> 
> This change is not addressing any real issue, it just makes the
> following ones simpler, right?
> 
> If so, I think it's better to drop the fixes tag here and add the above
> reasoning.

Correct, this doesn't fix any real issue. It has two purposes:

* Make it clearer for the reader where the numeric constants come from.
* Like you said, make subsequent changes simpler by introducing intermediate
constants that are used by subsequent patches.

Ack, will remove the "Fixes" tag and add the above description to justify
the change.

>> ---
>> Each individual patch in the v3 series tested with iPhone 15 Pro Max,
>> iOS 18.1.1: compiled cleanly, ran iperf3 between phone and computer,
>> observed no errors in either kernel log or interface statistics.
> 
> This should go in the cover letter (currently missing, please add it in
> the next iteration).

Agreed, will add a cover letter for v4. Depending on the outcome of the
comments/discussion on patch 4/6 in the series ("usbnet: ipheth: use static
NDP16 location in URB") will likely reiterate the explanation of why I
approached the changes the way I did. I think it provides important context,
and also points at a potential way to enhance the driver to make it more
flexible in case of possible future changes to iOS.