Hi Helin,
> -----Original Message-----
> From: Zhang, Helin
> Sent: Monday, February 09, 2015 6:41 AM
> To: dev at dpdk.org
> Cc: Cao, Waterman; Liang, Cunming; Liu, Jijiang; Ananyev, Konstantin;
> Richardson, Bruce; Zhang, Helin
> Subject: [PATCH v2 14/15] examples/l3fwd: support of unified packet type
>
> To unify packet types among all PMDs, bit masks and relevant macros
> of packet type for ol_flags are replaced by unified packet type and
> relevant macros.
>
> Signed-off-by: Helin Zhang <helin.zhang at intel.com>
> ---
> examples/l3fwd/main.c | 64
> ++++++++++++++++++++++++++++-----------------------
> 1 file changed, 35 insertions(+), 29 deletions(-)
>
> v2 changes:
> * Used redefined packet types and enlarged packet_type field in mbuf.
>
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 6f7d7d4..302322e 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -958,7 +958,7 @@ l3fwd_simple_forward(struct rte_mbuf *m, uint8_t portid,
> struct lcore_conf *qcon
>
> eth_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
>
> - if (m->ol_flags & PKT_RX_IPV4_HDR) {
> + if (RTE_ETH_IS_IPV4_HDR(m->packet_type)) {
> /* Handle IPv4 headers.*/
> ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(m, unsigned
> char *) +
> sizeof(struct ether_hdr));
> @@ -993,7 +993,7 @@ l3fwd_simple_forward(struct rte_mbuf *m, uint8_t portid,
> struct lcore_conf *qcon
>
> send_single_packet(m, dst_port);
>
> - } else {
> + } else if (RTE_ETH_IS_IPV6_HDR(m->packet_type)) {
If you changed to from 'else' to ' else if' here, then I suppose you'll need to
add another 'else' after it:
to handle case, where input packets are neither IPV4 neither IPv6.
Otherwise you might start 'leaking' such mbufs.
> /* Handle IPv6 headers.*/
> struct ipv6_hdr *ipv6_hdr;
>
> @@ -1039,11 +1039,11 @@ l3fwd_simple_forward(struct rte_mbuf *m, uint8_t
> portid, struct lcore_conf *qcon
> * to BAD_PORT value.
> */
> static inline __attribute__((always_inline)) void
> -rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint16_t *dp, uint32_t flags)
> +rfc1812_process(struct ipv4_hdr *ipv4_hdr, uint16_t *dp, uint16_t ptype)
Shouldn't it be 'uint32_t ptype'?
> {
> uint8_t ihl;
>
> - if ((flags & PKT_RX_IPV4_HDR) != 0) {
> + if (RTE_ETH_IS_IPV4_HDR(ptype)) {
>
> ihl = ipv4_hdr->version_ihl - IPV4_MIN_VER_IHL;
>
> @@ -1074,11 +1074,11 @@ get_dst_port(const struct lcore_conf *qconf, struct
> rte_mbuf *pkt,
> struct ipv6_hdr *ipv6_hdr;
> struct ether_hdr *eth_hdr;
>
> - if (pkt->ol_flags & PKT_RX_IPV4_HDR) {
> + if (RTE_ETH_IS_IPV4_HDR(pkt->packet_type)) {
> if (rte_lpm_lookup(qconf->ipv4_lookup_struct, dst_ipv4,
> &next_hop) != 0)
> next_hop = portid;
> - } else if (pkt->ol_flags & PKT_RX_IPV6_HDR) {
> + } else if (RTE_ETH_IS_IPV6_HDR(pkt->packet_type)) {
> eth_hdr = rte_pktmbuf_mtod(pkt, struct ether_hdr *);
> ipv6_hdr = (struct ipv6_hdr *)(eth_hdr + 1);
> if (rte_lpm6_lookup(qconf->ipv6_lookup_struct,
> @@ -1112,17 +1112,19 @@ process_packet(struct lcore_conf *qconf, struct
> rte_mbuf *pkt,
> ve = val_eth[dp];
>
> dst_port[0] = dp;
> - rfc1812_process(ipv4_hdr, dst_port, pkt->ol_flags);
> + rfc1812_process(ipv4_hdr, dst_port, pkt->packet_type);
>
> te = _mm_blend_epi16(te, ve, MASK_ETH);
> _mm_store_si128((__m128i *)eth_hdr, te);
> }
>
> /*
> - * Read ol_flags and destination IPV4 addresses from 4 mbufs.
> + * Read packet_type and destination IPV4 addresses from 4 mbufs.
> */
> static inline void
> -processx4_step1(struct rte_mbuf *pkt[FWDSTEP], __m128i *dip, uint32_t *flag)
> +processx4_step1(struct rte_mbuf *pkt[FWDSTEP],
> + __m128i *dip,
> + uint32_t *ipv4_flag)
> {
> struct ipv4_hdr *ipv4_hdr;
> struct ether_hdr *eth_hdr;
> @@ -1131,22 +1133,20 @@ processx4_step1(struct rte_mbuf *pkt[FWDSTEP],
> __m128i *dip, uint32_t *flag)
> eth_hdr = rte_pktmbuf_mtod(pkt[0], struct ether_hdr *);
> ipv4_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
> x0 = ipv4_hdr->dst_addr;
> - flag[0] = pkt[0]->ol_flags & PKT_RX_IPV4_HDR;
>
> eth_hdr = rte_pktmbuf_mtod(pkt[1], struct ether_hdr *);
> ipv4_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
> x1 = ipv4_hdr->dst_addr;
> - flag[0] &= pkt[1]->ol_flags;
>
> eth_hdr = rte_pktmbuf_mtod(pkt[2], struct ether_hdr *);
> ipv4_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
> x2 = ipv4_hdr->dst_addr;
> - flag[0] &= pkt[2]->ol_flags;
>
> eth_hdr = rte_pktmbuf_mtod(pkt[3], struct ether_hdr *);
> ipv4_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
> x3 = ipv4_hdr->dst_addr;
> - flag[0] &= pkt[3]->ol_flags;
> + *ipv4_flag = pkt[0]->packet_type & pkt[1]->packet_type &
> + pkt[2]->packet_type & pkt[3]->packet_type & RTE_PTYPE_L3_IPV4;
Why not as it was before:
flag[0] = pkt[0]->packet-type & ...
...
flag[0] &= pkt[1]->packet_type;
...
Why do you need to unite them?
>
> dip[0] = _mm_set_epi32(x3, x2, x1, x0);
> }
> @@ -1156,8 +1156,12 @@ processx4_step1(struct rte_mbuf *pkt[FWDSTEP], __m128i
> *dip, uint32_t *flag)
> * If lookup fails, use incoming port (portid) as destination port.
> */
> static inline void
> -processx4_step2(const struct lcore_conf *qconf, __m128i dip, uint32_t flag,
> - uint8_t portid, struct rte_mbuf *pkt[FWDSTEP], uint16_t dprt[FWDSTEP])
> +processx4_step2(const struct lcore_conf *qconf,
> + __m128i dip,
> + uint32_t ipv4_flag,
> + uint8_t portid,
> + struct rte_mbuf *pkt[FWDSTEP],
> + uint16_t dprt[FWDSTEP])
> {
> rte_xmm_t dst;
> const __m128i bswap_mask = _mm_set_epi8(12, 13, 14, 15, 8, 9, 10, 11,
> @@ -1167,7 +1171,7 @@ processx4_step2(const struct lcore_conf *qconf, __m128i
> dip, uint32_t flag,
> dip = _mm_shuffle_epi8(dip, bswap_mask);
>
> /* if all 4 packets are IPV4. */
> - if (likely(flag != 0)) {
> + if (likely(ipv4_flag)) {
> rte_lpm_lookupx4(qconf->ipv4_lookup_struct, dip, dprt, portid);
> } else {
> dst.x = dip;
> @@ -1218,13 +1222,13 @@ processx4_step3(struct rte_mbuf *pkt[FWDSTEP],
> uint16_t dst_port[FWDSTEP])
> _mm_store_si128(p[3], te[3]);
>
> rfc1812_process((struct ipv4_hdr *)((struct ether_hdr *)p[0] + 1),
> - &dst_port[0], pkt[0]->ol_flags);
> + &dst_port[0], pkt[0]->packet_type);
> rfc1812_process((struct ipv4_hdr *)((struct ether_hdr *)p[1] + 1),
> - &dst_port[1], pkt[1]->ol_flags);
> + &dst_port[1], pkt[1]->packet_type);
> rfc1812_process((struct ipv4_hdr *)((struct ether_hdr *)p[2] + 1),
> - &dst_port[2], pkt[2]->ol_flags);
> + &dst_port[2], pkt[2]->packet_type);
> rfc1812_process((struct ipv4_hdr *)((struct ether_hdr *)p[3] + 1),
> - &dst_port[3], pkt[3]->ol_flags);
> + &dst_port[3], pkt[3]->packet_type);
> }
>
> /*
> @@ -1411,7 +1415,7 @@ main_loop(__attribute__((unused)) void *dummy)
> uint16_t *lp;
> uint16_t dst_port[MAX_PKT_BURST];
> __m128i dip[MAX_PKT_BURST / FWDSTEP];
> - uint32_t flag[MAX_PKT_BURST / FWDSTEP];
> + uint32_t ipv4_flag[MAX_PKT_BURST / FWDSTEP];
> uint16_t pnum[MAX_PKT_BURST + 1];
> #endif
>
> @@ -1481,14 +1485,16 @@ main_loop(__attribute__((unused)) void *dummy)
> */
> int32_t n = RTE_ALIGN_FLOOR(nb_rx, 4);
> for (j = 0; j < n ; j+=4) {
> - uint32_t ol_flag =
> pkts_burst[j]->ol_flags
> - &
> pkts_burst[j+1]->ol_flags
> - &
> pkts_burst[j+2]->ol_flags
> - &
> pkts_burst[j+3]->ol_flags;
> - if (ol_flag & PKT_RX_IPV4_HDR ) {
> + uint32_t pkt_type =
> + pkts_burst[j]->packet_type &
> + pkts_burst[j+1]->packet_type &
> + pkts_burst[j+2]->packet_type &
> + pkts_burst[j+3]->packet_type;
> + if (pkt_type & RTE_PTYPE_L3_IPV4) {
>
> simple_ipv4_fwd_4pkts(&pkts_burst[j],
> portid,
> qconf);
> - } else if (ol_flag & PKT_RX_IPV6_HDR) {
> + } else if (pkt_type &
> + RTE_PTYPE_L3_IPV6) {
>
> simple_ipv6_fwd_4pkts(&pkts_burst[j],
> portid,
> qconf);
> } else {
> @@ -1513,13 +1519,13 @@ main_loop(__attribute__((unused)) void *dummy)
> for (j = 0; j != k; j += FWDSTEP) {
> processx4_step1(&pkts_burst[j],
> &dip[j / FWDSTEP],
> - &flag[j / FWDSTEP]);
> + &ipv4_flag[j / FWDSTEP]);
> }
>
> k = RTE_ALIGN_FLOOR(nb_rx, FWDSTEP);
> for (j = 0; j != k; j += FWDSTEP) {
> processx4_step2(qconf, dip[j / FWDSTEP],
> - flag[j / FWDSTEP], portid,
> + ipv4_flag[j / FWDSTEP], portid,
> &pkts_burst[j], &dst_port[j]);
> }
>
> --
> 1.9.3
