diff mbox series

[ovs-dev] 答复: [PATCH] netdev-dpdk: fix incorrect shinfo initialization

Message ID 078e4ccbbeb7466592a19c4f7c38a73b@inspur.com
State Not Applicable
Headers show
Series [ovs-dev] 答复: [PATCH] netdev-dpdk: fix incorrect shinfo initialization | expand

Commit Message

Yi Yang (杨燚)-云服务集团 Oct. 27, 2020, 12:30 a.m. UTC 
Hi, folks

Anybody can help merge this fix patch, please add comments if you think it has any issue, thanks a lot.

-----邮件原件-----
发件人: yang_y_yi@163.com [mailto:yang_y_yi@163.com] 
发送时间: 2020年10月14日 15:23
收件人: ovs-dev@openvswitch.org
抄送: blp@ovn.org; i.maximets@ovn.org; ian.stokes@intel.com; u9012063@gmail.com; olivier.matz@6wind.com; fbl@sysclose.org; Yi Yang (杨燚)-云服务集团 <yangyi01@inspur.com>; yang_y_yi@163.com
主题: [PATCH] netdev-dpdk: fix incorrect shinfo initialization

From: Yi Yang <yangyi01@inspur.com>

shinfo is used to store reference counter and free callback of an external buffer, but it is stored in mbuf if the mbuf has tailroom for it.

This is wrong because the mbuf (and its data) can be freed before the external buffer, for example:

  pkt2 = rte_pktmbuf_alloc(mp);
  rte_pktmbuf_attach(pkt2, pkt);
  rte_pktmbuf_free(pkt);

After this, pkt is freed, but it still contains shinfo, which is referenced by pkt2.

Fix this by always storing shinfo at the tail of external buffer.

Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
Co-authored-by: Olivier Matz <olivier.matz@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Signed-off-by: Yi Yang <yangyi01@inspur.com>
---
 lib/netdev-dpdk.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

     }
 
     /* Initialize shinfo. */
-    if (shinfo) {
-        shinfo->free_cb = netdev_dpdk_extbuf_free;
-        shinfo->fcb_opaque = buf;
-        rte_mbuf_ext_refcnt_set(shinfo, 1);
-    } else {
-        shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len,
-                                                    netdev_dpdk_extbuf_free,
-                                                    buf);
-        if (OVS_UNLIKELY(shinfo == NULL)) {
-            rte_free(buf);
-            VLOG_ERR("Failed to initialize shared info for mbuf while "
-                     "attempting to attach an external buffer.");
-            return NULL;
-        }
+    shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len,
+                                                netdev_dpdk_extbuf_free,
+                                                buf);
+    if (OVS_UNLIKELY(shinfo == NULL)) {
+        rte_free(buf);
+        VLOG_ERR("Failed to initialize shared info for mbuf while "
+                 "attempting to attach an external buffer.");
+        return NULL;
     }
 
     rte_pktmbuf_attach_extbuf(pkt, buf, rte_malloc_virt2iova(buf), buf_len,
--
1.8.3.1
diff mbox series

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 0b830be..c7f9326 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2654,12 +2654,8 @@  dpdk_pktmbuf_attach_extbuf(struct rte_mbuf *pkt, uint32_t data_len)
     uint16_t buf_len;
     void *buf;
 
-    if (rte_pktmbuf_tailroom(pkt) >= sizeof *shinfo) {
-        shinfo = rte_pktmbuf_mtod(pkt, struct rte_mbuf_ext_shared_info *);
-    } else {
-        total_len += sizeof *shinfo + sizeof(uintptr_t);
-        total_len = RTE_ALIGN_CEIL(total_len, sizeof(uintptr_t));
-    }
+    total_len += sizeof *shinfo + sizeof(uintptr_t);
+    total_len = RTE_ALIGN_CEIL(total_len, sizeof(uintptr_t));
 
     if (OVS_UNLIKELY(total_len > UINT16_MAX)) {
         VLOG_ERR("Can't copy packet: too big %u", total_len); @@ -2674,20 +2670,14 @@ dpdk_pktmbuf_attach_extbuf(struct rte_mbuf *pkt, uint32_t data_len)