Re: [PATCH net-next 6/6] net: dsa: sja1105: add broadcast and per-traffic class policers

[Nikolay Aleksandrov <nikolay@cumulusnetworks.com>]

On 29/03/2020 15:02, Nikolay Aleksandrov wrote:
> On 29/03/2020 14:46, Vladimir Oltean wrote:
>> On Sun, 29 Mar 2020 at 14:37, Vladimir Oltean <olteanv@gmail.com> wrote:
>>>
>>> On Sun, 29 Mar 2020 at 12:57, Ido Schimmel <idosch@idosch.org> wrote:
>>>>
>>>> + Nik, Roopa
>>>>
>>>> On Sun, Mar 29, 2020 at 02:52:02AM +0200, Vladimir Oltean wrote:
>>>>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> [snip]
>>>> In the past I was thinking about ways to implement this in Linux. The
>>>> only place in the pipeline where packets are actually classified to
>>>> broadcast / unknown unicast / multicast is at bridge ingress. Therefore,
>>>
>>> Actually I think only 'unknown unicast' is tricky here, and indeed the
>>> bridge driver is the only place in the software datapath that would
>>> know that.
> 
> Yep, unknown unicast is hard to pass outside of the bridge, especially at ingress
> where the bridge hasn't been hit yet. One possible solution is to expose a function
> from the bridge which can make such a decision at the cost of 1 more fdb hash lookup,
> but if the packet is going to hit the bridge anyway that cost won't be that high
> since it will have to do the same. We already have some internal bridge functionality
> exposed for netfilter, tc and some drivers so it would be in line with that.
> I haven't looked into how feasible the above is, so I'm open to other ideas (the
> bridge_slave functions for example, we've discussed such extensions before in other
> contexts). But I think this can be much simpler if we just expose the unknown unicast
> information, the mcast/bcast can be decided by the classifier already or with very
> little change. I think such exposed function can be useful to netfilter as well.
> 

Of course along with the unknown unicast, we should include unknown multicast.

>>> I know very little about frame classification in the Linux network
>>> stack, but would it be possible to introduce a match key in tc-flower
>>> for whether packets have a known destination or not?
>>>
>>>> my thinking was to implement these storm control policers as a
>>>> "bridge_slave" operation. It can then be offloaded to capable drivers
>>>> via the switchdev framework.
>>>>
>>>
>>> I think it would be a bit odd to duplicate tc functionality in the
>>> bridge sysfs. I don't have a better suggestion though.
>>>
>>
>> Not to mention that for hardware like this, to have the same level of
>> flexibility via a switchdev control would mean to duplicate quite a
>> lot of tc functionality. On this 5-port switch I can put a shared
>> broadcast policer on 2 ports (via the ingress_block functionality),
>> and individual policers on the other 3, and the bandwidth budgeting is
>> separate. I can only assume that there are more switches out there
>> that allow this.
>>>>>> I think that if we have this implemented in the Linux bridge, then your
>>>> patch can be used to support the policing of broadcast packets while
>>>> returning an error if user tries to police unknown unicast or multicast
>>>> packets.
>>>
>>> So even if the Linux bridge gains these knobs for flood policers,
>>> still have the dst_mac ff:ff:ff:ff:ff:ff as a valid way to configure
>>> one of those knobs?
>>>
>>>> Or maybe the hardware you are working with supports these types
>>>> as well?
>>>
>>> Nope, on this hardware it's just broadcast, I just checked that. Which
>>> simplifies things quite a bit.
>>>
>>>>
>>>> WDYT?
>>>>
>>>
>>> I don't know.
>>>
>>> Thanks,
>>> -Vladimir
>>
>> -Vladimir
>>
>