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 62160C43381 for ; Wed, 27 Feb 2019 02:04:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 271BE218E2 for ; Wed, 27 Feb 2019 02:04:10 +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="MwYLJNej"; dkim=fail reason="key not found in DNS" (0-bit key) header.d=codeaurora.org header.i=@codeaurora.org header.b="lZk0rbdd" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729694AbfB0CEI (ORCPT ); Tue, 26 Feb 2019 21:04:08 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:59804 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729464AbfB0CEI (ORCPT ); Tue, 26 Feb 2019 21:04:08 -0500 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 63E8660AA3; Wed, 27 Feb 2019 02:04:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1551233047; bh=csC8G+eweBPCphNd7R/I2RxLtN0wBAL475FF4IfHfeg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=MwYLJNejXw9ECTnApVcewEhf0uXbndx0XzymDk0kGLQkYg8aTlVf11a/DA2zNElhv rr+V+RizQwj6C+RUK9SSB7S4TEQYcTb9rZV1oCpxeuSFyX6mk1BEmKNm7B2fAPUC/c 8IKGKK4hNvOlhR8zEc/ixmhJ7Z4rpyhjZxwuZ8ME= Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id 3073F60A05; Wed, 27 Feb 2019 02:04:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1551233046; bh=csC8G+eweBPCphNd7R/I2RxLtN0wBAL475FF4IfHfeg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=lZk0rbdd60NFyuVRhGG29mFHd1mEK7VbNJMWodUeSLoSGWzsV2geMsWh4mXR/DrAP wPanK23s54fr5UR+H84nV8TmEWOdhU18Yus/oAWEB36qrZiuo8BxS+SSbq3qoDyDJj KplqldxyK9+94oY9zzHwP/XYvhRQuF4NGmzHLOKA= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Wed, 27 Feb 2019 10:04:06 +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: <967ec7b53270f1e29bb25e087c1a67a4@codeaurora.org> References: <20190222125815.12866-1-vkoul@kernel.org> <9baf097f-37ea-2bef-92dd-25a7040a16ed@gmail.com> <967ec7b53270f1e29bb25e087c1a67a4@codeaurora.org> Message-ID: <9962d7a13bb25c88cc42b3f4dd68cee7@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 15:45, xiaofeis@codeaurora.org wrote: > 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). > Customer doesn't care about the MAC controller's MAC address, just leave it as the driver randomly generated. They just want to assign the MAC address on wan and lan DSA logical interface. Many customer doesn't use DSA, for example, they use eth0.1/eth0.2 for lan/wan with one MAC controller. They configure switch wan port in vlan2 group, and lan port in vlan1 group, they usually assign mac address on the logical interface(eth0.1ð0.2), i think this is similar with DSA slave interfaces. >>> >>> 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; >>>>>