[PATCH] bus/pci: don't open uio device in secondary process
The uio_pci_generic driver clears the bus master bit when the device
file is closed. So, when the secondary process terminates after probing
a device, that device becomes unusable in the primary process.
To avoid that, the device file is now opened only in the primary
process. The commit that introduced this regression, 847d78fb95
("bus/pci: fix FD in secondary process"), only mentioned enabling access
to config space from secondary process, which still works, as it doesn't
rely on the device file.
Fixes: 847d78fb95 ("bus/pci: fix FD in secondary process")
Signed-off-by: Konrad Sztyber
---
drivers/bus/pci/linux/pci_uio.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index 4c1d3327a9..432316afcc 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -232,18 +232,6 @@ pci_uio_alloc_resource(struct rte_pci_device *dev,
loc->domain, loc->bus, loc->devid, loc->function);
return 1;
}
- snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
-
- /* save fd */
- fd = open(devname, O_RDWR);
- if (fd < 0) {
- PCI_LOG(ERR, "Cannot open %s: %s", devname, strerror(errno));
- goto error;
- }
-
- if (rte_intr_fd_set(dev->intr_handle, fd))
- goto error;
-
snprintf(cfgname, sizeof(cfgname),
"/sys/class/uio/uio%u/device/config", uio_num);
@@ -273,6 +261,19 @@ pci_uio_alloc_resource(struct rte_pci_device *dev,
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
+ /* the uio_pci_generic driver clears the bus master enable bit when the
device file is
+* closed, so open it only in the primary process */
+ snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
+ /* save fd */
+ fd = open(devname, O_RDWR);
+ if (fd < 0) {
+ PCI_LOG(ERR, "Cannot open %s: %s", devname, strerror(errno));
+ goto error;
+ }
+
+ if (rte_intr_fd_set(dev->intr_handle, fd))
+ goto error;
+
/* allocate the mapping details for secondary processes*/
*uio_res = rte_zmalloc("UIO_RES", sizeof(**uio_res), 0);
if (*uio_res == NULL) {
--
2.45.0
[PATCH v2] bus/pci: don't open uio device in secondary process
The uio_pci_generic driver clears the bus master bit when the device
file is closed. So, when the secondary process terminates after probing
a device, that device becomes unusable in the primary process.
To avoid that, the device file is now opened only in the primary
process. The commit that introduced this regression, 847d78fb9530
("bus/pci: fix FD in secondary process"), only mentioned enabling access
to config space from secondary process, which still works, as it doesn't
rely on the device file.
Fixes: 847d78fb9530 ("bus/pci: fix FD in secondary process")
Cc: sta...@dpdk.org
Signed-off-by: Konrad Sztyber
---
drivers/bus/pci/linux/pci_uio.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index 4c1d3327a9..5c4ba8098c 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -232,18 +232,6 @@ pci_uio_alloc_resource(struct rte_pci_device *dev,
loc->domain, loc->bus, loc->devid, loc->function);
return 1;
}
- snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
-
- /* save fd */
- fd = open(devname, O_RDWR);
- if (fd < 0) {
- PCI_LOG(ERR, "Cannot open %s: %s", devname, strerror(errno));
- goto error;
- }
-
- if (rte_intr_fd_set(dev->intr_handle, fd))
- goto error;
-
snprintf(cfgname, sizeof(cfgname),
"/sys/class/uio/uio%u/device/config", uio_num);
@@ -273,6 +261,21 @@ pci_uio_alloc_resource(struct rte_pci_device *dev,
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return 0;
+ /*
+* the uio_pci_generic driver clears the bus master enable bit when the
device file is
+* closed, so open it only in the primary process
+*/
+ snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
+ /* save fd */
+ fd = open(devname, O_RDWR);
+ if (fd < 0) {
+ PCI_LOG(ERR, "Cannot open %s: %s", devname, strerror(errno));
+ goto error;
+ }
+
+ if (rte_intr_fd_set(dev->intr_handle, fd))
+ goto error;
+
/* allocate the mapping details for secondary processes*/
*uio_res = rte_zmalloc("UIO_RES", sizeof(**uio_res), 0);
if (*uio_res == NULL) {
--
2.45.0
Re: [PATCH] bus/pci: don't open uio device in secondary process
On 10/7/24 19:49, Stephen Hemminger wrote:
On Wed, 28 Aug 2024 12:40:02 +0200
Konrad Sztyber wrote:
The uio_pci_generic driver clears the bus master bit when the device
file is closed. So, when the secondary process terminates after probing
a device, that device becomes unusable in the primary process.
To avoid that, the device file is now opened only in the primary
process. The commit that introduced this regression, 847d78fb95
("bus/pci: fix FD in secondary process"), only mentioned enabling access
to config space from secondary process, which still works, as it doesn't
rely on the device file.
Fixes: 847d78fb95 ("bus/pci: fix FD in secondary process")
Signed-off-by: Konrad Sztyber
Wouldn't this break use of interrupts in the secondary process?
Yes, it will. But I don't think we can support interrupts in the
secondary process *and*, at the same time, keep the device usable in the
primary process when secondary terminates. Maybe we could pass the fd
via SCM_RIGHTS? But I don't know if that results in the same struct file
being used by both processes.
The patch does need the minor fix of the comment style.
So resubmit
I already did, see:
https://inbox.dpdk.org/dev/20240829085724.270041-1-konrad.szty...@intel.com
Re: [PATCH] bus/pci: don't open uio device in secondary process
On 10/9/24 17:12, Stephen Hemminger wrote: That is what tap, and xdp are doing. Thanks for the info. I'll take a look and will send a next rev doing something similar in uio. Konrad
[PATCH v3] bus/pci: don't open uio device in secondary process
The uio_pci_generic driver clears the bus master bit when the device
file is closed. So, when the secondary process terminates after probing
a device, that device becomes unusable in the primary process.
To avoid that, the device file is now opened only in the primary process
and the secondary gets it over UNIX domain socket via SCM_RIGHTS.
Fixes: 847d78fb9530 ("bus/pci: fix FD in secondary process")
Cc: sta...@dpdk.org
Signed-off-by: Konrad Sztyber
---
v3:
Use the rte_mp_* infrastructure to pass the uio fd from the primary
process to the secondary.
v2:
Fixed coding style issues.
---
drivers/bus/pci/linux/pci_uio.c | 140
1 file changed, 126 insertions(+), 14 deletions(-)
diff --git a/drivers/bus/pci/linux/pci_uio.c b/drivers/bus/pci/linux/pci_uio.c
index 4c1d3327a9..220390d921 100644
--- a/drivers/bus/pci/linux/pci_uio.c
+++ b/drivers/bus/pci/linux/pci_uio.c
@@ -21,14 +21,22 @@
#include
#include
#include
+#include
+#include
#include "eal_filesystem.h"
#include "pci_init.h"
#include "private.h"
void *pci_map_addr = NULL;
+static int pci_uio_dev_count;
#define OFF_MAX ((uint64_t)(off_t)-1)
+#define SEND_FD_MP_KEY "pci_uio_send_fd"
+
+struct pci_uio_send_fd_param {
+ struct rte_pci_addr addr;
+};
int
pci_uio_read_config(const struct rte_intr_handle *intr_handle,
@@ -211,6 +219,93 @@ pci_uio_free_resource(struct rte_pci_device *dev,
rte_intr_fd_set(dev->intr_handle, -1);
rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_UNKNOWN);
}
+
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ assert(pci_uio_dev_count > 0);
+ pci_uio_dev_count--;
+ if (pci_uio_dev_count == 0)
+ rte_mp_action_unregister(SEND_FD_MP_KEY);
+ }
+}
+
+static int
+pci_uio_send_fd(const struct rte_mp_msg *request, const void *peer)
+{
+ struct rte_pci_device *dev;
+ const struct pci_uio_send_fd_param *param =
+ (const struct pci_uio_send_fd_param *)request->param;
+ struct rte_mp_msg reply = {};
+ int fd;
+
+ strlcpy(reply.name, request->name, sizeof(reply.name));
+ TAILQ_FOREACH(dev, &rte_pci_bus.device_list, next) {
+ if (!rte_pci_addr_cmp(&dev->addr, ¶m->addr))
+ break;
+ }
+
+ if (dev == NULL) {
+ PCI_LOG(ERR, "Could not find PCI device (" PCI_PRI_FMT ")",
+ param->addr.domain, param->addr.bus,
+ param->addr.devid, param->addr.function);
+ goto reply;
+ }
+
+ fd = rte_intr_fd_get(dev->intr_handle);
+ if (fd < 0) {
+ PCI_LOG(ERR, "Could not get fd (" PCI_PRI_FMT ")",
+ param->addr.domain, param->addr.bus,
+ param->addr.devid, param->addr.function);
+ goto reply;
+ }
+
+ reply.num_fds = 1;
+ reply.fds[0] = fd;
+reply:
+ if (rte_mp_reply(&reply, peer) != 0) {
+ PCI_LOG(ERR, "Failed to send reply: %d (" PCI_PRI_FMT ")",
+ rte_errno, param->addr.domain, param->addr.bus,
+ param->addr.devid, param->addr.function);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+pci_uio_request_fd(struct rte_pci_device *dev)
+{
+ struct rte_mp_msg request = {}, *reply;
+ struct timespec timeout = {.tv_sec = 1, .tv_nsec = 0};
+ struct pci_uio_send_fd_param *param =
+ (struct pci_uio_send_fd_param *)request.param;
+ struct rte_mp_reply replies;
+ int rc;
+
+ strlcpy(request.name, SEND_FD_MP_KEY, sizeof(request.name));
+ memcpy(¶m->addr, &dev->addr, sizeof(param->addr));
+ request.len_param = sizeof(*param);
+
+ rc = rte_mp_request_sync(&request, &replies, &timeout);
+ if (rc != 0 || replies.nb_received != 1) {
+ PCI_LOG(ERR, "Failed to request fd from primary: %d ("
PCI_PRI_FMT ")",
+ rte_errno, dev->addr.domain, dev->addr.bus,
+ dev->addr.devid, dev->addr.function);
+ return -1;
+ }
+
+ reply = replies.msgs;
+ if (reply->num_fds != 1) {
+ PCI_LOG(ERR, "Received unexpected number of fds: %d ("
PCI_PRI_FMT ")",
+ reply->num_fds, dev->addr.domain, dev->addr.bus,
+ dev->addr.devid, dev->addr.function);
+ free(reply);
+ return -1;
+ }
+
+ rte_intr_fd_set(dev->intr_handle, reply->fds[0]);
+ free(reply);
+
+ return 0;
}
int
@@ -220,7 +315,
Re: [PATCH v3] bus/pci: don't open uio device in secondary process
On 10/24/24 11:05, David Marchand wrote:
On Fri, Oct 11, 2024 at 1:17 PM Konrad Sztyber wrote:
The uio_pci_generic driver clears the bus master bit when the device
file is closed. So, when the secondary process terminates after probing
a device, that device becomes unusable in the primary process.
To avoid that, the device file is now opened only in the primary process
and the secondary gets it over UNIX domain socket via SCM_RIGHTS.
Fixes: 847d78fb9530 ("bus/pci: fix FD in secondary process")
Cc: sta...@dpdk.org
Signed-off-by: Konrad Sztyber
Recheck-request: rebase=main,iol-compile-amd64-testing
Is there anything that's required of me to get this patch merged?
