Re: [dpdk-dev] [PATCH v7 2/2] app/testpmd: conserve offload flags of mbuf

[Andrew Rybchenko]

On 04/27/2018 03:01 AM, Yongseok Koh wrote:
This patch is to accommodate an experimental feature of mbuf - external
buffer attachment. If mbuf is attached to an external buffer, its ol_flags
will have EXT_ATTACHED_MBUF set. Without enabling/using the feature,
everything remains same.
If PMD delivers Rx packets with non-direct mbuf, ol_flags should not be
overwritten. For mlx5 PMD, if Multi-Packet RQ is enabled, Rx packets could
be carried with externally attached mbufs.

Signed-off-by: Yongseok Koh <ys...@mellanox.com>
---
  app/test-pmd/csumonly.c | 3 +++
  app/test-pmd/macfwd.c   | 3 +++
  app/test-pmd/macswap.c  | 3 +++
  3 files changed, 9 insertions(+)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 53b98412a..4a82bbc92 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -851,6 +851,9 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
                        m->l4_len = info.l4_len;
                        m->tso_segsz = info.tso_segsz;
                }
+               if (!RTE_MBUF_DIRECT(m))
+                       tx_ol_flags |= m->ol_flags &
+                               (IND_ATTACHED_MBUF | EXT_ATTACHED_MBUF);

1. I see no point to check !RTE_MBUF_DIRECT(m). Just inherit flags.
2. Consider to do it when tx_ol_flags are initialized above as 0, i.e.
   tx_ol_flags = m->ol_flags & (IND_ATTACHED_MBUF | EXT_ATTACHED_MBUF);

                m->ol_flags = tx_ol_flags;
  
  		/* Do split & copy for the packet. */
diff --git a/app/test-pmd/macfwd.c b/app/test-pmd/macfwd.c
index 2adce7019..ba0021194 100644
--- a/app/test-pmd/macfwd.c
+++ b/app/test-pmd/macfwd.c
@@ -96,6 +96,9 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
                                &eth_hdr->d_addr);
                ether_addr_copy(&ports[fs->tx_port].eth_addr,
                                &eth_hdr->s_addr);
+               if (!RTE_MBUF_DIRECT(mb))
+                       ol_flags |= mb->ol_flags &
+                               (IND_ATTACHED_MBUF | EXT_ATTACHED_MBUF);

1. Do not update ol_flags which is global and applied for all mbufs in 
the burst.
2. There is no point to check for  (!RTE_MBUF_DIRECT(mb). Just inherit 
these flags.

                mb->ol_flags = ol_flags;
                mb->l2_len = sizeof(struct ether_hdr);
                mb->l3_len = sizeof(struct ipv4_hdr);
diff --git a/app/test-pmd/macswap.c b/app/test-pmd/macswap.c
index e2cc4812c..b8d15f6ba 100644
--- a/app/test-pmd/macswap.c
+++ b/app/test-pmd/macswap.c
@@ -127,6 +127,9 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
                ether_addr_copy(&eth_hdr->s_addr, &eth_hdr->d_addr);
                ether_addr_copy(&addr, &eth_hdr->s_addr);
  
+		if (!RTE_MBUF_DIRECT(mb))
+                       ol_flags |= mb->ol_flags &
+                               (IND_ATTACHED_MBUF | EXT_ATTACHED_MBUF);

Same as above.k

                mb->ol_flags = ol_flags;
                mb->l2_len = sizeof(struct ether_hdr);
                mb->l3_len = sizeof(struct ipv4_hdr);

With above fixed:
Acked-by: Andrew Rybchenko <arybche...@solarflare.com>