LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Joergen Andreasen <joergen.andreasen@microchip.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>,
	Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"allan.nielsen@microchip.com" <allan.nielsen@microchip.com>,
	"UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
	"vinicius.gomes@intel.com" <vinicius.gomes@intel.com>,
	"michael.chan@broadcom.com" <michael.chan@broadcom.com>,
	"saeedm@mellanox.com" <saeedm@mellanox.com>,
	"jiri@mellanox.com" <jiri@mellanox.com>,
	"idosch@mellanox.com" <idosch@mellanox.com>,
	"alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
	"kuba@kernel.org" <kuba@kernel.org>, Po Liu <po.liu@nxp.com>,
	Leo Li <leoyang.li@nxp.com>
Subject: Re: [PATCH v3 net-next 5/8] net: dsa: felix: support psfp filter on vsc9959
Date: Thu, 9 Sep 2021 13:33:57 +0200	[thread overview]
Message-ID: <9619848400baaa0d0d12cc6a2d799934323e2657.camel@microchip.com> (raw)
In-Reply-To: <20210831104913.g3n6dov6gwflc3pm@skbuf>

On Tue, 2021-08-31 at 10:49 +0000, Vladimir Oltean wrote:
> On Tue, Aug 31, 2021 at 09:59:11AM +0000, Xiaoliang Yang wrote:
> > On Tue, Aug 31, 2021 at 17:18:00PM +0300, Vladimir Oltean wrote:
> > > > > > I think in previous versions you were automatically
> > > > > > installing a
> > > > > > static MAC table entry when one was not present (either it
> > > > > > was
> > > > > > absent, or the entry was dynamically learned). Why did that
> > > > > > change?
> > > > > 
> > > > > The PSFP gate and police action are set on ingress port, and
> > > > > "
> > > > > tc-filter" has no parameter to set the forward port for the
> > > > > filtered
> > > > > stream. And I also think that adding a FDB mac entry in tc-
> > > > > filter
> > > > > command is not good.
> > > > 
> > > > Fair enough, but if that's what you want, we'll need to think a
> > > > lot
> > > > harder about how this needs to be modeled.
> > > > 
> > > > Would you not have to protect against a 'bridge fdb del'
> > > > erasing your
> > > > MAC table entry after you've set up the TSN stream on it?
> > > > 
> > > > Right now, DSA does not even call the driver's .port_fdb_del
> > > > method
> > > > from atomic context, just from deferred work context. So even
> > > > if you
> > > > wanted to complain and say "cannot remove FDB entry until SFID
> > > > stops
> > > > pointing at it", that would not be possible with today's code
> > > > structure.
> > > > 
> > > > And what would you do if the bridge wants to delete the FDB
> > > > entry
> > > > irrevocably, like when the user wants to delete the bridge in
> > > > its
> > > > entirety? You would still remain with filters in tc which are
> > > > not
> > > > backed by any MAC table entry.
> > > > 
> > > > Hmm..
> > > > Either the TSN standards for PSFP and FRER are meant to be
> > > > implemented
> > > > within the bridge driver itself, and not as part of tc, or the
> > > > Microchip implementation is very weird for wiring them into the
> > > > bridge MAC
> > > table.
> > > > Microchip people, any comments?
> > > 
> > > In sja1105's implementation of PSFP (which is not standard-
> > > compliant as it is
> > > based on TTEthernet, but makes more sense anyway), the Virtual
> > > Links (SFIDs
> > > here) are not based on the FDB table, but match only on the given
> > > source port.
> > > They behave much more like ACL entries.
> > > The way I've modeled them in Linux was to force the user to
> > > offload multiple
> > > actions for the same tc-filter, both a redirect action and a
> > > police/gate action.
> > > https://www.kernel.org/doc/html/latest/networking/dsa/sja1105.html#time-b
> > > ased-ingress-policing
> > > 
> > > I'm not saying this helps you, I'm just saying maybe the
> > > Microchip
> > > implementation is strange, but then again, I might be looking the
> > > wrong way
> > > at it.
> > 
> > Yes, Using redirect action can give PSFP filter a forward port to
> > add
> > MAC table entry. But it also has the issue that when using "bridge
> > fdb
> > del" to delete the MAC entry will cause the tc-filter rule not
> > working.
> 
> We need to define the expected behavior.
> 
> As far as the 802.1Q-2018 spec is concerned, there is no logical
> dependency between the FDB lookup and the PSFP streams. But there
> seems
> to be no explicit text that forbids it either, though.
> 
> If you install a tc-redirect rule and offload it as a bridge FDB
> entry,
> it needs to behave like a tc-redirect rule and not a bridge FDB
> entry.
> So it only needs to match on the intended source port. I don't
> believe
> that is possible. If it is, let's do that.
> 
> To me, putting PSFP inside the bridge driver is completely outside of
> the question. There is no evidence that it belongs there, and there
> are
> switch implementations from other vendors where the FDB lookup
> process
> is completely independent from the Qci stream identification process.
> Anyway, this strategy of combining the two could only work for the
> NULL
> stream identifiers in the first place (MAC DA + VLAN ID), not for the
> others (IP Stream identification etc etc).
> 
> So what remains, if nothing else is possible, is to require the user
> to
> manage the bridge FDB entries, and make sure that the kernel side is
> sane, and does not remain with broken data structures. That is going
> to
> be a PITA both for the user and for the kernel side, because we are
> going to make the tc-flower filters effectively depend upon the
> bridge
> state.
> 
> Let's wait for some feedback from Microchip engineers, how they
> envisioned this to be integrated with operating systems.

