From: Vladimir Oltean <olteanv@gmail.com>
To: Xiaoliang Yang <xiaoliang.yang_1@nxp.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>,
Li Yang <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>,
"Allan W. Nielsen" <allan.nielsen@microchip.com>,
Joergen Andreasen <joergen.andreasen@microchip.com>,
Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
Vinicius Costa Gomes <vinicius.gomes@intel.com>,
Nikolay Aleksandrov <nikolay@cumulusnetworks.com>,
Roopa Prabhu <roopa@cumulusnetworks.com>,
linux-devel@linux.nxdi.nxp.com
Subject: Re: [PATCH v1 net-next 1/3] net: dsa: felix: qos classified based on pcp
Date: Mon, 11 May 2020 11:19:28 +0300 [thread overview]
Message-ID: <CA+h21hpennftjgTr_CK85drFUErQUqZkcFA+zPe0L25VAbe=FA@mail.gmail.com> (raw)
In-Reply-To: <20200511054332.37690-2-xiaoliang.yang_1@nxp.com>
Hi Jiri, Jakub, Ido,
On Mon, 11 May 2020 at 08:50, Xiaoliang Yang <xiaoliang.yang_1@nxp.com> wrote:
>
> Set the default QoS Classification based on PCP and DEI of vlan tag,
> after that, frames can be Classified to different Qos based on PCP tag.
> If there is no vlan tag or vlan ignored, use port default Qos.
>
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> ---
The new skbedit priority offload action looks interesting to me.
But it also raises the question of what to do in the default case
where such rules are not installed. I think it is ok to support a
1-to-1 VLAN PCP to TC mapping by default? This should also be needed
for features such as Priority Flow Control.
> drivers/net/dsa/ocelot/felix.c | 6 ++++++
> drivers/net/dsa/ocelot/felix.h | 1 +
> drivers/net/dsa/ocelot/felix_vsc9959.c | 23 +++++++++++++++++++++++
> 3 files changed, 30 insertions(+)
>
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index a2dfd73f8a1a..0afdc6fc3f57 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -547,6 +547,12 @@ static int felix_setup(struct dsa_switch *ds)
> ocelot_configure_cpu(ocelot, port,
> OCELOT_TAG_PREFIX_NONE,
> OCELOT_TAG_PREFIX_LONG);
> +
> + /* Set the default QoS Classification based on PCP and DEI
> + * bits of vlan tag.
> + */
> + if (felix->info->port_qos_map_init)
> + felix->info->port_qos_map_init(ocelot, port);
Xiaoliang, just a small comment in case you need to resend.
The felix->info structure is intended to hold SoC-specific data that
is likely to differ between chips (like for example if vsc7511 support
ever appears in felix). But I see ANA:PORT:QOS_CFG and
ANA:PORT:QOS_PCP_DEI_MAP_CFG are common registers, so are there any
specific reasons why you put this in felix_vsc9959 and not in the
common ocelot code?
> }
>
> /* Include the CPU port module in the forwarding mask for unknown
> diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
> index b94386fa8d63..0d4ec34309c7 100644
> --- a/drivers/net/dsa/ocelot/felix.h
> +++ b/drivers/net/dsa/ocelot/felix.h
> @@ -35,6 +35,7 @@ struct felix_info {
> struct phylink_link_state *state);
> int (*prevalidate_phy_mode)(struct ocelot *ocelot, int port,
> phy_interface_t phy_mode);
> + void (*port_qos_map_init)(struct ocelot *ocelot, int port);
> };
>
> extern struct felix_info felix_info_vsc9959;
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index 1c56568d5aca..5c931fb3e4cd 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> @@ -4,6 +4,7 @@
> */
> #include <linux/fsl/enetc_mdio.h>
> #include <soc/mscc/ocelot_vcap.h>
> +#include <soc/mscc/ocelot_ana.h>
> #include <soc/mscc/ocelot_sys.h>
> #include <soc/mscc/ocelot.h>
> #include <linux/iopoll.h>
> @@ -1209,6 +1210,27 @@ static void vsc9959_mdio_bus_free(struct ocelot *ocelot)
> mdiobus_unregister(felix->imdio);
> }
>
> +static void vsc9959_port_qos_map_init(struct ocelot *ocelot, int port)
> +{
> + int i;
> +
> + ocelot_rmw_gix(ocelot,
> + ANA_PORT_QOS_CFG_QOS_PCP_ENA,
> + ANA_PORT_QOS_CFG_QOS_PCP_ENA,
> + ANA_PORT_QOS_CFG,
> + port);
> +
> + for (i = 0; i < FELIX_NUM_TC * 2; i++) {
> + ocelot_rmw_ix(ocelot,
> + (ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL & i) |
ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL is 1 bit. Are you sure this should
be % i and not % 2?
> + ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL(i),
> + ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL |
> + ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL_M,
> + ANA_PORT_PCP_DEI_MAP,
> + port, i);
> + }
> +}
> +
> struct felix_info felix_info_vsc9959 = {
> .target_io_res = vsc9959_target_io_res,
> .port_io_res = vsc9959_port_io_res,
> @@ -1232,4 +1254,5 @@ struct felix_info felix_info_vsc9959 = {
> .pcs_an_restart = vsc9959_pcs_an_restart,
> .pcs_link_state = vsc9959_pcs_link_state,
> .prevalidate_phy_mode = vsc9959_prevalidate_phy_mode,
> + .port_qos_map_init = vsc9959_port_qos_map_init,
> };
> --
> 2.17.1
>
Thanks,
-Vladimir
next prev parent reply other threads:[~2020-05-11 8:19 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-11 5:43 [PATCH v1 net-next 0/3] net: dsa: felix: tc taprio and CBS offload support Xiaoliang Yang
2020-05-11 5:43 ` [PATCH v1 net-next 1/3] net: dsa: felix: qos classified based on pcp Xiaoliang Yang
2020-05-11 8:19 ` Vladimir Oltean [this message]
2020-05-12 6:21 ` [EXT] " Xiaoliang Yang
2020-05-11 5:43 ` [PATCH v1 net-next 2/3] net: dsa: felix: Configure Time-Aware Scheduler via taprio offload Xiaoliang Yang
2020-05-11 23:52 ` David Miller
2020-05-12 0:10 ` Vladimir Oltean
2020-05-11 5:43 ` [PATCH v1 net-next 3/3] net: dsa: felix: add support Credit Based Shaper(CBS) for hardware offload Xiaoliang Yang
2020-05-11 22:34 ` Jakub Kicinski
2020-05-12 6:27 ` [EXT] " Xiaoliang Yang
2020-05-12 1:41 ` Vinicius Costa Gomes
2020-05-12 9:17 ` [EXT] " 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='CA+h21hpennftjgTr_CK85drFUErQUqZkcFA+zPe0L25VAbe=FA@mail.gmail.com' \
--to=olteanv@gmail.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=po.liu@nxp.com \
--cc=roopa@cumulusnetworks.com \
--cc=vinicius.gomes@intel.com \
--cc=vivien.didelot@gmail.com \
--cc=vladimir.oltean@nxp.com \
--cc=xiaoliang.yang_1@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 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).