On Mon, 13 Jan 2025 02:48:58 +0100 Foster Snowhill wrote:
> Thank you very much for the review!
>
> I went through the series again, noticed a couple minor things I think
> I should fix:
>
> * Patch 1/7 ("usbnet: ipheth: break up NCM header size computation")
> [p1] introduces two new preprocessor constants. Only one of them is
> used (the other one is intermediate, for clarity), and the usage is
> all the way in patch 6/7 ("usbnet: ipheth: fix DPE OoB read") [p6].
> I'd like to move the constant introduction patch right before the
> patch that uses one of them. There's no good reason they're spread
> out like they are in v4.
> * Commit message in patch 5/7 ("usbnet: ipheth: refactor NCM datagram
> loop") [p5] has a stray paragraph starting with "Fix an out-of-bounds
> DPE read...". This needs to be removed.
>
> I'd like to get this right. I'll make the changes above, add Cc stable,
> re-test all patches in sequence, and submit v5 soon. As this will be
> a different revision, I figure I can't formally apply your "Reviewed-by"
> anymore, the series may need another look once I post v5.
The opinions on the exact rules differ but you can definitely add my tag
on the patches which won't change.
> Also I have some doubts about patch 7/7 [p7] with regards to its
> applicability to backporting to older stable releases. This only adds a
> documentation comment, without fixing any particular issue. Doesn't
> sound like something that should go into stable. But maybe fine if it's
> part of a series?
Yes, it's fine as part of the series.
> I can also add that text in a commit message rather
> than the source code of the driver itself, or even just keep it in the
> cover letter. Do you have any opinion on this?
Maybe it's because I don't work with USB networking much but to me
the comment was useful.
