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:   Fri, 22 Feb 2019 07:43:36 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vinod Koul <vkoul@...nel.org>,
        "David S. Miller" <davem@...emloft.net>
Cc:     linux-arm-msm@...r.kernel.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Xiaofei Shen <xiaofeis@...eaurora.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

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;
> 

-- 
Florian