netdev - Re: [PATCH] net: dsa: read mac address from DT for slave device

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]

Date:   Mon, 25 Feb 2019 21:28:44 +0800
From:   xiaofeis@...eaurora.org
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Vinod Koul <vkoul@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        linux-arm-msm@...r.kernel.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Niklas Cassel <niklas.cassel@...aro.org>,
        netdev@...r.kernel.org
Subject: Re: [PATCH] net: dsa: read mac address from DT for slave device

Hi Florian

We have two slave DSA interfaces, wan0 and lan0, one is for wan port,
and the other is for lan port. Customer has it's mac address pool, they 
want
to assign the mac address from the pool on wan0, lan0, and other 
interfaces like
wifi, bt. Coreboot/uboot will populate it to the DTS node, so the driver 
can
get it from it's node. For DSA slave interface, it already has it's own 
DTS node, it's
easy to just add one porperty "local-mac-address" there for the usage in 
DSA driver.

If not use DSA framework, normally we will use eth0.x and eth0.y for wan 
and lan.
On this case, customer usually also assign the MAC address on these 
logical interface
from it's pool.

On 2019-02-22 23:43, Florian Fainelli wrote:
> On 2/22/19 4:58 AM, Vinod Koul wrote:
>> From: Xiaofei Shen <xiaofeis@...eaurora.org>
>> 
>> Before creating a slave netdevice, get the mac address from DTS and
>> apply in case it is valid.
> 
> Can you explain your use case in details?
> 
> Assigning a MAC address to a network device that represents a switch
> port does not quite make sense in general. The switch port is really
> representing one end of a pipe, so one side you have stations and on 
> the
> other side, you have the CPU/management Ethernet MAC controller's MAC
> address which constitutes a station as well. The DSA slave network
> devices are just software constructs meant to steer traffic towards
> specific ports of the switch, but they are all from the perpsective of
> traffic reaching the CPU Port in the first place, therefore traffic 
> that
> is generally a known unicast Ethernet frame with the CPU's MAC address
> as MAC DA (and of course all types of unknown MC, management traffic 
> etc.)
> 
> By default, DSA switch need to come up in a configuration where all
> ports (except CPU/management) must be strictly separate from every 
> other
> port such that we can achieve what a standalone Ethernet NIC would do.
> This works because all ports are isolated from one another, so there is
> no cross talk and so having the same MAC address (the one from the CPU)
> on the DSA slave network devices just works, each port is a separate
> broadcast domain.
> 
> Once you start bridging one or ore ports, the bridge root port will 
> have
> a MAC address, most likely the one the CPU/management Ethernet MAC, but
> similarly, this is not an issue and that's exactly how a software 
> bridge
> would work as well.
> 
>> 
>> Signed-off-by: Xiaofei Shen <xiaofeis@...eaurora.org>
>> Signed-off-by: Vinod Koul <vkoul@...nel.org>
>> ---
>>  include/net/dsa.h | 1 +
>>  net/dsa/dsa2.c    | 1 +
>>  net/dsa/slave.c   | 5 ++++-
>>  3 files changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index b3eefe8e18fd..aa24ce756679 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -198,6 +198,7 @@ struct dsa_port {
>>  	unsigned int		index;
>>  	const char		*name;
>>  	const struct dsa_port	*cpu_dp;
>> +	const char		*mac;
>>  	struct device_node	*dn;
>>  	unsigned int		ageing_time;
>>  	u8			stp_state;
>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>> index a1917025e155..afb7d9fa42f6 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -261,6 +261,7 @@ static int dsa_port_setup(struct dsa_port *dp)
>>  	int err = 0;
>> 
>>  	memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
>> +	dp->mac = of_get_mac_address(dp->dn);
>> 
>>  	if (dp->type != DSA_PORT_TYPE_UNUSED)
>>  		err = devlink_port_register(ds->devlink, &dp->devlink_port,
>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>> index a3fcc1d01615..8e64c4e947c6 100644
>> --- a/net/dsa/slave.c
>> +++ b/net/dsa/slave.c
>> @@ -1308,7 +1308,10 @@ int dsa_slave_create(struct dsa_port *port)
>>  	slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
>>  	slave_dev->hw_features |= NETIF_F_HW_TC;
>>  	slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
>> -	eth_hw_addr_inherit(slave_dev, master);
>> +	if (port->mac && is_valid_ether_addr(port->mac))
>> +		ether_addr_copy(slave_dev->dev_addr, port->mac);
>> +	else
>> +		eth_hw_addr_inherit(slave_dev, master);
>>  	slave_dev->priv_flags |= IFF_NO_QUEUE;
>>  	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
>>  	slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
>>