[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e7a7ca6a-cad9-4fb9-07b1-5dc557566f54@gmail.com>
Date: Thu, 9 Nov 2017 10:41:15 -0800
From: Florian Fainelli <f.fainelli@...il.com>
To: Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc: David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 net-next 0/5] IGMP snooping for local traffic
On 11/07/2017 04:41 PM, Florian Fainelli wrote:
> On 11/07/2017 03:17 PM, Andrew Lunn wrote:
>> On Tue, Nov 07, 2017 at 05:37:32PM -0500, Vivien Didelot wrote:
>>> Hi Andrew,
>>>
>>> Andrew Lunn <andrew@...n.ch> writes:
>>>
>>>>> In a switch case, they all translate to programming a MDB entry for
>>>>> a given switch port, right?
>>>>
>>>> No, in fact it is the exact opposite.
>>>
>>> Yes, they do. The proof is you call dsa_port_mdb_add.
>>
>> Note that i always say switchdev.
>>
>> switchdev has no concept of the CPU port. All switchdev has is the
>> concept of the external ports.
>
> Correct.
>
>>
>> So when there is a join on the br0 interface, the bridge code will
>> iterative over each port in the bridge, and make a switchdev call to
>> each of the external ports in the bridge asking it to forward
>> multicast traffic for a group to the host.
>
> Right, and that makes sense thus far.
>
>>
>> Now, deep down in DSA, we can translate this to a dsa_port_mdb_add, on
>> the CPU port. And we do that for every call the bridge makes for each
>> of the external ports in the bridge.
>
> Right, and we should actually do that, because this is a DSA specific
> detail: that there is a separate management port that needs specific
> treatment here.
>
>>
>> However, a pure switchdev device won't do that. It does not have a CPU
>> port. It probably needs to add a match/action rule to its tables for
>> the actual external port saying to forward the frame out the slow
>> path.
>
> Right, that's why DSA builds on top of switchdev for most notifications
> and also generate its own for things that are inherently DSA specific.
>
>>
>>> Still, what I see here _from a switch driver point of view_ is either
>>> program an MDB entry on a user port, or on its CPU port.
>>
>> I agree with this, if you make one change:
>>
>> _from a DSA switch driver point of view_
>>
>> However, in the general case, this is not true. We need an API which
>> works for Mellonex and Netranome as well, systems without a CPU port.
>
> We can have the exact same type of notification being sent today and
> with your changes target the bridge network devices, any other driver
> other than DSA which implements switchdev should just ignore that
> notification when they cannot resolve this network device to something
> that makes sense for their HW. In fact, Jiri and Ido (which should
> probably be copied on this patch series, along with Nikolay) was adamant
> to the idea:
>
> https://lists.linux-foundation.org/pipermail/bridge/2016-November/010124.html
>
> The usual model for notifications is that if you can interpret and act
> on them, great, if you can't do nothing.
BTW, to continue on your argument that we want this to work for both DSA
and switchdev, keep in mind that switchdev drivers, although
conceptually similar to DSA devices do not have a management interface
that consists of a separate CPU port, the management interface is
intertwined with DMA/PIO and specific descriptors within the switch
hardware. This is what differentiates switchdev drivers from DSA
drivers, switchdev drivers do not really have a concept of specific
host/CPU management interface, the over arching hardware/driver
represent the management interface and every other port is an
user-facing port.
This means that switchdev drivers won't ever have to treat a HOST_MDB
notification any differently than a PORT_MDB notification, because in
their case, they would decide, on a per-port basis whether to trap to
management. In DSA, we want the same, except that the management port is
also a semi-normal CPU port of the switch, so we need to target it
specifically.
My 2 cents.
--
Florian
Powered by blists - more mailing lists