Hi Mark, Fernando, On 9/13/23 18:41, Mark Pearson wrote: > > > On Wed, Sep 13, 2023, at 11:58 AM, Hans de Goede wrote: >> Hi Fernando, >> >> On 9/6/23 21:52, Fernando Eckhardt Valle wrote: >>> Newer Thinkpads have a feature called Mac Address Passthrough. >>> This patch provides a sysfs interface that userspace can use >>> to get this auxiliary mac address. >>> >>> Signed-off-by: Fernando Eckhardt Valle <fevalle@xxxxxx> >> >> Thank you for your patch. >> >> At a minimum for this patch to be accepted you will need >> to document the new sysfs interface in: >> >> Documentation/admin-guide/laptops/thinkpad-acpi.rst >> >> But I wonder if we should export this information to >> userspace in this way ? >> >> The reason why I'm wondering is because mac-address passthrough >> in case of using e.g. Lenovo Thunderbolt docks is already >> supported by the kernel by code for this in drivers/net/usb/r8152.c : >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/usb/r8152.c#n1613 >> >> So I'm wondering if we really need this, is there a planned >> userspace API consumer of the new sysfs interface ? >> >> Or is this only intended as a way for a user to query this, iow >> is this purely intended for informational purposes ? >> > Hi Hans, > > We've previously had strong pushback from the maintainers in the net tree that the MAC passthru should not be done there and should be done in user-space. I'd have to dig up the threads, but there was a preference for it to not be done in the kernel (and some frustrations at having vendor specific changes in the net driver). > > We've also seen various timing issues (some related to ME FW doing it's thing) that makes it tricky to handle in the kernel - with added delays being needed leading to patches that can't be accepted. > > This approach is one of the steps towards fixing this. Fernando did discuss and review this with me beforehand (apologies - I meant to add a note saying I'd been involved). If you think there is a better approach please let us know, but we figured as this is a Lenovo specific thing it made sense to have it here in thinkpad_acpi. > > There will be a consumer (I think it's a script and udev rule) to update the MAC if a passthru-MAC address is provided via the BIOS. This will be open-source, but we haven't really figured out how to release it yet. > > Fernando - please correct anything I've gotten wrong! Ah that is all good to know. That pretty much takes care of my objections / answers my questions. Fernando can you please submit a v2 which: 1. Adds documentation as mentioned already 2. Moves the special handling of "XXXXXXXXXXXX" from show() to init() (writing to auxmac[] in show() is a bit weird, also we only need to do this once, so it is init code) Regards, Hans
