From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5DDF1C43381 for ; Tue, 26 Feb 2019 07:45:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1C4392173C for ; Tue, 26 Feb 2019 07:45:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="key not found in DNS" (0-bit key) header.d=codeaurora.org header.i=@codeaurora.org header.b="Hm423+qy"; dkim=fail reason="key not found in DNS" (0-bit key) header.d=codeaurora.org header.i=@codeaurora.org header.b="J8iiX02s" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726789AbfBZHpe (ORCPT ); Tue, 26 Feb 2019 02:45:34 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:47620 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725940AbfBZHpe (ORCPT ); Tue, 26 Feb 2019 02:45:34 -0500 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 5839960E3F; Tue, 26 Feb 2019 07:45:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1551167133; bh=EZtY2w07rlLt4xNi3hxgNAAtBbHisnGsLalBL8NNq/c=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Hm423+qyd//Frf5UBeB2FbgUyOh1bww3Q1BrZDynHsRE1D+S5iKPB5Kfq69/wQ1JB mP0o9nTpYViW7TBf6RN/ZAYtRpgDHOEtTyt/OtXfPGYf9qTxGGmklIywDG2xwiDegg tybO5I+77wGQ4CfSOWhCt6L91WVt+5uJxtxOZITY= Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id 1A2EC60A00; Tue, 26 Feb 2019 07:45:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1551167132; bh=EZtY2w07rlLt4xNi3hxgNAAtBbHisnGsLalBL8NNq/c=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=J8iiX02sB1NIvgfD56fveUapqGiAjAPNELqYQ+1fy9wQM9For6Uy7IdHxlR6GkUMS tCMuygjzHqbeCcNNyAadVnVKr45W480Yoke9Ga/WIb9mi/5Qpk4qQu6Vp68sbBz6nx jM/768zYVl0aH3PJ7P3fFEVugUwIwAY14486dR/k= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Tue, 26 Feb 2019 15:45:32 +0800 From: xiaofeis@codeaurora.org To: Florian Fainelli Cc: Vinod Koul , "David S. Miller" , linux-arm-msm@vger.kernel.org, Bjorn Andersson , Andrew Lunn , Vivien Didelot , Niklas Cassel , netdev@vger.kernel.org Subject: Re: [PATCH] net: dsa: read mac address from DT for slave device In-Reply-To: References: <20190222125815.12866-1-vkoul@kernel.org> <9baf097f-37ea-2bef-92dd-25a7040a16ed@gmail.com> Message-ID: <967ec7b53270f1e29bb25e087c1a67a4@codeaurora.org> X-Sender: xiaofeis@codeaurora.org User-Agent: Roundcube Webmail/1.2.5 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 2019-02-26 01:27, Florian Fainelli wrote: > On 2/25/19 5:28 AM, xiaofeis@codeaurora.org wrote: >> 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. > > OK, but this is not necessary per my previous explanation: the CPU <=> > WAN pipe is a separate broadcast domain (otherwise it is a security > hole > since you exposing LAN machines to the outside world), and so there is > no need for a separate MAC address. It might be convenient to have one, > especially for the provider, if they run a management software (e.g.: > TR69), but it is not required per-se. > > Let me ask a secondary question here, how many Ethernet MACs connect to > the switch in this configuration? Is there one that is supposed to be > assigned all LAN traffic and one that is supposed to be assigned all > WAN > traffic? If so, then what you are doing makes even less > Only one MAC connected to switch cpu port, both lan0 and wan0 are on the top of same interface(eth0). >> >> On 2019-02-22 23:43, Florian Fainelli wrote: >>> On 2/22/19 4:58 AM, Vinod Koul wrote: >>>> From: Xiaofei Shen >>>> >>>> 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 >>>> Signed-off-by: Vinod Koul >>>> --- >>>>  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; >>>>