DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Huisong Li <lihuisong@huawei.com>
Cc: dev@dpdk.org, ferruh.yigit@xilinx.com,
	andrew.rybchenko@oktetlabs.ru, huangdaode@huawei.com,
	david.marchand@redhat.com
Subject: Re: [PATCH V2 1/6] bus/pci: fix a segfault when call callback
Date: Mon, 10 Oct 2022 21:49:46 +0200	[thread overview]
Message-ID: <11178860.MucGe3eQFb@thomas> (raw)
In-Reply-To: <20220915124522.5407-2-lihuisong@huawei.com>

15/09/2022 14:45, Huisong Li:
> After the driver probe is executed, the callback in application will
> be called. The callback in application may call some APIs which access the
> rte_pci_driver::driver by the device::driver pointer to get driver
> information. If the rte_pci_device::device::driver pointer isn't pointed to
> rte_pci_driver::driver in rte_pci_probe_one_driver, a segfault will occur.
> For example, when ethdev driver probe completes, the callback in
> application call rte_eth_dev_info_get which use dev->device->driver->name.
> So rte_pci_device::device::driver should point to rte_pci_driver::driver
> before executing the driver probe.
> 
> Fixes: c752998b5e2e ("pci: introduce library and driver")
> Cc: stable@dpdk.org

To be more precise, it is reverting
391797f04208 ("drivers/bus: move driver assignment to end of probing")

dev->device.driver is used by rte_dev_is_probed():

int
rte_dev_is_probed(const struct rte_device *dev)
{
    /* The field driver should be set only when the probe is successful. */
    return dev->driver != NULL;
}

And the field comment is clear in rte_device:

const struct rte_driver *driver; /**< Driver assigned after probing */

That's why I am not enthusiastic about setting this pointer before probing.

I understand it is more convenient to use this pointer
in a probing callback.
We need to check it is not breaking rte_dev_is_probed() usage.
It may be OK if there is no parallel probing,
and rte_dev_is_probed() is not called inside probing.

