Re: [PATCH] net: avoid assigning ethaddr to wrong devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 26, 2018 at 09:04:02AM +0300, Nikita Yushchenko wrote:
> >>>> It can happen that device tree contains ethernetN alias pointing to
> >>>> valid device, but that device is not supported by [running instance of]
> >>>> barebox. Then ethN remains unassigned, and can be later captured by
> >>>> dynamically registered device such as usbnet.
> >>>>
> >>>> For such "stranger" device, ethaddr preconfigured for ethN should not be
> >>>> assigned. Also, ethaddr of such device should not be written to
> >>>> ethernetN node of device tree passed to kernel being booted.
> >>>>
> >>>
> >>> There's only one usecase for matching edev->dev.id against the ethernetx
> >>> alias which has been introduced with:
> >>>
> >>> | commit a78431c7fc42193be252417bf06f7cc61765a51e
> >>> | Author: Renaud Barbier <renaud.barbier@xxxxxx>
> >>> | Date:   Wed Sep 4 08:37:03 2013 +0200
> >>> | 
> >>> |     net, of: fixup MAC address by alias
> >>> |     
> >>> |     If a network device has not been registered from the devicetree, we may
> >>> |     still find it by its alias in the devicetree. This way also platform based
> >>> |     network devices can obtain a valid MAC address in the devicetree.
> >>> |     
> >>> |     Signed-off-by: Renaud Barbier <renaud.barbier@xxxxxx>
> >>> |     Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> >>>
> >>> Your eth_is_stranger() returns true for the devices that Renaud wanted
> >>> to support, so instead of applying your patch we could equally well
> >>> revert that from Renaud.
> >>>
> >>> I don't have a good idea right now how to fix this. Maybe we have to
> >>> make sure that ethernet devices from dynamic buses never get an id
> >>> asigned that is also present in the aliases node.
> >>
> >> eth_is_stranger() for ethN returns true only if ethernetN alias exists
> >> AND ethN either does not have device tree node, or has node different
> >> from what is pointed by alias.
> >>
> >> My assumption was that if under linux ethdevice is configured via device
> >> tree node with ethernetX alias, then under barebox it should also be
> >> configured via device tree node with same alias.
> >>
> >> You mean, there is hardware that breaks this assumption?
> >> Which hardware it is?
> > 
> > It's PowerPC hardware which on barebox is not probed from devicetree, so
> > indeed there is no device node.
> 
> But if no device tree, then
>   alias = of_find_node_by_alias(of_get_root_node(), eth);
> should return NULL, and eth_is_stranger() should return false, thus
> making my patch no-op?

Ok, you are right. I misread the code. You call of_find_node_by_alias()
on the internal device tree, not the one the Kernel is started with, so
indeed eth_is_stranger() should return false.

Nevertheless I do not like this patch very much as it adds more code to
a place that is already hard to understand in all of its consequences.

I would like to explore the route that we assign these dynamic devices
an id that is not present in any alias node. That could be done by
searching for the highest alias number and give the dynamic devices one
number higher. Would that be doable?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox



[Index of Archives]     [Linux Embedded]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux