netdev - Re: [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC addresses.

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: <19848.21653.877174.416543@ipc1.ka-ro>
Date:	Tue, 22 Mar 2011 08:49:41 +0100
From:	Lothar Waßmann <LW@...O-electronics.de>
To:	Shawn Guo <shawn.guo@...escale.com>
Cc:	<netdev@...r.kernel.org>, <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH 4/4] drivers/net/fec: Don't mess with configured MAC
 addresses.

Hi,

Shawn Guo writes:
> On Mon, Mar 21, 2011 at 05:37:36PM +0100, Lothar Waßmann wrote:
> > The FEC driver currently uses the MAC address assigned to the first
> > interface incremented by one for the second interface.
> > 
> > Change this to be able to configure distinct MAC addresses via
> > platform_data.
> > 
> > Signed-off-by: Lothar Waßmann <LW@...O-electronics.de>
> > ---
> >  drivers/net/fec.c |    9 ++-------
> >  1 files changed, 2 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> > index 3666524..9d89e99 100644
> > --- a/drivers/net/fec.c
> > +++ b/drivers/net/fec.c
> > @@ -750,13 +750,12 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> >  	/*
> >  	 * 2) from flash or fuse (via platform data)
> >  	 */
> > +	if (pdata)
> > +		memcpy(iap, pdata->mac, ETH_ALEN);
> >  	if (!is_valid_ether_addr(iap)) {
> >  #ifdef CONFIG_M5272
> >  		if (FEC_FLASHMAC)
> >  			iap = (unsigned char *)FEC_FLASHMAC;
> > -#else
> > -		if (pdata)
> > -			memcpy(iap, pdata->mac, ETH_ALEN);
> >  #endif
> >  	}
> >  
> > @@ -772,10 +771,6 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
> >  	}
> >  
> >  	memcpy(ndev->dev_addr, iap, ETH_ALEN);
> > -
> > -	/* Adjust MAC if using macaddr */
> > -	if (iap == macaddr)
> > -		 ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->pdev->id;
> >  }
> >  
> >  /* ------------------------------------------------------------------------- */
> > -- 
> > 1.5.6.5
> > 
> > 
> NAK.
> 
> The 'mac' was designed as an optional member of fec
> platform_data, as some platforms may not be able to provide it. This
> patch breaks the order of getting mac, and will not work on the
> platform that does not provide an valid mac with either platform_data
> or controller mac registers, even there may be one in the kernel
> command line.
> 
> The existing code takes mac address from kernel command line as the
> first choice, and only falls through on other options when it's
> invalid/unavailable.  So an valid mac from  kernel command line will
> always get fec work anyway.
> 
The problem is that you can only assign one mac address on the kernel
cmdline and the second one will be derived from the first one.
And even if the MAC address is set via platform_data it will not be
used for the second interface.

IMO an ethernet driver has no business in inventing arbitrary MAC
addresses. Either the MAC address is supplied by the boot loader or
the platform code or it has to be assigned from userspace via
ifconfig.



Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@...o-electronics.de
___________________________________________________________
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html