All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
To: "Allan W. Nielsen" <allan.nielsen@microchip.com>,
	Vladimir Oltean <olteanv@gmail.com>
Cc: Po Liu <po.liu@nxp.com>, Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandru Marginean <alexandru.marginean@nxp.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Leo Li <leoyang.li@nxp.com>, Mingkai Hu <mingkai.hu@nxp.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jiri Pirko <jiri@resnulli.us>, Ido Schimmel <idosch@idosch.org>,
	Jakub Kicinski <kuba@kernel.org>, netdev <netdev@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Joergen Andreasen <joergen.andreasen@microchip.com>,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
	Nikolay Aleksandrov <nikolay@cumulusnetworks.com>,
	Roopa Prabhu <roopa@cumulusnetworks.com>,
	"linux-devel@linux.nxdi.nxp.com" <linux-devel@linux.nxdi.nxp.com>
Subject: RE: [EXT] Re: [PATCH v1 net-next 4/6] net: mscc: ocelot: VCAP IS1 support
Date: Thu, 7 May 2020 11:23:14 +0000	[thread overview]
Message-ID: <DB8PR04MB5785BE9AC6FAC6F395C8A20DF0A50@DB8PR04MB5785.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20200506211551.cf6mlad7ysmuqfvq@ws.localdomain>

Hi Allan,


> Hi Vladimir,
> 
> On 06.05.2020 13:53, Vladimir Oltean wrote:
[snip]
> >At the moment, the driver does not support more than 1 action. We might 
> >need to change that, but we can still install more filters with the 
> >same key and still be fine (see more below). When there is more than 1 
> >action, the IS1 stuff will be combined into a single rule programmed 
> >into IS1, and the IS2 stuff will be combined into a single new rule 
> >with the same keys installed into VCAP IS2. Would that not work?
> >
> >> The SW model have these two rules in the same table, and can stop 
> >> process at the first match. SW will do the action of the first frame 
> >> matching.
> >>
> >
> >Actually I think this is an incorrect assumption - software stops at 
> >the first action only if told to do so. Let me copy-paste a text from a 
> >different email thread.
> 
> I'm still not able to see how this proposal will give us the same behavioral in SW and in HW.
> 
> A simple example:
> 
> tc qdisc add dev enp0s3 ingress
> tc filter add dev enp0s3 protocol 802.1Q parent ffff: \
>      prio 10 flower vlan_id 5 action vlan modify id 10 tc filter add dev enp0s3 protocol 802.1Q parent ffff: \
>      prio 20 flower src_mac 00:00:00:00:00:08 action drop
> 
> We can then inject a frame with VID 5 and smac ::08:
> $ ef tx tap0 eth smac 00:00:00:00:00:08 ctag vid 5
> 
> We can then check the filter and see that it only hit the first rule:
> 
> $ tc -s filter show dev enp0s3 ingress
> filter protocol 802.1Q pref 10 flower chain 0 filter protocol 802.1Q pref 10 flower chain 0 handle 0x1
>    vlan_id 5
>    not_in_hw
>          action order 1: vlan  modify id 10 protocol 802.1Q priority 0 pipe
>           index 1 ref 1 bind 1 installed 19 sec used 6 sec
>          Action statistics:
>          Sent 42 bytes 1 pkt (dropped 0, overlimits 0 requeues 0)
>          backlog 0b 0p requeues 0
>
> filter protocol 802.1Q pref 20 flower chain 0 filter protocol 802.1Q pref 20 flower chain 0 handle 0x1
>   src_mac 00:00:00:00:00:08
>   not_in_hw
>         action order 1: gact action drop
>          random type none pass val 0
>          index 1 ref 1 bind 1 installed 11 sec used 11 sec
>         Action statistics:
>         Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>         backlog 0b 0p requeues 0
>
> If this was done with the proposed HW offload, then both rules would have been hit and we would have a different behavioral.
>
> This can be fixed by adding the "continue" action to the first rule:

> tc filter add dev enp0s3 protocol 802.1Q parent ffff: \
>      prio 10 flower vlan_id 5 action vlan modify id 10 continue tc filter add dev enp0s3 protocol 802.1Q parent ffff: \
>      prio 20 flower src_mac 00:00:00:00:00:08 action drop
>
> But that would again break if we add 2 rules manipulating the VLAN (as the HW does not continue with in a single TCAM).
>
> My point is: I do not think we can hide the fact that this is done in independent TCAMs in the silicon.
> 
> I think it is possible to do this with the chain feature (even though it is not a perfect match), but it would require more analysis.
> 
> /Allan

Do you mean it's better to set vlan modify filters in a different chain, and write the filter entries with a same chain in the same VCAP TCAM?
For example:
	tc filter add dev enp0s3 protocol 802.1Q chain 11 parent ffff: prio 10 flower skip_sw vlan_id 5 action vlan modify id 10
	tc filter add dev enp0s3 protocol 802.1Q chain 22 parent ffff: prio 20 flower skip_sw src_mac 00:00:00:00:00:08 action drop
for this usage, we only need to ensure a chain corresponding to a VCAP in ocelot ace driver. I'm not sure is my understanding right?

regards,
Xiaoliang

  reply	other threads:[~2020-05-07 11:23 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-06  7:48 [PATCH v1 net-next 0/6] net: ocelot: VCAP IS1 and ES0 support Xiaoliang Yang
2020-05-06  7:48 ` [PATCH v1 net-next 1/6] net: mscc: ocelot: introduce a new ocelot_target_{read,write} API Xiaoliang Yang
2020-05-06  7:48 ` [PATCH v1 net-next 2/6] net: mscc: ocelot: generalize existing code for VCAP IS2 Xiaoliang Yang
2020-05-06  7:48 ` [PATCH v1 net-next 3/6] net: mscc: ocelot: change vcap to be compatible with full and quad entry Xiaoliang Yang
2020-05-06  7:48 ` [PATCH v1 net-next 4/6] net: mscc: ocelot: VCAP IS1 support Xiaoliang Yang
2020-05-06  9:43   ` Allan W. Nielsen
2020-05-06 10:53     ` Vladimir Oltean
2020-05-06 21:15       ` Allan W. Nielsen
2020-05-07 11:23         ` Xiaoliang Yang [this message]
2020-05-07 19:29           ` [EXT] " Allan W. Nielsen
2020-05-06  7:48 ` [PATCH v1 net-next 5/6] net: mscc: ocelot: VCAP ES0 support Xiaoliang Yang
2020-05-06  7:49 ` [PATCH v1 net-next 6/6] net: dsa: tag_ocelot: use VLAN information from tagging header when available 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=DB8PR04MB5785BE9AC6FAC6F395C8A20DF0A50@DB8PR04MB5785.eurprd04.prod.outlook.com \
    --to=xiaoliang.yang_1@nxp.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=alexandru.marginean@nxp.com \
    --cc=allan.nielsen@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=horatiu.vultur@microchip.com \
    --cc=idosch@idosch.org \
    --cc=jiri@resnulli.us \
    --cc=joergen.andreasen@microchip.com \
    --cc=kuba@kernel.org \
    --cc=leoyang.li@nxp.com \
    --cc=linux-devel@linux.nxdi.nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingkai.hu@nxp.com \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=olteanv@gmail.com \
    --cc=po.liu@nxp.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.