Received-SPF: pass (google.com: domain of linux-kernel+bounces-137290-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 9 Apr 2024 19:08:13 +0300
From: Vladimir Oltean <olteanv@gmail.com>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: "David S. Miller" <davem@davemloft.net>, Andrew Lunn <andrew@lunn.ch>,
	Eric Dumazet <edumazet@google.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Woojung Huh <woojung.huh@microchip.com>,
	Arun Ramadoss <arun.ramadoss@microchip.com>, kernel@pengutronix.de,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	UNGLinuxDriver@microchip.com, David Ahern <dsahern@kernel.org>,
	Simon Horman <horms@kernel.org>,
	Willem de Bruijn <willemb@google.com>,
	=?utf-8?B?U8O4cmVu?= Andersen <san@skov.dk>
Subject: Re: [PATCH net-next v5 3/9] net: add IEEE 802.1q specific helpers
Message-ID: <20240409160813.c4afvjtgbi76tc3b@skbuf>
References: <20240409081851.3530641-1-o.rempel@pengutronix.de>
 <20240409081851.3530641-4-o.rempel@pengutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240409081851.3530641-4-o.rempel@pengutronix.de>

On Tue, Apr 09, 2024 at 10:18:45AM +0200, Oleksij Rempel wrote:
> diff --git a/net/Kconfig b/net/Kconfig
> index d5ab791f7afa2..b94ee06f28c18 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -452,6 +452,10 @@ config GRO_CELLS
>  config SOCK_VALIDATE_XMIT
>  	bool
>  
> +config NET_IEEE8021Q_HELPERS
> +	bool
> +	default n

default n is implicit, please remove this.

> +
>  config NET_SELFTESTS
>  	def_tristate PHYLIB
>  	depends on PHYLIB && INET
> diff --git a/net/core/Makefile b/net/core/Makefile
> index 21d6fbc7e884c..62be9aef25285 100644
> --- a/net/core/Makefile
> +++ b/net/core/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_NETPOLL) += netpoll.o
>  obj-$(CONFIG_FIB_RULES) += fib_rules.o
>  obj-$(CONFIG_TRACEPOINTS) += net-traces.o
>  obj-$(CONFIG_NET_DROP_MONITOR) += drop_monitor.o
> +obj-$(CONFIG_NET_IEEE8021Q_HELPERS) += ieee8021q_helpers.o
>  obj-$(CONFIG_NET_SELFTESTS) += selftests.o
>  obj-$(CONFIG_NETWORK_PHY_TIMESTAMPING) += timestamping.o
>  obj-$(CONFIG_NET_PTP_CLASSIFY) += ptp_classifier.o
> diff --git a/net/core/ieee8021q_helpers.c b/net/core/ieee8021q_helpers.c
> new file mode 100644
> index 0000000000000..3564dc83d1d3d
> --- /dev/null
> +++ b/net/core/ieee8021q_helpers.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2024 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
> +
> +#include <linux/printk.h>
> +#include <linux/types.h>
> +#include <net/dscp.h>
> +#include <net/ieee8021q.h>
> +
> +/* Following arrays map Traffic Types (TT) to traffic classes (TC) for different

The following

> + * number of queues as shown in the example provided by  IEEE 802.1Q-2022 in
> + * Annex I "I.3 Traffic type to traffic class mapping" and Table I-1 "Traffic
> + * type to traffic class mapping".
> + */
> +static const u8 ieee8021q_8queue_tt_tc_map[] = {
> +	[IEEE8021Q_TT_BK] = 0,
> +	[IEEE8021Q_TT_BE] = 1,
> +	[IEEE8021Q_TT_EE] = 2,
> +	[IEEE8021Q_TT_CA] = 3,
> +	[IEEE8021Q_TT_VI] = 4,
> +	[IEEE8021Q_TT_VO] = 5,
> +	[IEEE8021Q_TT_IC] = 6,
> +	[IEEE8021Q_TT_NC] = 7,
> +};
> +
> +static const u8 ieee8021q_7queue_tt_tc_map[] = {
> +	[IEEE8021Q_TT_BK] = 0,
> +	[IEEE8021Q_TT_BE] = 1,
> +	[IEEE8021Q_TT_EE] = 2,
> +	[IEEE8021Q_TT_CA] = 3,
> +	[IEEE8021Q_TT_VI] = 4,	[IEEE8021Q_TT_VO] = 4,
> +	[IEEE8021Q_TT_IC] = 5,
> +	[IEEE8021Q_TT_NC] = 6,
> +};
> +
> +static const u8 ieee8021q_6queue_tt_tc_map[] = {
> +	[IEEE8021Q_TT_BK] = 0,
> +	[IEEE8021Q_TT_BE] = 1,
> +	[IEEE8021Q_TT_EE] = 2,	[IEEE8021Q_TT_CA] = 2,
> +	[IEEE8021Q_TT_VI] = 3,	[IEEE8021Q_TT_VO] = 3,
> +	[IEEE8021Q_TT_IC] = 4,
> +	[IEEE8021Q_TT_NC] = 5,
> +};
> +
> +static const u8 ieee8021q_5queue_tt_tc_map[] = {
> +	[IEEE8021Q_TT_BK] = 0, [IEEE8021Q_TT_BE] = 0,
> +	[IEEE8021Q_TT_EE] = 1, [IEEE8021Q_TT_CA] = 1,
> +	[IEEE8021Q_TT_VI] = 2, [IEEE8021Q_TT_VO] = 2,
> +	[IEEE8021Q_TT_IC] = 3,
> +	[IEEE8021Q_TT_NC] = 4,
> +};
> +
> +static const u8 ieee8021q_4queue_tt_tc_map[] = {
> +	[IEEE8021Q_TT_BK] = 0, [IEEE8021Q_TT_BE] = 0,
> +	[IEEE8021Q_TT_EE] = 1, [IEEE8021Q_TT_CA] = 1,
> +	[IEEE8021Q_TT_VI] = 2, [IEEE8021Q_TT_VO] = 2,
> +	[IEEE8021Q_TT_IC] = 3, [IEEE8021Q_TT_NC] = 3,
> +};
> +
> +static const u8 ieee8021q_3queue_tt_tc_map[] = {
> +	[IEEE8021Q_TT_BK] = 0, [IEEE8021Q_TT_BE] = 0,
> +	[IEEE8021Q_TT_EE] = 0, [IEEE8021Q_TT_CA] = 0,
> +	[IEEE8021Q_TT_VI] = 1, [IEEE8021Q_TT_VO] = 1,
> +	[IEEE8021Q_TT_IC] = 2, [IEEE8021Q_TT_NC] = 2,
> +};
> +
> +static const u8 ieee8021q_2queue_tt_tc_map[] = {
> +	[IEEE8021Q_TT_BK] = 0, [IEEE8021Q_TT_BE] = 0,
> +	[IEEE8021Q_TT_EE] = 0, [IEEE8021Q_TT_CA] = 0,
> +	[IEEE8021Q_TT_VI] = 1, [IEEE8021Q_TT_VO] = 1,
> +	[IEEE8021Q_TT_IC] = 1, [IEEE8021Q_TT_NC] = 1,
> +};
> +
> +static const u8 ieee8021q_1queue_tt_tc_map[] = {
> +	[IEEE8021Q_TT_BK] = 0, [IEEE8021Q_TT_BE] = 0,
> +	[IEEE8021Q_TT_EE] = 0, [IEEE8021Q_TT_CA] = 0,
> +	[IEEE8021Q_TT_VI] = 0, [IEEE8021Q_TT_VO] = 0,
> +	[IEEE8021Q_TT_IC] = 0, [IEEE8021Q_TT_NC] = 0,
> +};
> +
> +/**
> + * ieee8021q_tt_to_tc - Map IEEE 802.1Q Traffic Type to Traffic Class
> + * @tt: IEEE 802.1Q Traffic Type
> + * @num_queues: Number of queues
> + *
> + * This function maps an IEEE 802.1Q Traffic Type to a Traffic Class (TC) based
> + * on the number of queues configured on the switch. The mapping is based on the
> + * example provided by IEEE 802.1Q-2022 in Annex I "I.3 Traffic type to traffic
> + * class mapping" and Table I-1 "Traffic type to traffic class mapping".
> + *
> + * Return: Traffic Class corresponding to the given Traffic Type or -EINVAL if
> + * the number of queues is not supported. -EINVAL is used to differentiate from
> + * -EOPNOTSUPP which is used to indicate that this library function is not
> + * compiled in.
> + */
> +int ieee8021q_tt_to_tc(int tt, int num_queues)
> +{

Range validation for tt? You index an array with it.

> +	switch (num_queues) {
> +	case 8:
> +		return ieee8021q_8queue_tt_tc_map[tt];
> +	case 7:
> +		return ieee8021q_7queue_tt_tc_map[tt];
> +	case 6:
> +		return ieee8021q_6queue_tt_tc_map[tt];
> +	case 5:
> +		return ieee8021q_5queue_tt_tc_map[tt];
> +	case 4:
> +		return ieee8021q_4queue_tt_tc_map[tt];
> +	case 3:
> +		return ieee8021q_3queue_tt_tc_map[tt];
> +	case 2:
> +		return ieee8021q_2queue_tt_tc_map[tt];
> +	case 1:
> +		return ieee8021q_1queue_tt_tc_map[tt];
> +	}
> +
> +	pr_err("Invalid number of queues %d\n", num_queues);
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(ieee8021q_tt_to_tc);
> +
> +/**
> + * ietf_dscp_to_ieee8021q_tt - Map IETF DSCP to IEEE 802.1Q Traffic Type
> + * @dscp: IETF DSCP value
> + *
> + * This function maps an IETF DSCP value to an IEEE 802.1Q Traffic Type (TT).
> + * Since there is no corresponding mapping between DSCP and IEEE 802.1Q Traffic
> + * Type, this function is inspired by the RFC8325 documentation which describe
> + * the mapping between DSCP and 802.11 User Priority (UP) values.
> + *
> + * Return: IEEE 802.1Q Traffic Type corresponding to the given DSCP value
> + */
> +

Extraneous blank line.

> +int ietf_dscp_to_ieee8021q_tt(int dscp)
> +{
> +	switch (dscp) {
> +	case DSCP_CS0:
> +	case DSCP_AF11:
> +	case DSCP_AF12:
> +	case DSCP_AF13:
> +		return IEEE8021Q_TT_BE;
> +	case DSCP_CS1:
> +		return IEEE8021Q_TT_BK;
> +	case DSCP_CS2:
> +	case DSCP_AF21:
> +	case DSCP_AF22:
> +	case DSCP_AF23:
> +		return IEEE8021Q_TT_EE;
> +	case DSCP_CS3:
> +	case DSCP_AF31:
> +	case DSCP_AF32:
> +	case DSCP_AF33:
> +		return IEEE8021Q_TT_CA;
> +	case DSCP_CS4:
> +	case DSCP_AF41:
> +	case DSCP_AF42:
> +	case DSCP_AF43:
> +		return IEEE8021Q_TT_VI;
> +	case DSCP_CS5:
> +	case DSCP_EF:
> +	case DSCP_VOICE_ADMIT:
> +		return IEEE8021Q_TT_VO;
> +	case DSCP_CS6:
> +		return IEEE8021Q_TT_IC;
> +	case DSCP_CS7:
> +		return IEEE8021Q_TT_NC;
> +	}
> +
> +	return (dscp >> 3) & 0x7;

Use a macro for this operation rather than "magic numbers"?

> +}
> +EXPORT_SYMBOL_GPL(ietf_dscp_to_ieee8021q_tt);
> -- 
> 2.39.2
>