netdev - Re: [patch net-next 0/9] mlxsw: implement port mirroring offload

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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