netdev - Re: [PATCH net-next RFC 0/5] Update r8152 to version two

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Nov 2022 22:03:36 +0100
From:   Francois Romieu <romieu@...zoreil.com>
To:     Albert Zhou <albert.zhou.50@...il.com>
Cc:     gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
        nic_swsd@...ltek.com, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
        Hayes Wang <hayeswang@...ltek.com>
Subject: Re: [PATCH net-next RFC 0/5] Update r8152 to version two

Albert Zhou <albert.zhou.50@...il.com> :
> This patch integrates the version-two r8152 drivers from Realtek into
> the kernel. I am new to kernel development, so apologies if I make
> newbie mistakes.

While it makes sense to minimize differences betwenn Realtek's in-house
driver and kernel r8152 driver, it does not mean that the out-of-tree
driver is suitable for a straight kernel inclusion.

If you want things to move forward in a not too painful way, you should
split the more than 650 ko patch into smaller, more focused patches
(huge patches also makes bisection mildly effective btw).

In its current form, the submission is imho a bit abrasive to review.

[...]
> Albert Zhou (5):
>   net: move back netif_set_gso_max helpers
>   r8152: update to version two

This code misuses mutex in {read, write}_mii_word.

It includes code and data that should be moved to firmware files.

>   r8152: remove backwards compatibility

Backwards compatibility code should had been avoided in the first
place.

[...]
>   r8152: remove redundant code

Same thing.

-- 
Ueimor