netdev - Re: [PATCH net-next RFC v4 3/5] dt-bindings: net: phy: add timestamp preferred choice property

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]

Message-ID: <20230511135631.dzszb4zen26mtsqq@skbuf>
Date: Thu, 11 May 2023 16:56:31 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Köry Maincent <kory.maincent@...tlin.com>
Cc: netdev@...r.kernel.org, kuba@...nel.org, glipus@...il.com,
	maxime.chevallier@...tlin.com, vadim.fedorenko@...ux.dev,
	richardcochran@...il.com, gerhard@...leder-embedded.com,
	thomas.petazzoni@...tlin.com, krzysztof.kozlowski+dt@...aro.org,
	robh+dt@...nel.org, linux@...linux.org.uk
Subject: Re: [PATCH net-next RFC v4 3/5] dt-bindings: net: phy: add timestamp
 preferred choice property

On Thu, May 11, 2023 at 03:25:50PM +0200, Köry Maincent wrote:
> The user may not and don't need to know which hardware timestamping is better.
> He just want to use the best one by default without investigation and
> benchmarking.
> It is more related to the hardware design of the board which should be
> described in the devicetree, don't you think? Of course it should not break
> anything and if it does, well then let the user select it in userspace.
> But if you really think my point is irrelevant then I will drop this feature.

You are putting an equality sign between user space and the end-user of
a system. A user space distribution has a lot of configuration files
where the end user isn't expected to know how to configure them all, and
that's not an argument for putting them in the kernel/device tree, is it?

I can relate to 2 examples which are closer to what I know slightly
better (Documentation/devicetree/bindings/net/dsa/dsa-port.yaml).

One is "label" (the netdev name of a switch port). It has been argued
that we should deprecate this, because udev permits selecting specific
netdev named based on hardware properties already.

I agree with this, and this is why on NXP LS1028A, we don't use "label"
in the device tree, but advise people to ship this in the rootfs:

cat /etc/udev/rules.d/10-network.rules
# ENETC rules
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.0", DRIVERS=="fsl_enetc", NAME:="eno0"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.1", DRIVERS=="fsl_enetc", NAME:="eno1"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.2", DRIVERS=="fsl_enetc", NAME:="eno2"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.6", DRIVERS=="fsl_enetc", NAME:="eno3"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:01.0", DRIVERS=="fsl_enetc_vf", NAME:="eno0vf0"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:01.1", DRIVERS=="fsl_enetc_vf", NAME:="eno0vf1"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:01.2", DRIVERS=="fsl_enetc_vf", NAME:="eno1vf0"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:01.3", DRIVERS=="fsl_enetc_vf", NAME:="eno1vf1"
# LS1028 switch rules
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p0", NAME="swp0"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p1", NAME="swp1"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p2", NAME="swp2"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p3", NAME="swp3"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p4", NAME="swp4"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p5", NAME="swp5"

The other example is "dsa-tag-protocol", which you'd normally expect
that user space would select through /sys/class/net/ethN/dsa/tagging and
that would be the end of the story. I was only convinced to let it live
in the device tree because a tagging protocol change might be necessary
for traffic on the port to work at all, and if traffic doesn't work,
then the kernel can't load userspace through e.g. NFS, and thus, user
space can't change this setting. In your case, I don't think this
argument applies.

I guess the general rule of thumb is - if a functionality can live outside
the kernel or of the device tree, it's probably better that it does.