* [PATCH] net: Set the actual ethaddr in register_preset_mac_address() @ 2015-06-09 7:04 Wadim Egorov 2015-06-10 4:32 ` Sascha Hauer 0 siblings, 1 reply; 6+ messages in thread From: Wadim Egorov @ 2015-06-09 7:04 UTC (permalink / raw) To: barebox Set the ethaddr for the current edev. Signed-off-by: Wadim Egorov <w.egorov@phytec.de> --- net/eth.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/eth.c b/net/eth.c index 89bddba..03e0a2e 100644 --- a/net/eth.c +++ b/net/eth.c @@ -49,6 +49,7 @@ static void register_preset_mac_address(struct eth_device *edev, const char *eth ethaddr_to_string(ethaddr, ethaddr_str); if (is_valid_ether_addr(ethaddr)) { + memcpy(edev->ethaddr, ethaddr, 6); dev_info(&edev->dev, "got preset MAC address: %s\n", ethaddr_str); dev_set_param(&edev->dev, "ethaddr", ethaddr_str); } -- 1.9.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: Set the actual ethaddr in register_preset_mac_address() 2015-06-09 7:04 [PATCH] net: Set the actual ethaddr in register_preset_mac_address() Wadim Egorov @ 2015-06-10 4:32 ` Sascha Hauer 2015-06-10 6:44 ` Wadim Egorov 0 siblings, 1 reply; 6+ messages in thread From: Sascha Hauer @ 2015-06-10 4:32 UTC (permalink / raw) To: Wadim Egorov; +Cc: barebox Hi Wadim, On Tue, Jun 09, 2015 at 09:04:25AM +0200, Wadim Egorov wrote: > Set the ethaddr for the current edev. > > Signed-off-by: Wadim Egorov <w.egorov@phytec.de> > --- > net/eth.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/eth.c b/net/eth.c > index 89bddba..03e0a2e 100644 > --- a/net/eth.c > +++ b/net/eth.c > @@ -49,6 +49,7 @@ static void register_preset_mac_address(struct eth_device *edev, const char *eth > ethaddr_to_string(ethaddr, ethaddr_str); > > if (is_valid_ether_addr(ethaddr)) { > + memcpy(edev->ethaddr, ethaddr, 6); > dev_info(&edev->dev, "got preset MAC address: %s\n", ethaddr_str); > dev_set_param(&edev->dev, "ethaddr", ethaddr_str); > } In which case is this necessary? Normally a dev_set_param on "ethaddr" should already set edev->ethaddr, there should be no need to copy this manually. 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@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: Set the actual ethaddr in register_preset_mac_address() 2015-06-10 4:32 ` Sascha Hauer @ 2015-06-10 6:44 ` Wadim Egorov 2015-06-10 11:28 ` AW: " Gabor Janak (g.janak@agilion.de) 2015-06-15 6:55 ` Sascha Hauer 0 siblings, 2 replies; 6+ messages in thread From: Wadim Egorov @ 2015-06-10 6:44 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox Hello Sascha, On 10.06.2015 06:32, Sascha Hauer wrote: > Hi Wadim, > > On Tue, Jun 09, 2015 at 09:04:25AM +0200, Wadim Egorov wrote: >> Set the ethaddr for the current edev. >> >> Signed-off-by: Wadim Egorov <w.egorov@phytec.de> >> --- >> net/eth.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/eth.c b/net/eth.c >> index 89bddba..03e0a2e 100644 >> --- a/net/eth.c >> +++ b/net/eth.c >> @@ -49,6 +49,7 @@ static void register_preset_mac_address(struct eth_device *edev, const char *eth >> ethaddr_to_string(ethaddr, ethaddr_str); >> >> if (is_valid_ether_addr(ethaddr)) { >> + memcpy(edev->ethaddr, ethaddr, 6); >> dev_info(&edev->dev, "got preset MAC address: %s\n", ethaddr_str); >> dev_set_param(&edev->dev, "ethaddr", ethaddr_str); >> } > In which case is this necessary? Normally a dev_set_param on "ethaddr" > should already set edev->ethaddr, there should be no need to copy this > manually. > > Sascha when booting from ethernet on the AM335x, net_new() (called in net_udp_new) will check if ethaddr is valid. This check fails, because ethaddr is not set at this moment and a random MAC will be used. Wadim > _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* AW: [PATCH] net: Set the actual ethaddr in register_preset_mac_address() 2015-06-10 6:44 ` Wadim Egorov @ 2015-06-10 11:28 ` Gabor Janak (g.janak@agilion.de) 2015-06-15 6:55 ` Sascha Hauer 1 sibling, 0 replies; 6+ messages in thread From: Gabor Janak (g.janak@agilion.de) @ 2015-06-10 11:28 UTC (permalink / raw) To: Wadim Egorov; +Cc: barebox Hi, This is an alternative patch - but not on git trunk - ist againts the last tag i are allowed to use. I don't know if the ethaddr should be copied in register_preset_mac_address or after calling. below is only tested on am335x. -------SNIP------ Fix missing eheraddr setting. If some modul register an MAC for an network device like cpsw, the etheraddr of the device was never set. This is a workaround only. Don't put upstream. --- barebox-mlo-2014.09.0/net/eth.c 2014-09-04 15:28:52.000000000 +0200 +++ barebox-mlo-2014.09.0/net/eth.c 2014-09-23 17:18:21.592884794 +0200 @@ -358,9 +357,8 @@ list_add_tail(&edev->list, &netdev_list); ret = eth_get_registered_ethaddr(edev, ethaddr); - if (!ret) + if (!ret && is_valid_ether_addr(ethaddr)) found = 1; - if (!found) { ret = edev->get_ethaddr(edev, ethaddr); if (!ret) @@ -368,7 +366,11 @@ } if (found) + { register_preset_mac_address(edev, ethaddr); + /* fix missing mac in edev */ + memcpy(edev->ethaddr,ethaddr,6); + } if (IS_ENABLED(CONFIG_OFDEVICE) && edev->parent && edev->parent->device_node) -------SNIP------ -----Ursprüngliche Nachricht----- Von: barebox [mailto:barebox-bounces@lists.infradead.org] Im Auftrag von Wadim Egorov Gesendet: Mittwoch, 10. Juni 2015 08:44 An: Sascha Hauer Cc: barebox@lists.infradead.org Betreff: Re: [PATCH] net: Set the actual ethaddr in register_preset_mac_address() Hello Sascha, On 10.06.2015 06:32, Sascha Hauer wrote: > Hi Wadim, > > On Tue, Jun 09, 2015 at 09:04:25AM +0200, Wadim Egorov wrote: >> Set the ethaddr for the current edev. >> >> Signed-off-by: Wadim Egorov <w.egorov@phytec.de> >> --- >> net/eth.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/net/eth.c b/net/eth.c >> index 89bddba..03e0a2e 100644 >> --- a/net/eth.c >> +++ b/net/eth.c >> @@ -49,6 +49,7 @@ static void register_preset_mac_address(struct eth_device *edev, const char *eth >> ethaddr_to_string(ethaddr, ethaddr_str); >> >> if (is_valid_ether_addr(ethaddr)) { >> + memcpy(edev->ethaddr, ethaddr, 6); >> dev_info(&edev->dev, "got preset MAC address: %s\n", ethaddr_str); >> dev_set_param(&edev->dev, "ethaddr", ethaddr_str); >> } > In which case is this necessary? Normally a dev_set_param on "ethaddr" > should already set edev->ethaddr, there should be no need to copy this > manually. > > Sascha when booting from ethernet on the AM335x, net_new() (called in net_udp_new) will check if ethaddr is valid. This check fails, because ethaddr is not set at this moment and a random MAC will be used. Wadim > _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: Set the actual ethaddr in register_preset_mac_address() 2015-06-10 6:44 ` Wadim Egorov 2015-06-10 11:28 ` AW: " Gabor Janak (g.janak@agilion.de) @ 2015-06-15 6:55 ` Sascha Hauer 2015-06-15 9:08 ` Wadim Egorov 1 sibling, 1 reply; 6+ messages in thread From: Sascha Hauer @ 2015-06-15 6:55 UTC (permalink / raw) To: Wadim Egorov; +Cc: barebox On Wed, Jun 10, 2015 at 08:44:11AM +0200, Wadim Egorov wrote: > Hello Sascha, > > > On 10.06.2015 06:32, Sascha Hauer wrote: > >Hi Wadim, > > > >On Tue, Jun 09, 2015 at 09:04:25AM +0200, Wadim Egorov wrote: > >>Set the ethaddr for the current edev. > >> > >>Signed-off-by: Wadim Egorov <w.egorov@phytec.de> > >>--- > >> net/eth.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >>diff --git a/net/eth.c b/net/eth.c > >>index 89bddba..03e0a2e 100644 > >>--- a/net/eth.c > >>+++ b/net/eth.c > >>@@ -49,6 +49,7 @@ static void register_preset_mac_address(struct eth_device *edev, const char *eth > >> ethaddr_to_string(ethaddr, ethaddr_str); > >> if (is_valid_ether_addr(ethaddr)) { > >>+ memcpy(edev->ethaddr, ethaddr, 6); > >> dev_info(&edev->dev, "got preset MAC address: %s\n", ethaddr_str); > >> dev_set_param(&edev->dev, "ethaddr", ethaddr_str); > >> } > >In which case is this necessary? Normally a dev_set_param on "ethaddr" > >should already set edev->ethaddr, there should be no need to copy this > >manually. > > > >Sascha > > when booting from ethernet on the AM335x, net_new() (called in net_udp_new) > will check if ethaddr is valid. This check fails, because ethaddr is not > set at this moment and a random MAC will be used. You mean from the MLO? Is CONFIG_PARAMETER disabled? If yes I understand the problem. Could you try this patch? Please ignore the discards const compiler warning for now. Sascha ---------------------8<----------------------------- From 3b97ddde406b15c5b3db91268e1e4b4aedaf0564 Mon Sep 17 00:00:00 2001 From: Sascha Hauer <s.hauer@pengutronix.de> Date: Mon, 15 Jun 2015 08:52:15 +0200 Subject: [PATCH] net: eth: Do not rely on CONFIG_PARAMETER to be enabled register_preset_mac_address only works when CONFIG_PARAMETER is enabled because otherwise dev_set_param is a no-op. Add a function to set the MAC address explicitly without the need of CONFIG_PARAMETER and use it where appropriate. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- include/net.h | 2 ++ net/eth.c | 25 +++++++++++++++++-------- net/net.c | 2 +- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/include/net.h b/include/net.h index 364011b..ecc39d2 100644 --- a/include/net.h +++ b/include/net.h @@ -60,6 +60,7 @@ struct eth_device { IPaddr_t serverip; IPaddr_t netmask; IPaddr_t gateway; + char ethaddr_param[6]; char ethaddr[6]; }; @@ -67,6 +68,7 @@ struct eth_device { int eth_register(struct eth_device* dev); /* Register network device */ void eth_unregister(struct eth_device* dev); /* Unregister network device */ +int eth_set_ethaddr(struct eth_device *edev, const char *ethaddr); int eth_send(struct eth_device *edev, void *packet, int length); /* Send a packet */ int eth_rx(void); /* Check for received packets */ diff --git a/net/eth.c b/net/eth.c index 0c1ff73..eba4c5c 100644 --- a/net/eth.c +++ b/net/eth.c @@ -42,15 +42,25 @@ struct eth_ethaddr { static LIST_HEAD(ethaddr_list); +int eth_set_ethaddr(struct eth_device *edev, const char *ethaddr) +{ + int ret; + + ret = edev->set_ethaddr(edev, ethaddr); + if (ret) + return ret; + + memcpy(edev->ethaddr, ethaddr, ETH_ALEN); +} + static void register_preset_mac_address(struct eth_device *edev, const char *ethaddr) { unsigned char ethaddr_str[sizeof("xx:xx:xx:xx:xx:xx")]; - ethaddr_to_string(ethaddr, ethaddr_str); - if (is_valid_ether_addr(ethaddr)) { + ethaddr_to_string(ethaddr, ethaddr_str); dev_info(&edev->dev, "got preset MAC address: %s\n", ethaddr_str); - dev_set_param(&edev->dev, "ethaddr", ethaddr_str); + eth_set_ethaddr(edev, ethaddr); } } @@ -261,13 +271,11 @@ int eth_rx(void) return 0; } -static int eth_set_ethaddr(struct param_d *param, void *priv) +static int eth_param_set_ethaddr(struct param_d *param, void *priv) { struct eth_device *edev = priv; - edev->set_ethaddr(edev, edev->ethaddr); - - return 0; + return eth_set_ethaddr(edev, edev->ethaddr_param); } #ifdef CONFIG_OFTREE @@ -350,7 +358,8 @@ int eth_register(struct eth_device *edev) dev_add_param_ip(dev, "serverip", NULL, NULL, &edev->serverip, edev); dev_add_param_ip(dev, "gateway", NULL, NULL, &edev->gateway, edev); dev_add_param_ip(dev, "netmask", NULL, NULL, &edev->netmask, edev); - dev_add_param_mac(dev, "ethaddr", eth_set_ethaddr, NULL, edev->ethaddr, edev); + dev_add_param_mac(dev, "ethaddr", eth_param_set_ethaddr, NULL, + edev->ethaddr_param, edev); if (edev->init) edev->init(edev); diff --git a/net/net.c b/net/net.c index 07350ad..75292c7 100644 --- a/net/net.c +++ b/net/net.c @@ -348,7 +348,7 @@ static struct net_connection *net_new(IPaddr_t dest, rx_handler_f *handler, random_ether_addr(edev->ethaddr); ethaddr_to_string(edev->ethaddr, str); printf("warning: No MAC address set. Using random address %s\n", str); - dev_set_param(&edev->dev, "ethaddr", str); + eth_set_ethaddr(edev, edev->ethaddr); } /* If we don't have an ip only broadcast is allowed */ -- 2.1.4 -- 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@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: Set the actual ethaddr in register_preset_mac_address() 2015-06-15 6:55 ` Sascha Hauer @ 2015-06-15 9:08 ` Wadim Egorov 0 siblings, 0 replies; 6+ messages in thread From: Wadim Egorov @ 2015-06-15 9:08 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox On 15.06.2015 08:55, Sascha Hauer wrote: > On Wed, Jun 10, 2015 at 08:44:11AM +0200, Wadim Egorov wrote: >> Hello Sascha, >> >> >> On 10.06.2015 06:32, Sascha Hauer wrote: >>> Hi Wadim, >>> >>> On Tue, Jun 09, 2015 at 09:04:25AM +0200, Wadim Egorov wrote: >>>> Set the ethaddr for the current edev. >>>> >>>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de> >>>> --- >>>> net/eth.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/net/eth.c b/net/eth.c >>>> index 89bddba..03e0a2e 100644 >>>> --- a/net/eth.c >>>> +++ b/net/eth.c >>>> @@ -49,6 +49,7 @@ static void register_preset_mac_address(struct eth_device *edev, const char *eth >>>> ethaddr_to_string(ethaddr, ethaddr_str); >>>> if (is_valid_ether_addr(ethaddr)) { >>>> + memcpy(edev->ethaddr, ethaddr, 6); >>>> dev_info(&edev->dev, "got preset MAC address: %s\n", ethaddr_str); >>>> dev_set_param(&edev->dev, "ethaddr", ethaddr_str); >>>> } >>> In which case is this necessary? Normally a dev_set_param on "ethaddr" >>> should already set edev->ethaddr, there should be no need to copy this >>> manually. >>> >>> Sascha >> when booting from ethernet on the AM335x, net_new() (called in net_udp_new) >> will check if ethaddr is valid. This check fails, because ethaddr is not >> set at this moment and a random MAC will be used. > You mean from the MLO? Is CONFIG_PARAMETER disabled? If yes I understand > the problem. Could you try this patch? > Please ignore the discards const compiler warning for now. > > Sascha Yes, from the MLO. And yes, CONFIG_PARAMETER is disabled. Tested your patch with a phyCORE AM335x board. Tested-by: Wadim Egorov <w.egorov@phytec.de> > > ---------------------8<----------------------------- > > From 3b97ddde406b15c5b3db91268e1e4b4aedaf0564 Mon Sep 17 00:00:00 2001 > From: Sascha Hauer <s.hauer@pengutronix.de> > Date: Mon, 15 Jun 2015 08:52:15 +0200 > Subject: [PATCH] net: eth: Do not rely on CONFIG_PARAMETER to be enabled > > register_preset_mac_address only works when CONFIG_PARAMETER > is enabled because otherwise dev_set_param is a no-op. Add a > function to set the MAC address explicitly without the need > of CONFIG_PARAMETER and use it where appropriate. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > include/net.h | 2 ++ > net/eth.c | 25 +++++++++++++++++-------- > net/net.c | 2 +- > 3 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/include/net.h b/include/net.h > index 364011b..ecc39d2 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -60,6 +60,7 @@ struct eth_device { > IPaddr_t serverip; > IPaddr_t netmask; > IPaddr_t gateway; > + char ethaddr_param[6]; > char ethaddr[6]; > }; > > @@ -67,6 +68,7 @@ struct eth_device { > > int eth_register(struct eth_device* dev); /* Register network device */ > void eth_unregister(struct eth_device* dev); /* Unregister network device */ > +int eth_set_ethaddr(struct eth_device *edev, const char *ethaddr); > > int eth_send(struct eth_device *edev, void *packet, int length); /* Send a packet */ > int eth_rx(void); /* Check for received packets */ > diff --git a/net/eth.c b/net/eth.c > index 0c1ff73..eba4c5c 100644 > --- a/net/eth.c > +++ b/net/eth.c > @@ -42,15 +42,25 @@ struct eth_ethaddr { > > static LIST_HEAD(ethaddr_list); > > +int eth_set_ethaddr(struct eth_device *edev, const char *ethaddr) > +{ > + int ret; > + > + ret = edev->set_ethaddr(edev, ethaddr); > + if (ret) > + return ret; > + > + memcpy(edev->ethaddr, ethaddr, ETH_ALEN); > +} > + > static void register_preset_mac_address(struct eth_device *edev, const char *ethaddr) > { > unsigned char ethaddr_str[sizeof("xx:xx:xx:xx:xx:xx")]; > > - ethaddr_to_string(ethaddr, ethaddr_str); > - > if (is_valid_ether_addr(ethaddr)) { > + ethaddr_to_string(ethaddr, ethaddr_str); > dev_info(&edev->dev, "got preset MAC address: %s\n", ethaddr_str); > - dev_set_param(&edev->dev, "ethaddr", ethaddr_str); > + eth_set_ethaddr(edev, ethaddr); > } > } > > @@ -261,13 +271,11 @@ int eth_rx(void) > return 0; > } > > -static int eth_set_ethaddr(struct param_d *param, void *priv) > +static int eth_param_set_ethaddr(struct param_d *param, void *priv) > { > struct eth_device *edev = priv; > > - edev->set_ethaddr(edev, edev->ethaddr); > - > - return 0; > + return eth_set_ethaddr(edev, edev->ethaddr_param); > } > > #ifdef CONFIG_OFTREE > @@ -350,7 +358,8 @@ int eth_register(struct eth_device *edev) > dev_add_param_ip(dev, "serverip", NULL, NULL, &edev->serverip, edev); > dev_add_param_ip(dev, "gateway", NULL, NULL, &edev->gateway, edev); > dev_add_param_ip(dev, "netmask", NULL, NULL, &edev->netmask, edev); > - dev_add_param_mac(dev, "ethaddr", eth_set_ethaddr, NULL, edev->ethaddr, edev); > + dev_add_param_mac(dev, "ethaddr", eth_param_set_ethaddr, NULL, > + edev->ethaddr_param, edev); > > if (edev->init) > edev->init(edev); > diff --git a/net/net.c b/net/net.c > index 07350ad..75292c7 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -348,7 +348,7 @@ static struct net_connection *net_new(IPaddr_t dest, rx_handler_f *handler, > random_ether_addr(edev->ethaddr); > ethaddr_to_string(edev->ethaddr, str); > printf("warning: No MAC address set. Using random address %s\n", str); > - dev_set_param(&edev->dev, "ethaddr", str); > + eth_set_ethaddr(edev, edev->ethaddr); > } > > /* If we don't have an ip only broadcast is allowed */ _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-06-15 9:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-09 7:04 [PATCH] net: Set the actual ethaddr in register_preset_mac_address() Wadim Egorov 2015-06-10 4:32 ` Sascha Hauer 2015-06-10 6:44 ` Wadim Egorov 2015-06-10 11:28 ` AW: " Gabor Janak (g.janak@agilion.de) 2015-06-15 6:55 ` Sascha Hauer 2015-06-15 9:08 ` Wadim Egorov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox