netdev - Re: cls_matchall and port mirroring questions

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]

Message-ID: <20170128141904.GB2620@splinter.mtl.com>
Date:   Sat, 28 Jan 2017 16:19:04 +0200
From:   Ido Schimmel <idosch@...lanox.com>
To:     Florian Fainelli <f.fainelli@...il.com>
CC:     <netdev@...r.kernel.org>, <jiri@...lanox.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        <yotamg@...lanox.com>
Subject: Re: cls_matchall and port mirroring questions

On Thu, Jan 26, 2017 at 07:00:50PM -0800, Florian Fainelli wrote:
> Hi,
> 
> As I am adding support for cls_matchall in the b53/bcm_sf2 drivers, I
> was looking into several, yet unrelated things:
> 
> - mlxsw does not seem to specify whether the port used for capture
> remains usable, or blocks non-mirror traffic ingressing/egressing it, do
> we want a control knob for that? If not, what is a sensible default,
> block all non capture traffic?

Doesn't make sense to me to add such a default. It's up to the user.

> - do we have an updated man page for tc-matchall.8 that features how to
> use the statistical sampler too? b53 switches have a divider that allows
> us to select how many frames we want to receive (10 bit value).

https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=5c5670fae43027778e84b9d9ff3b9d91a10a8131

Yotam (Cced) already commented that he intends to send iproute patches.

> - b53 supports capture against a particular MAC SA or DA (or both), do
> we want to be able to control that somehow?

Can't you just use flower (for example) instead of matchall in that
case?

> What about Marvell switches, what can they do?

No idea :)

> -  a fair amount of code dealing with the cls_matchall mirroring entry
> is not switch driver specific, in fact, the only things that are switch
> driver specific are:
> 	- list pointer where to store this entry (typically in the private
> network device context)
> 	- operation to check whether the device belongs to us (identical
> netdev_ops)
> 	- retrieval of the destination port number (to_port) which is also
> typically available in network device private context
> 
> Do we want to move a fair amount of code into switchdev, treat
> cls_matchall entries as a specific switchdev object, and have drivers
> take over at the same level that mlxsw_sp_port_add_cls_matchall_mirror()
> currently starts?

I prefer the current way in which we re-use as many as possible core
APIs without adding switchdev-specific code. I don't have a concrete
argument against your proposal, though.