Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.117.160 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.117.160; helo=mail.nvidia.com; pr=C
References: <20220929185207.2183473-1-daniel.machon@microchip.com>
 <20220929185207.2183473-5-daniel.machon@microchip.com>
User-agent: mu4e 1.6.6; emacs 28.1
From:   Petr Machata <petrm@nvidia.com>
To:     Daniel Machon <daniel.machon@microchip.com>
CC:     <netdev@vger.kernel.org>, <davem@davemloft.net>,
        <petrm@nvidia.com>, <maxime.chevallier@bootlin.com>,
        <thomas.petazzoni@bootlin.com>, <edumazet@google.com>,
        <kuba@kernel.org>, <pabeni@redhat.com>,
        <lars.povlsen@microchip.com>, <Steen.Hegelund@microchip.com>,
        <UNGLinuxDriver@microchip.com>, <joe@perches.com>,
        <linux@armlinux.org.uk>, <horatiu.vultur@microchip.com>,
        <Julia.Lawall@inria.fr>, <vladimir.oltean@nxp.com>,
        <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH net-next v2 4/6] net: microchip: sparx5: add support for
 apptrust
Date:   Fri, 30 Sep 2022 17:49:15 +0200
In-Reply-To: <20220929185207.2183473-5-daniel.machon@microchip.com>
Message-ID: <87zgegu9kq.fsf@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Sep 2022 16:11:19.6107
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: dbcc487e-b23f-421a-1e6f-08daa2fe6949
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.160];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: BL02EPF0000C409.namprd05.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5149
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


Daniel Machon <daniel.machon@microchip.com> writes:

