DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] Bug with commit 64051bb1 (devargs: unify scratch buffer storage)
@ 2021-04-16 22:04 Harris, James R
  2021-04-17 14:59 ` Xueming(Steven) Li
  0 siblings, 1 reply; 2+ messages in thread
From: Harris, James R @ 2021-04-16 22:04 UTC (permalink / raw)
  To: dev, Xueming(Steven) Li


SPDK has identified a regression with commit 64051bb1 (devargs: unify scratch buffer storage).  The issue seems to be with this part of the patch:

@@ -276,15 +287,8 @@ rte_devargs_insert(struct rte_devargs **da)
                if (strcmp(listed_da->bus->name, (*da)->bus->name) == 0 &&
                                strcmp(listed_da->name, (*da)->name) == 0) {
                        /* device already in devargs list, must be updated */
-                       listed_da->type = (*da)->type;
-                       listed_da->policy = (*da)->policy;
-                       free(listed_da->args);
-                       listed_da->args = (*da)->args;
-                       listed_da->bus = (*da)->bus;
-                       listed_da->cls = (*da)->cls;
-                       listed_da->bus_str = (*da)->bus_str;
-                       listed_da->cls_str = (*da)->cls_str;
-                       listed_da->data = (*da)->data;
+                       rte_devargs_reset(listed_da);
+                       *listed_da = **da;
                        /* replace provided devargs with found one */
                        *da = listed_da;

Previously the data members were copied one-by-one, preserving the pointers in the listed_da’s TAILQ_ENTRY.  But after this patch, rte_devargs_reset() zeroes the entire rte_devargs structure, including the pointers in the TAILQ_ENTRY.  If we do a subsequent rte_devargs_remove() on this same entry, we segfault since the TAILQ_ENTRY’s pointers are invalid.  There could be similar segfaults with any subsequent rte_devargs_insert() calls that require iterating the global list of devargs entries.

rte_devargs_insert() could manually copy the TAILQ_ENTRY pointers to *da before calling rte_devargs_reset() – that at least fixes the SPDK regression.  But it’s not clear to me how many of the other rte_devargs_reset() callsites added by this patch also need to be changed in some way.



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-04-17 14:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16 22:04 [dpdk-dev] Bug with commit 64051bb1 (devargs: unify scratch buffer storage) Harris, James R
2021-04-17 14:59 ` Xueming(Steven) Li

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git