Messages in this thread | | | Date | Wed, 19 Oct 2022 11:04:33 -0400 | Subject | Re: [PATCH net] net: fman: Use physical address for userspace interfaces | From | Sean Anderson <> |
| |
On 10/19/22 02:46, Geert Uytterhoeven wrote: > Hi Madalin, > > On Wed, Oct 19, 2022 at 7:20 AM Madalin Bucur <madalin.bucur@nxp.com> wrote: >>> -----Original Message----- >>> From: Sean Anderson <sean.anderson@seco.com> >>> Sent: 19 October 2022 00:47 >>> To: Andrew Lunn <andrew@lunn.ch>; Andrew Davis <afd@ti.com> >>> Cc: David S . Miller <davem@davemloft.net>; netdev@vger.kernel.org; >>> linux-kernel@vger.kernel.org; Madalin Bucur <madalin.bucur@nxp.com>; >>> Jakub Kicinski <kuba@kernel.org>; Eric Dumazet <edumazet@google.com>; >>> Paolo Abeni <pabeni@redhat.com>; Camelia Alexandra Groza >>> <camelia.groza@nxp.com>; Geert Uytterhoeven <geert@linux-m68k.org> >>> Subject: Re: [PATCH net] net: fman: Use physical address for userspace >>> interfaces >>> >>> >>> >>> On 10/18/22 5:39 PM, Andrew Lunn wrote: >>>> On Tue, Oct 18, 2022 at 01:33:55PM -0500, Andrew Davis wrote: >>>>> On 10/18/22 12:37 PM, Sean Anderson wrote: >>>>>> Hi Andrew, >>>>>> >>>>>> On 10/18/22 1:22 PM, Andrew Lunn wrote: >>>>>>> On Mon, Oct 17, 2022 at 12:28:06PM -0400, Sean Anderson wrote: >>>>>>>> For whatever reason, the address of the MAC is exposed to >>> userspace in >>>>>>>> several places. We need to use the physical address for this >>> purpose to >>>>>>>> avoid leaking information about the kernel's memory layout, and >>> to keep >>>>>>>> backwards compatibility. >>>>>>> >>>>>>> How does this keep backwards compatibility? Whatever is in user >>> space >>>>>>> using this virtual address expects a virtual address. If it now >>> gets a >>>>>>> physical address it will probably do the wrong thing. Unless there >>> is >>>>>>> a one to one mapping, and you are exposing virtual addresses >>> anyway. >>>>>>> >>>>>>> If you are going to break backwards compatibility Maybe it would >>> be >>>>>>> better to return 0xdeadbeef? Or 0? >>>>>>> >>>>>>> Andrew >>>>>>> >>>>>> >>>>>> The fixed commit was added in v6.1-rc1 and switched from physical to >>>>>> virtual. So this is effectively a partial revert to the previous >>>>>> behavior (but keeping the other changes). See [1] for discussion. >>>> >>>> Please don't assume a reviewer has seen the previous >>>> discussion. Include the background in the commit message to help such >>>> reviewers. > >>>>> I see it asked in that thread, but not answered. Why are you exposing >>>>> "physical" addresses to userspace? There should be no reason for that. >>>> >>>> I don't see anything about needing physical or virtual address in the >>>> discussion, or i've missed it. >>> >>> Well, Madalin originally added this, so perhaps she has some insight. >>> >>> I have no idea why we set the IFMAP stuff, since that seems like it's for >>> PCMCIA. Not sure about sysfs either. >>> >>>> If nobody knows why it is needed, either use an obfusticated value, or >>>> remove it all together. If somebody/something does need it, they will >>>> report the regression. >>> >>> I'd rather apply this (or v2 of this) and then remove the "feature" in >>> follow-up. >>> >>> --Sean >> >> >> root@localhost:~# grep 1ae /etc/udev/rules.d/72-fsl-dpaa-persistent-networking.rules >> SUBSYSTEM=="net", DRIVERS=="fsl_dpa*", ATTR{device_addr}=="1ae0000", NAME="fm1-mac1" >> SUBSYSTEM=="net", DRIVERS=="fsl_dpa*", ATTR{device_addr}=="1ae2000", NAME="fm1-mac2" >> SUBSYSTEM=="net", DRIVERS=="fsl_dpa*", ATTR{device_addr}=="1ae4000", NAME="fm1-mac3" >> SUBSYSTEM=="net", DRIVERS=="fsl_dpa*", ATTR{device_addr}=="1ae6000", NAME="fm1-mac4" >> SUBSYSTEM=="net", DRIVERS=="fsl_dpa*", ATTR{device_addr}=="1ae8000", NAME="fm1-mac5" >> SUBSYSTEM=="net", DRIVERS=="fsl_dpa*", ATTR{device_addr}=="1aea000", NAME="fm1-mac6" > > So you rely on the physical address. > It's a pity this uses a custom sysfs file. > Can't you obtain this information some other way? > Anyway, as this is in use, it became part of the ABI.
In my udev rules I use ID_PATH. Since this is a devicetree platform, the path name includes the device address by convention.
--Sean
>> root@localhost:~# grep 1ae /sys/devices/platform/soc/soc:fsl,dpaa/soc:fsl,dpaa:ethernet@*/net/fm1-mac*/device_addr >> /sys/devices/platform/soc/soc:fsl,dpaa/soc:fsl,dpaa:ethernet@2/net/fm1-mac3/device_addr:1ae4000 >> /sys/devices/platform/soc/soc:fsl,dpaa/soc:fsl,dpaa:ethernet@3/net/fm1-mac4/device_addr:1ae6000 >> /sys/devices/platform/soc/soc:fsl,dpaa/soc:fsl,dpaa:ethernet@4/net/fm1-mac5/device_addr:1ae8000 >> /sys/devices/platform/soc/soc:fsl,dpaa/soc:fsl,dpaa:ethernet@5/net/fm1-mac6/device_addr:1aea000 > > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
| |