We at Microchip agrees that it is a difficult task to map the PSFP
implementation in Felix to the “tc flower” filter command, but please
remember that Ocelot and its derivatives were designed long before
the 802.1Qci standard was ratified and also before anyone has
considered how to control it in Linux.

We think that the best approach is to require the user to manage
bridge FDB entries manually as suggested by Xiaoliang.

Our newer PSFP designs uses the TCAM instead of the MAC table
which maps a lot better to the “tc flower” filter command.


  parent reply	other threads:[~2021-09-09 11:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-31  3:45 [PATCH v3 net-next 0/8] net: dsa: felix: psfp support " Xiaoliang Yang
2021-08-31  3:45 ` [PATCH v3 net-next 1/8] net: mscc: ocelot: export struct ocelot_mact_entry Xiaoliang Yang
2021-08-31  3:45 ` [PATCH v3 net-next 2/8] net: mscc: ocelot: add MAC table write and lookup operations Xiaoliang Yang
2021-08-31  3:45 ` [PATCH v3 net-next 3/8] net: mscc: ocelot: set vcap IS2 chain to goto PSFP chain Xiaoliang Yang
2021-08-31  3:45 ` [PATCH v3 net-next 4/8] net: mscc: ocelot: add gate and police action offload to PSFP Xiaoliang Yang
2021-08-31  3:45 ` [PATCH v3 net-next 5/8] net: dsa: felix: support psfp filter on vsc9959 Xiaoliang Yang
2021-08-31  7:54   ` Vladimir Oltean
2021-08-31  8:41     ` Xiaoliang Yang
2021-08-31  8:46       ` Vladimir Oltean
2021-08-31  8:55         ` Vladimir Oltean
2021-08-31  8:59         ` Xiaoliang Yang
2021-08-31  9:07           ` Vladimir Oltean
2021-08-31  9:18             ` Vladimir Oltean
2021-08-31  9:59               ` Xiaoliang Yang
2021-08-31 10:49                 ` Vladimir Oltean
2021-09-02  3:14                   ` Xiaoliang Yang
2021-09-09 11:33                   ` Joergen Andreasen [this message]
2021-09-09 12:01                     ` Vladimir Oltean
2021-08-31  3:45 ` [PATCH v3 net-next 6/8] net: dsa: felix: add stream gate settings for psfp Xiaoliang Yang
2021-08-31  3:45 ` [PATCH v3 net-next 7/8] net: mscc: ocelot: use index to set vcap policer Xiaoliang Yang
2021-08-31  3:45 ` [PATCH v3 net-next 8/8] net: dsa: felix: use vcap policer to set flow meter for psfp Xiaoliang Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9619848400baaa0d0d12cc6a2d799934323e2657.camel@microchip.com \
    --to=joergen.andreasen@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=allan.nielsen@microchip.com \
    --cc=davem@davemloft.net \
    --cc=idosch@mellanox.com \
    --cc=jiri@mellanox.com \
    --cc=kuba@kernel.org \
    --cc=leoyang.li@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=po.liu@nxp.com \
    --cc=saeedm@mellanox.com \
    --cc=vinicius.gomes@intel.com \
    --cc=vladimir.oltean@nxp.com \
    --cc=xiaoliang.yang_1@nxp.com \
    --subject='Re: [PATCH v3 net-next 5/8] net: dsa: felix: support psfp filter on vsc9959' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).