回复：[PATCH v1 2/4] net/nbl: add support for Tx and Rx VLAN offload

["Dimon" <dimon.zhao@nebula-matrix.com>]

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]

Hi Stephen,
Thank you for your review.
I'd like to explain why we need to implement a custom VLAN insertion function.
As shown in the code below, we first call `rte_vlan_insert` to insert the VLAN header.
Only if this fails will we call our own implementation, `nbl_res_txrx_vlan_insert_out_mbuf`, to insert the VLAN header.
This mainly corresponds to the scenario where `mbuf->refcnt > 1`.
In this case, the content of `mbuf` cannot be modified, so `rte_vlan_insert` will fail.
Our own implementation, `nbl_res_txrx_vlan_insert_out_mbuf`, puts the Ethernet header and VLAN header in a Tx desc,
and then puts the remaining header and data in the next Tx desc.
This achieves VLAN header insertion without modifying the `mbuf` content.
if (tx_pkt->ol_flags & RTE_MBUF_F_TX_VLAN) {
 if (likely(can_push)) {
 if (rte_vlan_insert(&tx_pkt)) {
 can_push = 0;
 u = (union nbl_tx_extend_head *)(&tx_region[desc_index]);
 }
 }
 if (unlikely(!can_push)) {
 addr_offset += sizeof(struct rte_ether_hdr);
nbl_res_txrx_vlan_insert_out_mbuf(tx_pkt, u, RTE_ETHER_TYPE_VLAN,
 tx_pkt->vlan_tci);
 }
}
------------------------------------------------------------------
发件人：Stephen Hemminger <stephen@networkplumber.org>
发送时间：2025年11月8日(周六) 00:10
收件人：Dimon<dimon.zhao@nebula-matrix.com>
抄　送：dev<dev@dpdk.org>; Alvin<alvin.wang@nebula-matrix.com>; Leon<leon.yu@nebula-matrix.com>; Sam<sam.chen@nebula-matrix.com>
主　题：Re: [PATCH v1 2/4] net/nbl: add support for Tx and Rx VLAN offload
On Thu, 6 Nov 2025 23:34:57 -0800
Dimon Zhao <dimon.zhao@nebula-matrix.com> wrote:
> +static inline void nbl_res_txrx_vlan_insert_out_mbuf(struct rte_mbuf *tx_pkt,
> + union nbl_tx_extend_head *u,
> + u16 vlan_proto, u16 vlan_tci)
> +{
> + struct rte_vlan_hdr *vlan_hdr;
> + struct rte_ether_hdr *ether_hdr;
> +
> + ether_hdr = (struct rte_ether_hdr *)((u8 *)u + sizeof(struct nbl_tx_ehdr_leonis));
> + rte_memcpy(ether_hdr, rte_pktmbuf_mtod(tx_pkt, u8 *), sizeof(struct rte_ether_hdr));
> +
> + vlan_hdr = (struct rte_vlan_hdr *)(ether_hdr + 1);
> + vlan_hdr->vlan_tci = rte_cpu_to_be_16(vlan_tci);
> + vlan_hdr->eth_proto = ether_hdr->ether_type;
> +
> + ether_hdr->ether_type = rte_cpu_to_be_16(vlan_proto);
> +}
> +
Please do not use rte_memcpy for small fixed size structures.
Prefer:
 rte_ether_addr_copy
 struct assignment
 memcpy
There already is a standard function for vlan insert, could this be used here?

[-- Attachment #2: Type: text/html, Size: 9047 bytes --]