[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160721090706.GA2171@nanopsycho.orion>
Date: Thu, 21 Jul 2016 11:07:07 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, yotamg@...lanox.com,
eladr@...lanox.com, idosch@...lanox.com, nogahf@...lanox.com,
ogerlitz@...lanox.com
Subject: Re: [patch net-next 0/9] mlxsw: implement port mirroring offload
Thu, Jul 21, 2016 at 11:00:33AM CEST, jhs@...atatu.com wrote:
>On 16-07-21 04:19 AM, Jiri Pirko wrote:
>>From: Jiri Pirko <jiri@...lanox.com>
>>
>>This patchset introduces tc matchall classifier and its offload
>>to Spectrum hardware. In combination with mirred action, defined port mirroring
>>setup is offloaded by mlxsw/spectrum driver.
>>
>>The commands used for creating mirror ports:
>>
>># ingress mirroring using matchall
>>tc qdisc add dev eth25 handle ffff: ingress
>>tc filter add dev eth25 parent ffff: \
>> matchall skip_sw \
>> action mirred egress mirror \
>> dev eth27
>>
>># egress mirroring using matchall
>>tc qdisc add dev eth25 handle 1: root prio
>>tc filter add dev eth25 parent 1: \
>> matchall skip_sw \
>> action mirred egress mirror \
>> dev eth27
>>
>
>
>Kudos to Mellanox for all this nice work!
>
>I am assuming the chip is capable as well of doing
>mirroring via the ACL infrastructure and you are adding
>this classifier because you are going via the SPAN
Yes, we are using span.
>infrastructure. If answer is yes, thencould we have used
>a classifier like u32 here?
>i.e something like:
>tc filter add dev eth25 xxxx protocol all \
>u32 match u32 0 0 \
>action mirred ...
That could be used. But I believe it is nicer to have explicit match-all
classifier for this case. That puts nice limit to what could be matched.
>
>BTW: I am not a big styling lawyer on netdev (I am normally the
>victim) but would be useful to look at some of these patches with
>that coding style in in mind (I think some of the chip specific
>patches had some style issue in function definition).
Could you point to that? checkpatch.pl does not say anything and I also
don't see anything.
>
>again - kudos
Thanks.
>
>cheers,
>jamal
Powered by blists - more mailing lists