Messages in this thread | | | Date | Thu, 11 Apr 2024 14:51:58 +0300 | From | Vladimir Oltean <> | Subject | Re: [PATCH net-next v6 3/9] net: add IEEE 802.1q specific helpers |
| |
I have the following comments already written; sending the email mainly to close the window. All of these comments are guarded by a big: "I'm not sure if this really belongs in the kernel." Anyway, here goes.
On Wed, Apr 10, 2024 at 10:05:50AM +0200, Oleksij Rempel wrote: > IEEE 802.1q specification provides recommendation and examples which can > be used as good default values for different drivers. > > This patch implements mapping examples documented in IEEE 802.1Q-2022 in > Annex I "I.3 Traffic type to traffic class mapping" and IETF DSCP naming > and mapping DSCP to Traffic Type inspired by RFC8325. > > This helpers will be used in followup patches for dsa/microchip DCB > implementation. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > diff --git a/include/net/ieee8021q.h b/include/net/ieee8021q.h > new file mode 100644 > index 0000000000000..3bec7ec951362 > --- /dev/null > +++ b/include/net/ieee8021q.h > @@ -0,0 +1,55 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright (c) 2024 Pengutronix, Oleksij Rempel <kernel@pengutronix.de> */ > + > +#ifndef _NET_IEEE8021Q_H > +#define _NET_IEEE8021Q_H > + > +#include <linux/errno.h> > + > +/** > + * enum ieee8021q_traffic_type - 802.1Q traffic type priority values (802.1Q-2022) > + * > + * @IEEE8021Q_TT_BK: Background > + * @IEEE8021Q_TT_BE: Best Effort (default). According to 802.1Q-2022, BE is 0 > + * but has higher priority than BK which is 1. > + * @IEEE8021Q_TT_EE: Excellent Effort > + * @IEEE8021Q_TT_CA: Critical Applications > + * @IEEE8021Q_TT_VI: Video, < 100 ms latency and jitter > + * @IEEE8021Q_TT_VO: Voice, < 10 ms latency and jitter > + * @IEEE8021Q_TT_IC: Internetwork Control > + * @IEEE8021Q_TT_NC: Network Control
We get kernel-doc warnings about IEEE8021Q_TT_MAX not being documented. Simon also suggested to make it private, which I guess will work.
> + */ > +enum ieee8021q_traffic_type { > + 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, > + > + IEEE8021Q_TT_MAX, > +}; > + > +#define SIMPLE_IETF_DSCP_TO_IEEE8021Q_TT(dscp) ((dscp >> 3) & 0x7) > + > +#if IS_ENABLED(CONFIG_NET_IEEE8021Q_HELPERS) > + > +int ietf_dscp_to_ieee8021q_tt(u8 dscp); > +int ieee8021q_tt_to_tc(unsigned int tt, unsigned int num_queues); > + > +#else > + > +static inline int ietf_dscp_to_ieee8021q_tt(int dscp)
Function prototype differs when CONFIG_NET_IEEE8021Q_HELPERS is disabled and when it is enabled (u8 dscp vs int dscp).
> +{ > + return -EOPNOTSUPP; > +} > + > +static inline int ieee8021q_tt_to_tc(int tt, int num_queues)
Same here (unsigned int tt vs int tt).
> +{ > + return -EOPNOTSUPP; > +} > + > +#endif > +#endif /* _NET_IEEE8021Q_H */ > diff --git a/net/Kconfig b/net/Kconfig > index d5ab791f7afa2..f0a8692496ffa 100644 > --- a/net/Kconfig > +++ b/net/Kconfig > @@ -452,6 +452,9 @@ config GRO_CELLS > config SOCK_VALIDATE_XMIT > bool > > +config NET_IEEE8021Q_HELPERS > + bool > + > 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..74b42334746da > --- /dev/null > +++ b/net/core/ieee8021q_helpers.c > @@ -0,0 +1,208 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2024 Pengutronix, Oleksij Rempel <kernel@pengutronix.de> > + > +#include <linux/array_size.h> > +#include <linux/printk.h> > +#include <linux/types.h> > +#include <net/dscp.h> > +#include <net/ieee8021q.h> > + > +/* The following arrays map Traffic Types (TT) to traffic classes (TC) for > + * different 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
s/switch/NIC/, ideally it should be useful beyond switches :)
> + * 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
Needs to also describe the other error case, or be less specific.
> + * -EOPNOTSUPP which is used to indicate that this library function is not > + * compiled in. > + */ > +int ieee8021q_tt_to_tc(unsigned int tt, unsigned int num_queues)
Can the enum ieee8021q_traffic_type be used instead of unsigned int?
> +{ > + if (tt >= IEEE8021Q_TT_MAX) { > + pr_err("Requested Traffic Type (%d) is out of range (%d)\n", tt, > + IEEE8021Q_TT_MAX); > + return -EINVAL; > + } > + > + switch (num_queues) { > + case 8: > + compiletime_assert(ARRAY_SIZE(ieee8021q_8queue_tt_tc_map) != > + IEEE8021Q_TT_MAX - 1, > + "ieee8021q_8queue_tt_tc_map != max - 1"); > + return ieee8021q_8queue_tt_tc_map[tt]; > + case 7: > + compiletime_assert(ARRAY_SIZE(ieee8021q_7queue_tt_tc_map) != > + IEEE8021Q_TT_MAX - 1, > + "ieee8021q_7queue_tt_tc_map != max - 1"); > + > + return ieee8021q_7queue_tt_tc_map[tt]; > + case 6: > + compiletime_assert(ARRAY_SIZE(ieee8021q_6queue_tt_tc_map) != > + IEEE8021Q_TT_MAX - 1, > + "ieee8021q_6queue_tt_tc_map != max - 1"); > + > + return ieee8021q_6queue_tt_tc_map[tt]; > + case 5: > + compiletime_assert(ARRAY_SIZE(ieee8021q_5queue_tt_tc_map) != > + IEEE8021Q_TT_MAX - 1, > + "ieee8021q_5queue_tt_tc_map != max - 1"); > + > + return ieee8021q_5queue_tt_tc_map[tt]; > + case 4: > + compiletime_assert(ARRAY_SIZE(ieee8021q_4queue_tt_tc_map) != > + IEEE8021Q_TT_MAX - 1, > + "ieee8021q_4queue_tt_tc_map != max - 1"); > + > + return ieee8021q_4queue_tt_tc_map[tt]; > + case 3: > + compiletime_assert(ARRAY_SIZE(ieee8021q_3queue_tt_tc_map) != > + IEEE8021Q_TT_MAX - 1, > + "ieee8021q_3queue_tt_tc_map != max - 1"); > + > + return ieee8021q_3queue_tt_tc_map[tt]; > + case 2: > + compiletime_assert(ARRAY_SIZE(ieee8021q_2queue_tt_tc_map) != > + IEEE8021Q_TT_MAX - 1, > + "ieee8021q_2queue_tt_tc_map != max - 1"); > + > + return ieee8021q_2queue_tt_tc_map[tt]; > + case 1: > + compiletime_assert(ARRAY_SIZE(ieee8021q_1queue_tt_tc_map) != > + IEEE8021Q_TT_MAX - 1, > + "ieee8021q_1queue_tt_tc_map != max - 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 > + */ > +int ietf_dscp_to_ieee8021q_tt(u8 dscp) > +{ > + switch (dscp) { > + case DSCP_CS0: > + case DSCP_AF11: > + case DSCP_AF12: > + case DSCP_AF13:
Is it correct for AF11, AF12, AF13 to be classified together with CS0 rather than with CS1? It looks strange when their upper 3 bits are the same as CS1.
> + 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 SIMPLE_IETF_DSCP_TO_IEEE8021Q_TT(dscp); > +} > +EXPORT_SYMBOL_GPL(ietf_dscp_to_ieee8021q_tt); > -- > 2.39.2 >
| |