The behavior of PKT_RX_VLAN_PKT was not very well defined, resulting in PMDs not advertising the same flags in similar conditions.
Following discussion in [1], introduce 2 new flags PKT_RX_VLAN_STRIPPED and PKT_RX_QINQ_STRIPPED that are better defined: PKT_RX_VLAN_STRIPPED: a vlan has been stripped by the hardware and its tci is saved in mbuf->vlan_tci. This can only happen if vlan stripping is enabled in the RX configuration of the PMD. For now, the old flag PKT_RX_VLAN_PKT is kept but marked as deprecated. It should be removed from applications and PMDs in a future revision. This patch also updates the drivers. For PKT_RX_VLAN_PKT: - e1000, enic, i40e, mlx5, nfp, vmxnet3: done, PKT_RX_VLAN_PKT already had the same meaning than PKT_RX_VLAN_STRIPPED, minor update is required. - fm10k: done, PKT_RX_VLAN_PKT already had the same meaning than PKT_RX_VLAN_STRIPPED, and vlan stripping is always enabled on fm10k. - ixgbe: modification done for standard mode (vector does not support vlan stripping) - the other drivers do not support vlan stripping. For PKT_RX_QINQ_PKT, it was only supported on i40e, and the meaning was already correct, so we can reuse the same value for PKT_RX_QINQ_STRIPPED. [1] http://dpdk.org/ml/archives/dev/2016-April/037837.html, Signed-off-by: Olivier Matz <olivier.matz at 6wind.com> --- RFC -> v1: - fix checkpatch and check-git-log.sh issues - add a deprecation notice for the old vlan flags - rebase on head app/test-pmd/rxonly.c | 4 +-- doc/guides/rel_notes/deprecation.rst | 5 ++++ drivers/net/e1000/em_rxtx.c | 3 ++- drivers/net/e1000/igb_rxtx.c | 3 ++- drivers/net/enic/enic_rx.c | 2 +- drivers/net/i40e/i40e_rxtx.c | 2 +- drivers/net/ixgbe/ixgbe_ethdev.c | 7 +++++ drivers/net/ixgbe/ixgbe_rxtx.c | 21 +++++++++++---- drivers/net/ixgbe/ixgbe_rxtx.h | 1 + drivers/net/mlx5/mlx5_rxtx.c | 6 +++-- drivers/net/nfp/nfp_net.c | 2 +- drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +- lib/librte_mbuf/rte_mbuf.c | 2 ++ lib/librte_mbuf/rte_mbuf.h | 50 ++++++++++++++++++++++++++++++++---- 14 files changed, 90 insertions(+), 20 deletions(-) diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c index 14555ab..c69b344 100644 --- a/app/test-pmd/rxonly.c +++ b/app/test-pmd/rxonly.c @@ -156,9 +156,9 @@ pkt_burst_receive(struct fwd_stream *fs) printf("hash=0x%x ID=0x%x ", mb->hash.fdir.hash, mb->hash.fdir.id); } - if (ol_flags & PKT_RX_VLAN_PKT) + if (ol_flags & PKT_RX_VLAN_STRIPPED) printf(" - VLAN tci=0x%x", mb->vlan_tci); - if (ol_flags & PKT_RX_QINQ_PKT) + if (ol_flags & PKT_RX_QINQ_STRIPPED) printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x", mb->vlan_tci, mb->vlan_tci_outer); if (mb->packet_type) { diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index ad05eba..2233a90 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -57,3 +57,8 @@ Deprecation Notices a handle, like the way kernel exposes an fd to user for locating a specific file, and to keep all major structures internally, so that we are likely to be free from ABI violations in future. + +* The mbuf flags PKT_RX_VLAN_PKT and PKT_RX_QINQ_PKT are deprecated and + are respectively replaced by PKT_RX_VLAN_STRIPPED and + PKT_RX_QINQ_STRIPPED, that are better described. The old flags and + their behavior will be kept in 16.07 and will be removed in 16.11. diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c index 3d36f21..6d8750a 100644 --- a/drivers/net/e1000/em_rxtx.c +++ b/drivers/net/e1000/em_rxtx.c @@ -629,7 +629,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status) uint64_t pkt_flags; /* Check if VLAN present */ - pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ? PKT_RX_VLAN_PKT : 0); + pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ? + PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0); return pkt_flags; } diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c index 18aeead..9d80a0b 100644 --- a/drivers/net/e1000/igb_rxtx.c +++ b/drivers/net/e1000/igb_rxtx.c @@ -729,7 +729,8 @@ rx_desc_status_to_pkt_flags(uint32_t rx_status) uint64_t pkt_flags; /* Check if VLAN present */ - pkt_flags = (rx_status & E1000_RXD_STAT_VP) ? PKT_RX_VLAN_PKT : 0; + pkt_flags = ((rx_status & E1000_RXD_STAT_VP) ? + PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED : 0); #if defined(RTE_LIBRTE_IEEE1588) if (rx_status & E1000_RXD_STAT_TMST) diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c index f92f6bc..6459e97 100644 --- a/drivers/net/enic/enic_rx.c +++ b/drivers/net/enic/enic_rx.c @@ -197,7 +197,7 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf) /* VLAN stripping */ if (bwflags & CQ_ENET_RQ_DESC_FLAGS_VLAN_STRIPPED) { - pkt_flags |= PKT_RX_VLAN_PKT; + pkt_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED; mbuf->vlan_tci = enic_cq_rx_desc_vlan(cqrd); } else { mbuf->vlan_tci = 0; diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index c833aa3..aa161a9 100644 --- a/drivers/net/i40e/i40e_rxtx.c +++ b/drivers/net/i40e/i40e_rxtx.c @@ -99,7 +99,7 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union i40e_rx_desc *rxdp) #ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) & (1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) { - mb->ol_flags |= PKT_RX_QINQ_PKT; + mb->ol_flags |= PKT_RX_QINQ_STRIPPED; mb->vlan_tci_outer = mb->vlan_tci; mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_2); PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u", diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index a2b170b..e7717e3 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -1636,6 +1636,7 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on) { struct ixgbe_hwstrip *hwstrip = IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(dev->data->dev_private); + struct ixgbe_rx_queue *rxq; if (queue >= IXGBE_MAX_RX_QUEUE_NUM) return; @@ -1644,6 +1645,12 @@ ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev, uint16_t queue, bool on) IXGBE_SET_HWSTRIP(hwstrip, queue); else IXGBE_CLEAR_HWSTRIP(hwstrip, queue); + + if (queue >= dev->data->nb_rx_queues) + return; + + rxq = dev->data->rx_queues[queue]; + rxq->vlan_strip = on; } static void diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c index 9c6eaf2..3d740df 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx.c +++ b/drivers/net/ixgbe/ixgbe_rxtx.c @@ -1221,16 +1221,23 @@ ixgbe_rxd_pkt_info_to_pkt_flags(uint16_t pkt_info) } static inline uint64_t -rx_desc_status_to_pkt_flags(uint32_t rx_status) +rx_desc_status_to_pkt_flags(uint32_t rx_status, uint8_t vlan_strip) { uint64_t pkt_flags; + uint64_t vlan_flags; + + /* if vlan is stripped, set the proper flag */ + if (vlan_strip) + vlan_flags = PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED; + else + vlan_flags = PKT_RX_VLAN_PKT; /* * Check if VLAN present only. * Do not check whether L3/L4 rx checksum done by NIC or not, * That can be found from rte_eth_rxmode.hw_ip_checksum flag */ - pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ? PKT_RX_VLAN_PKT : 0; + pkt_flags = (rx_status & IXGBE_RXD_STAT_VP) ? vlan_flags : 0; #ifdef RTE_LIBRTE_IEEE1588 if (rx_status & IXGBE_RXD_STAT_TMST) @@ -1287,6 +1294,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq) uint32_t pkt_info[LOOK_AHEAD]; int i, j, nb_rx = 0; uint32_t status; + uint8_t vlan_strip = rxq->vlan_strip; /* get references to current descriptor and S/W ring entry */ rxdp = &rxq->rx_ring[rxq->rx_tail]; @@ -1328,7 +1336,8 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq) mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan); /* convert descriptor fields to rte mbuf flags */ - pkt_flags = rx_desc_status_to_pkt_flags(s[j]); + pkt_flags = rx_desc_status_to_pkt_flags(s[j], + vlan_strip); pkt_flags |= rx_desc_error_to_pkt_flags(s[j]); pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags ((uint16_t)pkt_info[j]); @@ -1544,6 +1553,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_rx; uint16_t nb_hold; uint64_t pkt_flags; + uint8_t vlan_strip; nb_rx = 0; nb_hold = 0; @@ -1551,6 +1561,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, rx_id = rxq->rx_tail; rx_ring = rxq->rx_ring; sw_ring = rxq->sw_ring; + vlan_strip = rxq->vlan_strip; while (nb_rx < nb_pkts) { /* * The order of operations here is important as the DD status @@ -1660,7 +1671,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, /* Only valid if PKT_RX_VLAN_PKT set in pkt_flags */ rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan); - pkt_flags = rx_desc_status_to_pkt_flags(staterr); + pkt_flags = rx_desc_status_to_pkt_flags(staterr, vlan_strip); pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr); pkt_flags = pkt_flags | ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info); @@ -1753,7 +1764,7 @@ ixgbe_fill_cluster_head_buf( */ head->vlan_tci = rte_le_to_cpu_16(desc->wb.upper.vlan); pkt_info = rte_le_to_cpu_32(desc->wb.lower.lo_dword.data); - pkt_flags = rx_desc_status_to_pkt_flags(staterr); + pkt_flags = rx_desc_status_to_pkt_flags(staterr, rxq->vlan_strip); pkt_flags |= rx_desc_error_to_pkt_flags(staterr); pkt_flags |= ixgbe_rxd_pkt_info_to_pkt_flags((uint16_t)pkt_info); head->ol_flags = pkt_flags; diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h index 3691a19..9ca0e8b 100644 --- a/drivers/net/ixgbe/ixgbe_rxtx.h +++ b/drivers/net/ixgbe/ixgbe_rxtx.h @@ -146,6 +146,7 @@ struct ixgbe_rx_queue { uint8_t crc_len; /**< 0 if CRC stripped, 4 otherwise. */ uint8_t drop_en; /**< If not 0, set SRRCTL.Drop_En. */ uint8_t rx_deferred_start; /**< not in global dev start. */ + uint8_t vlan_strip; /**< 1 if vlan stripping enabled. */ /** need to alloc dummy mbuf, for wraparound when scanning hw ring */ struct rte_mbuf fake_mbuf; /** hold packets to return to application */ diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index 13c8d71..ac96fc9 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1051,7 +1051,8 @@ mlx5_rx_burst_sp(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) pkt_buf->ol_flags = rxq_cq_to_ol_flags(rxq, flags); #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) { - pkt_buf->ol_flags |= PKT_RX_VLAN_PKT; + pkt_buf->ol_flags |= PKT_RX_VLAN_PKT | + PKT_RX_VLAN_STRIPPED; pkt_buf->vlan_tci = vlan_tci; } #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */ @@ -1207,7 +1208,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) seg->ol_flags = rxq_cq_to_ol_flags(rxq, flags); #ifdef HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS if (flags & IBV_EXP_CQ_RX_CVLAN_STRIPPED_V1) { - seg->ol_flags |= PKT_RX_VLAN_PKT; + seg->ol_flags |= PKT_RX_VLAN_PKT | + PKT_RX_VLAN_STRIPPED; seg->vlan_tci = vlan_tci; } #endif /* HAVE_EXP_DEVICE_ATTR_VLAN_OFFLOADS */ diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index ea5a2a3..5c9f350 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -1800,7 +1800,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) if ((rxds->rxd.flags & PCIE_DESC_RX_VLAN) && (hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) { mb->vlan_tci = rte_cpu_to_le_32(rxds->rxd.vlan); - mb->ol_flags |= PKT_RX_VLAN_PKT; + mb->ol_flags |= PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED; } /* Adding the mbuff to the mbuff array passed by the app */ diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c index 9fe8752..ccafc0c 100644 --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c @@ -579,7 +579,7 @@ vmxnet3_rx_offload(const Vmxnet3_RxCompDesc *rcd, struct rte_mbuf *rxm) { /* Check for hardware stripped VLAN tag */ if (rcd->ts) { - rxm->ol_flags |= PKT_RX_VLAN_PKT; + rxm->ol_flags |= (PKT_RX_VLAN_PKT | PKT_RX_VLAN_STRIPPED); rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci); } diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index eec1456..2ece742 100644 --- a/lib/librte_mbuf/rte_mbuf.c +++ b/lib/librte_mbuf/rte_mbuf.c @@ -258,8 +258,10 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask) /* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; */ /* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */ /* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */ + case PKT_RX_VLAN_STRIPPED: return "PKT_RX_VLAN_STRIPPED"; case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP"; case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST"; + case PKT_RX_QINQ_STRIPPED: return "PKT_RX_QINQ_STRIPPED"; default: return NULL; } } diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 48911a6..5b8a11a 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -79,7 +79,16 @@ extern "C" { * Keep these flags synchronized with rte_get_rx_ol_flag_name() and * rte_get_tx_ol_flag_name(). */ -#define PKT_RX_VLAN_PKT (1ULL << 0) /**< RX packet is a 802.1q VLAN packet. */ + +/** + * Deprecated. + * RX packet is a 802.1q VLAN packet. This flag was set by PMDs when + * the packet is recognized as a VLAN, but the behavior between PMDs + * was not the same. This flag is kept for some time to avoid breaking + * applications and should be replaced by PKT_RX_VLAN_STRIPPED. + */ +#define PKT_RX_VLAN_PKT (1ULL << 0) + #define PKT_RX_RSS_HASH (1ULL << 1) /**< RX packet with RSS hash result. */ #define PKT_RX_FDIR (1ULL << 2) /**< RX packet with FDIR match indicate. */ #define PKT_RX_L4_CKSUM_BAD (1ULL << 3) /**< L4 cksum of RX pkt. is not OK. */ @@ -89,11 +98,37 @@ extern "C" { #define PKT_RX_HBUF_OVERFLOW (0ULL << 0) /**< Header buffer overflow. */ #define PKT_RX_RECIP_ERR (0ULL << 0) /**< Hardware processing error. */ #define PKT_RX_MAC_ERR (0ULL << 0) /**< MAC error. */ + +/** + * A vlan has been stripped by the hardware and its tci is saved in + * mbuf->vlan_tci. This can only happen if vlan stripping is enabled + * in the RX configuration of the PMD. + */ +#define PKT_RX_VLAN_STRIPPED (1ULL << 6) + +/* hole, some bits can be reused here */ + #define PKT_RX_IEEE1588_PTP (1ULL << 9) /**< RX IEEE1588 L2 Ethernet PT Packet. */ #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/ #define PKT_RX_FDIR_ID (1ULL << 13) /**< FD id reported if FDIR match. */ #define PKT_RX_FDIR_FLX (1ULL << 14) /**< Flexible bytes reported if FDIR match. */ -#define PKT_RX_QINQ_PKT (1ULL << 15) /**< RX packet with double VLAN stripped. */ + +/** + * The 2 vlans have been stripped by the hardware and their tci are + * saved in mbuf->vlan_tci (inner) and mbuf->vlan_tci_outer (outer). + * This can only happen if vlan stripping is enabled in the RX + * configuration of the PMD. If this flag is set, PKT_RX_VLAN_STRIPPED + * must also be set. + */ +#define PKT_RX_QINQ_STRIPPED (1ULL << 15) + +/** + * Deprecated. + * RX packet with double VLAN stripped. + * This flag is replaced by PKT_RX_QINQ_STRIPPED. + */ +#define PKT_RX_QINQ_PKT PKT_RX_QINQ_STRIPPED + /* add new RX flags here */ /* add new TX flags here */ @@ -761,7 +796,10 @@ struct rte_mbuf { /* * The packet type, which is the combination of outer/inner L2, L3, L4 - * and tunnel types. + * and tunnel types. The packet_type is about data really present in the + * mbuf. Example: if vlan stripping is enabled, a received vlan packet + * would have RTE_PTYPE_L2_ETHER and not RTE_PTYPE_L2_VLAN because the + * vlan is stripped from the data. */ union { uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */ @@ -778,7 +816,8 @@ struct rte_mbuf { uint32_t pkt_len; /**< Total pkt len: sum of all segments. */ uint16_t data_len; /**< Amount of data in segment buffer. */ - uint16_t vlan_tci; /**< VLAN Tag Control Identifier (CPU order) */ + /** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */ + uint16_t vlan_tci; union { uint32_t rss; /**< RSS hash result if RSS enabled */ @@ -804,7 +843,8 @@ struct rte_mbuf { uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */ - uint16_t vlan_tci_outer; /**< Outer VLAN Tag Control Identifier (CPU order) */ + /** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ_STRIPPED is set. */ + uint16_t vlan_tci_outer; /* second cache line - fields only used in slow path or on TX */ MARKER cacheline1 __rte_cache_min_aligned; -- 2.8.0.rc3
