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=-0.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS 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 B7012C43381 for ; Mon, 4 Mar 2019 03:48:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 794AB20823 for ; Mon, 4 Mar 2019 03:48:50 +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="fREq/aVX"; dkim=fail reason="key not found in DNS" (0-bit key) header.d=codeaurora.org header.i=@codeaurora.org header.b="Xysg3Ypk" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726098AbfCDDst (ORCPT ); Sun, 3 Mar 2019 22:48:49 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:52648 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725938AbfCDDss (ORCPT ); Sun, 3 Mar 2019 22:48:48 -0500 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 3939D60CED; Mon, 4 Mar 2019 03:48:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1551671327; bh=wsYgfQFmn5mZScu4E1CRRAGcAu0o5fGWEmxApNSISdE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=fREq/aVXsTj49eJqrb+TSno4HgRY2H98HxyPU754NnsJtwqptkW9J/fmc92Kxsyik wWUT4ui9VQfT5xoFsYsrx20Q9JIcFK1pq7XXJNaWvDzEK9DuaSZYu6JCZxogzhWngt r7+dqY83bgslIJwUC6aDjt7JIKHmRkewFQKWe8lc= Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id ECB23608CC; Mon, 4 Mar 2019 03:48:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1551671326; bh=wsYgfQFmn5mZScu4E1CRRAGcAu0o5fGWEmxApNSISdE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Xysg3YpkM6KmJWynaayA2RwgHerAdXEQZ7ZByTA21FVvpcX5JMRZQ87l+WaSl0AYR v7b336Ek8dZ9VOtTKKrjdGN5RODUjqqO6jzfUHhBEnjIDX++aU8ptqm3VjDfXM/6rU CiiPzZ0QwisIehEolahZzBTJpKsaiy+oH9DO2ztc= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Mon, 04 Mar 2019 11:48:45 +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> <967ec7b53270f1e29bb25e087c1a67a4@codeaurora.org> <9962d7a13bb25c88cc42b3f4dd68cee7@codeaurora.org> <9f0a1ca6-2efd-a4e6-77b5-14b68a3a791d@gmail.com> <399272053c9e69386e709923544ba0d6@codeaurora.org> Message-ID: <363f681cf8ff4bdf0b8c1db68ca6ada6@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-28 11:54, Florian Fainelli wrote: > On 2/27/2019 6:23 PM, xiaofeis@codeaurora.org wrote: >> On 2019-02-27 11:13, Florian Fainelli wrote: >>> On 2/26/2019 6:04 PM, xiaofeis@codeaurora.org wrote: >>>> 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. >>> >>> Yes it is a similar use case, and in both cases there is no really a >>> functional need for a separate MAC address for lan/eth0.1 or >>> wan/eth0.2 >>> since the switch should be configured to perform IVL (Individual VLAN >>> Learning) and would determine the egress port just fine based on the >>> MAC >>> DA. Because it is an established practice does not mean we should not >>> challenge it :). >>> >>> My issue with your change is that because DSA is meant to be a >>> flexible >>> framework we do not know the type/nature of DSA master network device >>> that is going to be used. That DSA master network device may or may >>> not >>> have it own MAC DA filtering capability. Having to filter its own MAC >>> address is fine and a well established behavior, having to filter for >>> more than one unicast address starts to be questionable and eats up >>> filter space that could be better used for filtering MC addresses >>> instead. Another possible concern is a station trying to spoof the >>> MAC >>> address, some switches may support protecting only one UC/management >>> MAC >>> address, so having more than one could create security attack >>> surfaces. >>> >>> To give you an example, I work with 3 generations of DSA master >>> network >>> controllers (bcmgenet and bcmsysport drivers). >>> >>> - GENET supports 17 perfect filters, but we must include its own MAC >>> address, the broadcast address and that leaves only 15 filters for MC >>> >>> - SYSTEMPORT is always attached to a switch but supports filtering >>> the >>> MAC DA based on its own MAC and then it is in promiscuous mode >>> >>> So with your scheme, we would leave only 13 filters for MC on GENET >>> and >>> we would putting the interface in promiscuous mode for SYSTEMPORT. >>> >>> Until we have a better switch-side filtering framework (and this is >>> being worked on right now), I would prefer that we defer accepting >>> those >>> type of features. Andrew and Vivien might feel differently about that >>> though. >> This patch is just add one more option, if there is valid mac address >> populated >> in the DTS, then use it or else still inherti from master. I don't >> think >> it will >> break the DSA flexible framework. I think this change make DSA more >> flexible on >> MAC address setting. >> Many cusomter use some of our QCA chips, some direclty use DSA, some >> use >> internal similar >> mechanism(one netdevice for each switch port with swtichdev), we >> didn't >> see any limiation >> when they populiate the mac address for each port in DTS with only one >> HW mac controller. >> So my  opinion is this patch is want to add a option which is already >> used in many >> products, this change does not break anything, developer/customer can >> chose use or not. > > As I wrote, I am not totally opposed to it, I would prefer we had a > better infrastructure for UC/MC filtering in place before landing this > change but that is not there yet, and won't happen over night. So > please > address Andrew's feedback to provide an update to the DSA binding > document, repost and I will certainly Ack it this time. > > Please be considerate of people giving you feedback and do not try to > circle back to your use case and just explaining in a different way > than > "works for me, accept it", because that's not going to work really well > in the long run. > > Looking forward for more contributions on the qca8k driver, thanks! Thanks Florian, I have reply Andres's mail to provide DSA binding document change.