Le 01/28/17 à 07:55, Jiri Pirko a écrit : > Sat, Jan 28, 2017 at 03:19:04PM CET, ido...@mellanox.com wrote: >> 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? > > Definitelly. No extensions to matchall, as otherwise it could not be > called like that anylonger:)
OK, that make sense. I thought of matchall as action that would solely deal with all port mirroring features, but using flower on top of an ingress or egress qdisc + src/dst MAC would definitively and I guess make senses too. > > >> >>> 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. > > This (tc-offload) is completely unrelated to switchdev. So it would make > no sense. That is true, there is a bit of code (allocation of tc entries, parsing of actions list etc.) that could be made generic; and one could argue that each tc action programming request could look like some kind of special switchdev object. I am totally fine keeping things the way they are though. Thanks! -- Florian