> Make use of set/getapptrust() to implement per-selector trust and trust
> order.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  .../ethernet/microchip/sparx5/sparx5_dcb.c    | 116 ++++++++++++++++++
>  .../ethernet/microchip/sparx5/sparx5_main.h   |   3 +
>  .../ethernet/microchip/sparx5/sparx5_port.c   |   4 +-
>  .../ethernet/microchip/sparx5/sparx5_port.h   |   2 +
>  .../ethernet/microchip/sparx5/sparx5_qos.c    |   4 +
>  5 files changed, 127 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> index db17c124dac8..10aeb422b1ae 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_dcb.c
> @@ -8,6 +8,22 @@
>
>  #include "sparx5_port.h"
>
> +static const struct sparx5_dcb_apptrust {
> +	u8 selectors[256];
> +	int nselectors;
> +	char *names;

I think this should be just "name".

> +} *apptrust[SPX5_PORTS];
> +
> +/* Sparx5 supported apptrust configurations */
> +static const struct sparx5_dcb_apptrust apptrust_conf[4] = {
> +	/* Empty *must* be first */
> +	{ { 0                         }, 0, "empty"    },
> +	{ { IEEE_8021QAZ_APP_SEL_DSCP }, 1, "dscp"     },
> +	{ { DCB_APP_SEL_PCP           }, 1, "pcp"      },
> +	{ { IEEE_8021QAZ_APP_SEL_DSCP,
> +	    DCB_APP_SEL_PCP           }, 2, "dscp pcp" },
> +};
> +
>  /* Validate app entry.
>   *
>   * Check for valid selectors and valid protocol and priority ranges.
> @@ -37,12 +53,59 @@ static int sparx5_dcb_app_validate(struct net_device *dev,
>  	return err;
>  }
>
> +/* Validate apptrust configuration.
> + *
> + * Return index of supported apptrust configuration if valid, otherwise return
> + * error.
> + */
> +static int sparx5_dcb_apptrust_validate(struct net_device *dev, u8 *selectors,
> +					int nselectors, int *err)
> +{
> +	bool match;
> +	int i, ii;
> +
> +	for (i = 0; i < ARRAY_SIZE(apptrust_conf); i++) {

I would do this here:

    if (apptrust_conf[i].nselectors != nselectors) continue;

to avoid having to think about the semantics of comparing to all those
zeroes in apptrust_conf.selectors array.

> +		match = true;
> +		for (ii = 0; ii < nselectors; ii++) {
> +			if (apptrust_conf[i].selectors[ii] !=
> +			    *(selectors + ii)) {
> +				match = false;
> +				break;
> +			}
> +		}
> +		if (match)
> +			break;
> +	}
> +
> +	/* Requested trust configuration is not supported */
> +	if (!match) {
> +		netdev_err(dev, "Valid apptrust configurations are:\n");
> +		for (i = 0; i < ARRAY_SIZE(apptrust_conf); i++)
> +			pr_info("order: %s\n", apptrust_conf[i].names);
> +		*err = -EOPNOTSUPP;
> +	}
> +
> +	return i;
> +}
> +
> +static bool sparx5_dcb_apptrust_contains(int portno, u8 selector)
> +{
> +	int i;
> +
> +	for (i = 0; i < IEEE_8021QAZ_APP_SEL_MAX + 1; i++)
> +		if (apptrust[portno]->selectors[i] == selector)
> +			return true;
> +
> +	return false;
> +}
> +
>  static int sparx5_dcb_app_update(struct net_device *dev)
>  {
>  	struct dcb_app app_itr = { .selector = DCB_APP_SEL_PCP };
>  	struct sparx5_port *port = netdev_priv(dev);
>  	struct sparx5_port_qos_pcp_map *pcp_map;
>  	struct sparx5_port_qos qos = {0};
> +	int portno = port->portno;
>  	int i;
>
>  	pcp_map = &qos.pcp.map;
> @@ -53,6 +116,12 @@ static int sparx5_dcb_app_update(struct net_device *dev)
>  		pcp_map->map[i] = dcb_getapp(dev, &app_itr);
>  	}
>
> +	/* Enable use of pcp for queue classification ? */
> +	if (sparx5_dcb_apptrust_contains(portno, DCB_APP_SEL_PCP)) {
> +		qos.pcp.qos_enable = true;
> +		qos.pcp.dp_enable = qos.pcp.qos_enable;
> +	}
> +
>  	return sparx5_port_qos_set(port, &qos);
>  }
>
> @@ -95,7 +164,54 @@ static int sparx5_dcb_ieee_delapp(struct net_device *dev, struct dcb_app *app)
>  	return sparx5_dcb_app_update(dev);
>  }
>
> +static int sparx5_dcb_setapptrust(struct net_device *dev, u8 *selectors,
> +				  int nselectors)
> +{
> +	struct sparx5_port *port = netdev_priv(dev);
> +	int err = 0, idx;
> +
> +	idx = sparx5_dcb_apptrust_validate(dev, selectors, nselectors, &err);
> +	if (err < 0)
> +		return err;
> +
> +	apptrust[port->portno] = &apptrust_conf[idx];
> +
> +	return sparx5_dcb_app_update(dev);
> +}
> +
> +static int sparx5_dcb_getapptrust(struct net_device *dev, u8 *selectors,
> +				  int *nselectors)
> +{
> +	struct sparx5_port *port = netdev_priv(dev);
> +	const struct sparx5_dcb_apptrust *trust;
> +
> +	trust = apptrust[port->portno];
> +
> +	memcpy(selectors, trust->selectors, trust->nselectors);
> +	*nselectors = trust->nselectors;
> +
> +	return 0;
> +}
> +
> +int sparx5_dcb_init(struct sparx5 *sparx5)
> +{
> +	struct sparx5_port *port;
> +	int i;
> +
> +	for (i = 0; i < SPX5_PORTS; i++) {
> +		port = sparx5->ports[i];
> +		if (!port)
> +			continue;
> +		/* Initialize [dscp, pcp] default trust */
> +		apptrust[port->portno] = &apptrust_conf[3];
> +	}
> +
> +	return sparx5_dcb_app_update(port->ndev);
> +}
> +
>  const struct dcbnl_rtnl_ops sparx5_dcbnl_ops = {
>  	.ieee_setapp = sparx5_dcb_ieee_setapp,
>  	.ieee_delapp = sparx5_dcb_ieee_delapp,
> +	.dcbnl_setapptrust = sparx5_dcb_setapptrust,
> +	.dcbnl_getapptrust = sparx5_dcb_getapptrust,
>  };
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index 0d8e04c64584..d07ef2e8b321 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -357,6 +357,9 @@ int sparx5_config_dsm_calendar(struct sparx5 *sparx5);
>  void sparx5_get_stats64(struct net_device *ndev, struct rtnl_link_stats64 *stats);
>  int sparx_stats_init(struct sparx5 *sparx5);
>
> +/* sparx5_dcb.c */
> +int sparx5_dcb_init(struct sparx5 *sparx5);
> +
>  /* sparx5_netdev.c */
>  void sparx5_set_port_ifh_timestamp(void *ifh_hdr, u64 timestamp);
>  void sparx5_set_port_ifh_rew_op(void *ifh_hdr, u32 rew_op);
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
> index 9ffaaf34d196..99e86e87aa16 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.c
> @@ -1163,8 +1163,8 @@ int sparx5_port_qos_pcp_set(const struct sparx5_port *port,
>  	int i;
>
>  	/* Enable/disable pcp and dp for qos classification. */
> -	spx5_rmw(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_SET(1) |
> -		 ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_SET(1),
> +	spx5_rmw(ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA_SET(qos->qos_enable) |
> +		 ANA_CL_QOS_CFG_PCP_DEI_DP_ENA_SET(qos->dp_enable),
>  		 ANA_CL_QOS_CFG_PCP_DEI_QOS_ENA | ANA_CL_QOS_CFG_PCP_DEI_DP_ENA,
>  		 sparx5, ANA_CL_QOS_CFG(port->portno));
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
> index 9c5fb6b651db..fae9f5464548 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_port.h
> @@ -97,6 +97,8 @@ struct sparx5_port_qos_pcp_map {
>
>  struct sparx5_port_qos_pcp {
>  	struct sparx5_port_qos_pcp_map map;
> +	bool qos_enable;
> +	bool dp_enable;
>  };
>
>  struct sparx5_port_qos {
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
> index 1e79d0ef0cb8..379e540e5e6a 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_qos.c
> @@ -389,6 +389,10 @@ int sparx5_qos_init(struct sparx5 *sparx5)
>  	if (ret < 0)
>  		return ret;
>
> +	ret = sparx5_dcb_init(sparx5);
> +	if (ret < 0)
> +		return ret;
> +
>  	return 0;
>  }