DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Xingui Yang <yangxingui@huawei.com>
Cc: <dev@dpdk.org>, <david.marchand@redhat.com>,
	<aman.deep.singh@intel.com>, <fengchengwen@huawei.com>,
	<yangshuaisong@h-partners.com>, <lihuisong@huawei.com>,
	<liuyonglong@huawei.com>, <kangfenglong@huawei.com>
Subject: Re: [PATCH v2] app/testpmd: add VLAN priority insert support
Date: Tue, 16 Jun 2026 07:23:28 -0700	[thread overview]
Message-ID: <20260616072328.1dcb8cf8@phoenix.local> (raw)
In-Reply-To: <20260616131001.2955655-1-yangxingui@huawei.com>

On Tue, 16 Jun 2026 21:10:01 +0800
Xingui Yang <yangxingui@huawei.com> wrote:

> The tx_vlan set and tx_qinq set commands only accepted VLAN ID in range
> [0, 4095]. This prevented users from setting 802.1p priority and CFI
> bits when using hardware VLAN insertion.
> 
> Since mbuf vlan_tci field already supports full 16-bit VLAN Tag Control
> Information (TCI), relax the validation for TX paths to allow priority
> and CFI bits. The vlan_id parameter now accepts:
>   - Bits 0-11:  VLAN ID (0-4095)
>   - Bit 12:    CFI (Canonical Format Indicator)
>   - Bits 13-15: Priority (0-7, 802.1p CoS)
> 
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Suggested-by: fengchengwen <fengchengwen@huawei.com>
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> ---
> v2:
> - Removed --enable-vlan-priority option and global variable as suggested
>   by Stephen Hemminger. The feature is now always enabled for TX paths
> - RX VLAN filter continues to enforce strict VLAN ID validation as
>   suggested by fengchengwen
> - Added documentation updates for testpmd_funcs.rst and release notes
> 
>  app/test-pmd/config.c                       | 13 ++++++++-----
>  doc/guides/rel_notes/release_26_07.rst      |  7 +++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 17 ++++++++++++++---
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 9d457ca88e..38758f9c05 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1241,8 +1241,11 @@ void print_valid_ports(void)
>  }
>  
>  static int
> -vlan_id_is_invalid(uint16_t vlan_id)
> +vlan_id_is_invalid(uint16_t vlan_id, bool is_tx)
>  {
> +	if (is_tx)
> +		return 0;
> +
>  	if (vlan_id < 4096)
>  		return 0;
>  	fprintf(stderr, "Invalid vlan_id %d (must be < 4096)\n", vlan_id);
> @@ -6876,7 +6879,7 @@ rx_vft_set(portid_t port_id, uint16_t vlan_id, int on)
>  
>  	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 1;
> -	if (vlan_id_is_invalid(vlan_id))
> +	if (vlan_id_is_invalid(vlan_id, false))
>  		return 1;
>  	diag = rte_eth_dev_vlan_filter(port_id, vlan_id, on);
>  	if (diag == 0)
> @@ -6923,7 +6926,7 @@ tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>  	struct rte_eth_dev_info dev_info;
>  	int ret;
>  
> -	if (vlan_id_is_invalid(vlan_id))
> +	if (vlan_id_is_invalid(vlan_id, true))
>  		return;

Why have the is_tx flag if it is always used as constant?
Just remove the whole vlan_id_is_invalid() branch test in the transmit path.
Maybe add a comment that any VLAN is allowed on transmit?

Or make a new function. Since VLAN of 0xffff is reserved. Though you might want
to allow it since testpmd is for testing even invalid packets.


      reply	other threads:[~2026-06-16 14:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-12  8:14 [PATCH] app/testpmd: add VLAN priority insert support Xingui Yang
2026-06-15  9:46 ` fengchengwen
2026-06-16 13:17   ` yangxingui
2026-06-15 19:12 ` Stephen Hemminger
2026-06-16 13:19   ` yangxingui
2026-06-16 13:10 ` [PATCH v2] " Xingui Yang
2026-06-16 14:23   ` Stephen Hemminger [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260616072328.1dcb8cf8@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=aman.deep.singh@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=fengchengwen@huawei.com \
    --cc=kangfenglong@huawei.com \
    --cc=lihuisong@huawei.com \
    --cc=liuyonglong@huawei.com \
    --cc=yangshuaisong@h-partners.com \
    --cc=yangxingui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox