netdev - Re: [PATCH net-next v2 3/3] net: dsa: realtek: support reset controller

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: Wed, 1 Nov 2023 16:55:19 -0300
From: Luiz Angelo Daros de Luca <luizluca@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: netdev@...r.kernel.org, linus.walleij@...aro.org, alsi@...g-olufsen.dk, 
	andrew@...n.ch, vivien.didelot@...il.com, f.fainelli@...il.com, 
	davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, robh+dt@...nel.org, 
	krzk+dt@...nel.org, arinc.unal@...nc9.com
Subject: Re: [PATCH net-next v2 3/3] net: dsa: realtek: support reset controller

Hi Vladimir,

> realtek-mdio is an MDIO driver while realtek-smi is a platform driver
> implementing a bitbang protocol. They might never be used together in
> a system to share RAM and not even installed together in small
> systems. If I do need to share the code, I would just link it twice.
> Would something like this be acceptable?
>
> drivers/net/dsa/realtek/Makefile
> -obj-$(CONFIG_NET_DSA_REALTEK_MDIO)     += realtek-mdio.o
> -obj-$(CONFIG_NET_DSA_REALTEK_SMI)      += realtek-smi.o
> +obj-$(CONFIG_NET_DSA_REALTEK_MDIO)     += realtek-mdio.o realtek_common.o
> +obj-$(CONFIG_NET_DSA_REALTEK_SMI)      += realtek-smi.o realtek_common.o

Just a follow up.

It is not that simple to include a .c file into an existing single
file module. It looks like you need to rename the original file as all
linked objects must not conflict with the module name. The kernel
build seems to create a new object file for each module. Is there a
clearer way? I think #include a common .c file would not be
acceptable.

I tested your shared module suggestion. It is the clearest one but it
also increased the overall size quite a bit. Even linking two objects
seems to use the double of space used by the functions alone. These
are some values (mips)

drivers/net/dsa/realtek/realtek_common.o  5744  without exports
drivers/net/dsa/realtek/realtek_common.o 33472  exporting the two
reset functions (assert/deassert)

drivers/net/dsa/realtek/realtek-mdio.o   18756  without the reset
funcs (to be used as a module)
drivers/net/dsa/realtek/realtek-mdio.o   20480  including the
realtek_common.c (#include <realtek_common.c>)
drivers/net/dsa/realtek/realtek-mdio.o   22696  linking the realtek_common.o

drivers/net/dsa/realtek/realtek-smi.o    30712  without the reset
funcs (to be used as a module)
drivers/net/dsa/realtek/realtek-smi.o    34604  linking the realtek_common.o

drivers/net/dsa/realtek/realtek-mdio.ko  28800  without the reset
funcs (it will use realtek_common.ko)
drivers/net/dsa/realtek/realtek-mdio.ko  32736  linking the realtek_common.o

drivers/net/dsa/realtek/realtek-smi.ko   40708  without the reset
funcs (it will use realtek_common.ko)
drivers/net/dsa/realtek/realtek-smi.ko   44612  linking the realtek_common.o

In summary, we get about 1.5kb of code with the extra functions,
almost 4kb if we link a common object containing the functions and
33kb if we use a module for those two functions.

I can go with any option. I just need to know which one would be
accepted to update my patches.
1) keep duplicated functions on each file
2) share the code including the .c on both
3) share the code linking a common object in both modules (and
renaming existing .c files)
4) create a new module used by both modules.

The devices that would use this driver have very restricted storage
space. Every kbyte counts.

Regards,

Luiz