lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 11 May 2020 11:19:28 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Xiaoliang Yang <xiaoliang.yang_1@....com>
Cc:     Po Liu <po.liu@....com>, Claudiu Manoil <claudiu.manoil@....com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Li Yang <leoyang.li@....com>, Mingkai Hu <mingkai.hu@....com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jiri Pirko <jiri@...nulli.us>,
        Ido Schimmel <idosch@...sch.org>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Horatiu Vultur <horatiu.vultur@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "Allan W. Nielsen" <allan.nielsen@...rochip.com>,
        Joergen Andreasen <joergen.andreasen@...rochip.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Vinicius Costa Gomes <vinicius.gomes@...el.com>,
        Nikolay Aleksandrov <nikolay@...ulusnetworks.com>,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        linux-devel@...ux.nxdi.nxp.com
Subject: Re: [PATCH v1 net-next 1/3] net: dsa: felix: qos classified based on pcp

Hi Jiri, Jakub, Ido,

On Mon, 11 May 2020 at 08:50, Xiaoliang Yang <xiaoliang.yang_1@....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@....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

Powered by blists - more mailing lists