At the very minimum, we need to update some comments in the code,
to mention that the pointer is set before probing,
and reset if probing failed.



  reply	other threads:[~2022-10-10 19:49 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20220825024425.10534-1-lihuisong@huawei.com>
2022-09-15 12:45 ` [PATCH V2 0/6] [PATCH 0/6] app/testpmd: support attach and detach port for MP Huisong Li
2022-09-15 12:45   ` [PATCH V2 1/6] bus/pci: fix a segfault when call callback Huisong Li
2022-10-10 19:49     ` Thomas Monjalon [this message]
2022-10-25  3:25       ` lihuisong (C)
2022-09-15 12:45   ` [PATCH V2 2/6] bus/vdev: " Huisong Li
2022-09-15 12:45   ` [PATCH V2 3/6] ethdev: fix push new event Huisong Li
2022-09-27 10:49     ` Thomas Monjalon
2022-10-08  4:09       ` lihuisong (C)
2022-10-25  3:26         ` lihuisong (C)
2022-09-15 12:45   ` [PATCH V2 4/6] app/testpmd: check the validity of the port Huisong Li
2022-09-22  5:07     ` Singh, Aman Deep
2022-09-15 12:45   ` [PATCH V2 5/6] app/testpmd: support attach and detach port for MP Huisong Li
2022-09-15 12:45   ` [PATCH V2 6/6] app/testpmd: stop packet forwarding in new and destroy event Huisong Li
2022-12-06  6:45 ` [PATCH V3 0/5] app/testpmd: support mulitple process attach and detach port Huisong Li
2022-12-06  6:45   ` [PATCH V3 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2022-12-06  6:45   ` [PATCH V3 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2022-12-06  6:45   ` [PATCH V3 3/5] app/testpmd: check the validity of the port Huisong Li
2022-12-06  6:45   ` [PATCH V3 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2022-12-06  6:45   ` [PATCH V3 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2022-12-06  9:26 ` [PATCH V4 0/5] app/testpmd: support mulitple process attach and detach port Huisong Li
2022-12-06  9:26   ` [PATCH V4 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2023-01-11 12:51     ` Ferruh Yigit
2023-01-12  2:44       ` lihuisong (C)
2023-02-15 16:09         ` Ferruh Yigit
2023-02-28  2:21           ` lihuisong (C)
2023-06-06 16:12             ` Ferruh Yigit
2023-06-07 10:11               ` lihuisong (C)
2023-06-15  2:21                 ` lihuisong (C)
2022-12-06  9:26   ` [PATCH V4 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2023-01-11 12:51     ` Ferruh Yigit
2023-01-12  4:12       ` lihuisong (C)
2022-12-06  9:26   ` [PATCH V4 3/5] app/testpmd: check the validity of the port Huisong Li
2022-12-06  9:26   ` [PATCH V4 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2023-01-11 12:51     ` Ferruh Yigit
2023-01-12  4:14       ` lihuisong (C)
2022-12-06  9:26   ` [PATCH V4 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2023-01-11 12:52     ` Ferruh Yigit
2023-01-12  4:16       ` lihuisong (C)
2023-01-09 12:38   ` [PATCH V4 0/5] app/testpmd: support mulitple process attach and detach port lihuisong (C)
2023-01-10 16:51   ` Ferruh Yigit
2023-01-11  0:53     ` lihuisong (C)
2023-01-11 10:27       ` Ferruh Yigit
2023-01-11 10:46         ` Ferruh Yigit
2023-01-12  2:26           ` lihuisong (C)
2023-01-18 14:12           ` Thomas Monjalon
2023-01-19 10:31             ` lihuisong (C)
2023-01-19 14:35               ` Thomas Monjalon
2023-01-28  1:39                 ` lihuisong (C)
2023-01-31  3:33 ` [PATCH V5 0/5] app/testpmd: support multiple " Huisong Li
2023-01-31  3:33   ` [PATCH V5 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2023-01-31  3:33   ` [PATCH V5 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2023-05-22 11:04     ` fengchengwen
2023-05-27  1:58       ` lihuisong (C)
2023-01-31  3:33   ` [PATCH V5 3/5] app/testpmd: check the validity of the port Huisong Li
2023-01-31  3:33   ` [PATCH V5 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2023-01-31  3:33   ` [PATCH V5 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2023-05-16 11:27   ` [PATCH V5 0/5] app/testpmd: support multiple process attach and detach port lihuisong (C)
2023-05-23  0:46   ` fengchengwen
2023-05-27  2:11 ` [PATCH V6 " Huisong Li
2023-05-27  2:11   ` [PATCH V6 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2023-05-27  2:11   ` [PATCH V6 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2023-05-27  2:11   ` [PATCH V6 3/5] app/testpmd: check the validity of the port Huisong Li
2023-05-27  2:11   ` [PATCH V6 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2023-05-27  2:11   ` [PATCH V6 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2023-06-06 16:26   ` [PATCH V6 0/5] app/testpmd: support multiple process attach and detach port Ferruh Yigit
2023-06-07 10:14     ` lihuisong (C)
2023-07-14  7:21   ` lihuisong (C)
2023-08-02  3:15 ` [PATCH RESEND v6 " Huisong Li
2023-08-02  3:15   ` [PATCH RESEND v6 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2023-08-02  3:15   ` [PATCH RESEND v6 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2023-08-02  3:15   ` [PATCH RESEND v6 3/5] app/testpmd: check the validity of the port Huisong Li
2023-08-02  3:15   ` [PATCH RESEND v6 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2023-08-02  3:16   ` [PATCH RESEND v6 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2023-10-09 10:34   ` [PATCH RESEND v6 0/5] app/testpmd: support multiple process attach and detach port lihuisong (C)
2023-10-30 12:17     ` lihuisong (C)
2023-12-08  2:25       ` lihuisong (C)
2024-01-30  6:36 ` [PATCH v7 " Huisong Li
2024-01-30  6:36   ` [PATCH v7 1/5] drivers/bus: restore driver assignment at front of probing Huisong Li
2024-01-30  6:36   ` [PATCH v7 2/5] ethdev: fix skip valid port in probing callback Huisong Li
2024-01-30  6:36   ` [PATCH v7 3/5] app/testpmd: check the validity of the port Huisong Li
2024-01-30  6:36   ` [PATCH v7 4/5] app/testpmd: add attach and detach port for multiple process Huisong Li
2024-01-30  6:36   ` [PATCH v7 5/5] app/testpmd: stop forwarding in new or destroy event Huisong Li
2024-03-08 10:38   ` [PATCH v7 0/5] app/testpmd: support multiple process attach and detach port lihuisong (C)
2024-04-23 11:17   ` lihuisong (C)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=11178860.MucGe3eQFb@thomas \
    --to=thomas@monjalon.net \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@xilinx.com \
    --cc=huangdaode@huawei.com \
    --cc=lihuisong@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).