Skip to content

Conversation

ZhaoQiang-b45475
Copy link
Contributor

@ZhaoQiang-b45475 ZhaoQiang-b45475 commented Sep 17, 2025

Add Qbv support, Qbv(IEEE 802.1Qbv) is Enhancements for Scheduled Traffic (EST), one
feature of TSN. The PTP clock provides the time reference for Qbv
In subsys/net/l2/ethernet/Kconfig, add NET_QBV config
In driver, add set_config/get_config to set and get Qbv configuration.
support enable/disable, set/get times, set/get list length and
set/get gate control list.
Add a Qbv shell cmd to set/get qbv

if (ret < 0) {
return ret;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For each command, we should probably verify that the interface is Ethernet type.
Why force user to use the device as a parameter if we are using interface internally? It should make things clearer if user just uses network interface index to configure Qbv.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not resolve the configuration but let the reviewer do it. See https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers for details.

We should add here the interface type check, see my comment below.

@ZhaoQiang-b45475 ZhaoQiang-b45475 force-pushed the support-qbv branch 2 times, most recently from 8bc2f6c to f440963 Compare September 18, 2025 03:04
@zephyrbot zephyrbot requested a review from jukkar September 18, 2025 03:05
@ZhaoQiang-b45475 ZhaoQiang-b45475 force-pushed the support-qbv branch 2 times, most recently from af4702c to 95da213 Compare September 18, 2025 03:16
Copy link
Member

@yangbolu1991 yangbolu1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comments for DSA part.
Thanks.

@nashif
Copy link
Member

nashif commented Sep 25, 2025

Support Qbv

is this for IEEE 802.1Qbv Ethernet? Can you please change the title and body with more detais?

@nashif nashif changed the title Support qbv Add Qbv support, Qbv is Enhancements for Scheduled Traffic (EST) Sep 25, 2025
@zephyrbot zephyrbot requested a review from jukkar September 29, 2025 06:27
@ZhaoQiang-b45475 ZhaoQiang-b45475 force-pushed the support-qbv branch 3 times, most recently from d1e9dd6 to 3bafe36 Compare September 29, 2025 07:03
Comment on lines 153 to 159
config NET_SHELL_QBV_SUPPORTED
bool "Qbv Shell"
depends on NET_L2_ETHERNET_MGMT
help
Enable Qbv Shell.
The Qbv shell currently supports set/enable operations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not what I meant. The supported option should look similar as the other options around it.

config NET_SHELL_QBV_SUPPORTED
	bool "Qbv shell"
	default y
	depends on NET_SHELL_SHOW_DISABLED_COMMANDS || NET_QBV

if (ret < 0) {
return ret;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not resolve the configuration but let the reviewer do it. See https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers for details.

We should add here the interface type check, see my comment below.

PR_WARNING("No such interface in index %d\n", idx);
return -ENOEXEC;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is missing here is verification that the interface is Ethernet type. That can be done like this

	if (net_if_l2(iface) != &NET_L2_GET_NAME(ETHERNET)) {
		PR_WARNING("Qbv can be set only for Ethernet\n");
		return -ENOEXEC;
	}

PR_WARNING("No such interface in index %d\n", idx);
return -ENOEXEC;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interface type check should be added here too. Perhaps you could refactor code for the idx, iface and the interface type check into separate checker function and call that for each sub-shell command.

/* qbv set_config <iface_index> <row> <gate_control> <interval> */
static int cmd_qbv_set_gc(const struct shell *sh, size_t argc, char **argv)
{
struct net_if *iface;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you change each sub-command to work like this:

static int cmd_qbv_xxx(const struct ...., )
{
#if defined(CONFIG_NET_QBV)

#else
	PR_INFO("Set %s to enable %s support.\n", "CONFIG_NET_QBV", "Qbv");
#endif
}

This way user is informed what options to set if Qbv is not enabled. And the code is compiled out if CONFIG_NET_SHELL_QBV_SUPPORTED is turned off (set to n, it is y by default).

Supported set/get_config API.

Signed-off-by: Qiang Zhao <[email protected]>
Add NET_QBV in Kconfig, Qbv is Enhancements for Scheduled Traffic (EST),
one feature of TSN. The PTP clock provides the time reference for Qbv

Signed-off-by: Qiang Zhao <[email protected]>
Add DSA Qbv support, add set_config/get_config to set and get
Qbv configuration. support enable/disable, set/get times,
set/get list length and set/get gate control list.

Signed-off-by: Qiang Zhao <[email protected]>
Added Qbv shell subcommand to net command.
Supported enable, set_config, set_time and get_info functions.

Signed-off-by: Qiang Zhao <[email protected]>
add Qbv capability for dsa_nxp_imx_netc

Signed-off-by: Qiang Zhao <[email protected]>
Copy link

sonarqubecloud bot commented Oct 9, 2025

#ifdef CONFIG_NET_QBV
memset(&(prv->qbv_config[cfg->port_idx].tgs_config), 0, sizeof(netc_tb_tgs_gcl_t));
memset(prv->qbv_config[cfg->port_idx].gcList, 0,
sizeof(netc_tgs_gate_entry_t) * CONFIG_DSA_NXP_NETC_GCL_LEN);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about replace these two memset with the suggested code at: line 476

if (row > CONFIG_DSA_NXP_NETC_GCL_LEN) {
LOG_ERR("The gate control list length exceeds the limit");
ret = -ENOTSUP;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
break;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will fix next version

if (row > CONFIG_DSA_NXP_NETC_GCL_LEN) {
LOG_ERR("The gate control list length exceeds the limit");
ret = -ENOTSUP;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
break;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will fix next version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants