Re: [PATCH v6 net-next 0/8] net: dsa: felix: psfp support on vsc9959

From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"allan.nielsen@microchip.com" <allan.nielsen@microchip.com>,
	"joergen.andreasen@microchip.com"
	<joergen.andreasen@microchip.com>,
	"UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
	"vinicius.gomes@intel.com" <vinicius.gomes@intel.com>,
	"jiri@mellanox.com" <jiri@mellanox.com>,
	"idosch@mellanox.com" <idosch@mellanox.com>,
	"alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
	Po Liu <po.liu@nxp.com>, Leo Li <leoyang.li@nxp.com>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	"andrew@lunn.ch" <andrew@lunn.ch>,
	"vivien.didelot@gmail.com" <vivien.didelot@gmail.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
	"horatiu.vultur@microchip.com" <horatiu.vultur@microchip.com>
Subject: Re: [PATCH v6 net-next 0/8] net: dsa: felix: psfp support on vsc9959
Date: Fri, 1 Oct 2021 22:46:34 +0000	[thread overview]
Message-ID: <20211001224633.u7ylsyy4mpl5kmmo@skbuf> (raw)
In-Reply-To: <20211001151115.5f583f4a@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Fri, Oct 01, 2021 at 03:11:15PM -0700, Jakub Kicinski wrote:
> On Thu, 30 Sep 2021 15:59:40 +0800 Xiaoliang Yang wrote:
> > VSC9959 hardware supports Per-Stream Filtering and Policing(PSFP).
> > This patch series add PSFP support on tc flower offload of ocelot
> > driver. Use chain 30000 to distinguish PSFP from VCAP blocks. Add gate
> > and police set to support PSFP in VSC9959 driver.
>
> Vladimir, any comments?

Sorry, I was intending to try out the patches and get an overall feel
from there, but I had an incredibly busy week and simply didn't have time.
If it's okay to wait a bit more I will do that tomorrow.
In general I feel that the most glaring issue Xiaoliang has still
avoided to address is the one discussed here:
https://patchwork.kernel.org/project/netdevbpf/patch/20210831034536.17497-6-xiaoliang.yang_1@nxp.com/#24416737
where basically some tc filters depend on some bridge fdb entries, and
there's no way to prevent the bridge from deleting the fdb entries which
would in turn break the tc filters, but also no way of removing the tc
filters when the bridge fdb entries disappear.
The hardware design is poor, no two ways around that, but arguably it's
a tricky issue to handle in software too, the bridge simply doesn't give
switchdev drivers a chance to veto an fdb removal, and I've no idea what
changing that would even mean. So I can understand why Xiaoliang is
avoiding it.
That's why I wanted to run the patches too, first I feel that we should
provide a selftest for the feature, and that is absent from this patch
series, and second I would like to see how broken can the driver state
end up being if we just leave tc filters around which are just inactive
in the absence of a bridge, or a bridge fdb entry. I simply don't know
that right now.
It's almost as if we would be better off stealing some hardware FDB
entries from the bridge and reserving them for the tc filter, and not
depending on the bridge driver at all.