| Message ID | 20240930171753.2572922-3-sdf@fomichev.me (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: ncdevmem: Add ncdevmem to ksft | expand |
On Mon, Sep 30, 2024 at 10:18 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > So we can plug the other ones in the future if needed. > > Cc: Mina Almasry <almasrymina@google.com> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> Feedback is minor, so with or without it: Reviewed-by: Mina Almasry <almasrymina@google.com> > --- > tools/testing/selftests/net/ncdevmem.c | 215 +++++++++++++++---------- > 1 file changed, 134 insertions(+), 81 deletions(-) > > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c > index 9245d3f158dd..557175c3bf02 100644 > --- a/tools/testing/selftests/net/ncdevmem.c > +++ b/tools/testing/selftests/net/ncdevmem.c > @@ -71,17 +71,118 @@ static char *ifname = "eth1"; > static unsigned int ifindex; > static unsigned int dmabuf_id; > > -void print_bytes(void *ptr, size_t size) > +struct memory_buffer { > + int fd; > + size_t size; > + > + int devfd; > + int memfd; > + char *buf_mem; > +}; > + > +struct memory_provider { > + struct memory_buffer *(*alloc)(size_t size); > + void (*free)(struct memory_buffer *ctx); > + void (*memcpy_to_device)(struct memory_buffer *dst, size_t off, > + void *src, int n); AFAICT all this code in this api and its implementation is unused. I'm guessing this is for the future TX path testing? If you get the chance to slice these changes out of this patch and punting them for when the TX changes are ready, please do. Just to minimize dead code until TX path is added, but since these are tests, dead code is not a huge deal. > + void (*memcpy_from_device)(void *dst, struct memory_buffer *src, > + size_t off, int n); > +}; > + > +static struct memory_buffer *udmabuf_alloc(size_t size) > { > - unsigned char *p = ptr; > - int i; > + struct udmabuf_create create; > + struct memory_buffer *ctx; > + int ret; > > - for (i = 0; i < size; i++) > - printf("%02hhX ", p[i]); > - printf("\n"); > + ctx = malloc(sizeof(*ctx)); > + if (!ctx) > + error(1, ENOMEM, "malloc failed"); > + > + ctx->size = size; > + > + ctx->devfd = open("/dev/udmabuf", O_RDWR); > + if (ctx->devfd < 0) > + error(1, errno, > + "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n", > + TEST_PREFIX); > + > + ctx->memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING); > + if (ctx->memfd < 0) > + error(1, errno, "%s: [skip,no-memfd]\n", TEST_PREFIX); > + > + ret = fcntl(ctx->memfd, F_ADD_SEALS, F_SEAL_SHRINK); > + if (ret < 0) > + error(1, errno, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); > + > + ret = ftruncate(ctx->memfd, size); > + if (ret == -1) > + error(1, errno, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); > + > + memset(&create, 0, sizeof(create)); > + > + create.memfd = ctx->memfd; > + create.offset = 0; > + create.size = size; > + ctx->fd = ioctl(ctx->devfd, UDMABUF_CREATE, &create); > + if (ctx->fd < 0) > + error(1, errno, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX); > + > + ctx->buf_mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, > + ctx->fd, 0); > + if (ctx->buf_mem == MAP_FAILED) > + error(1, errno, "%s: [FAIL, map udmabuf]\n", TEST_PREFIX); > + > + return ctx; > +} > + > +static void udmabuf_free(struct memory_buffer *ctx) > +{ > + munmap(ctx->buf_mem, ctx->size); > + close(ctx->fd); > + close(ctx->memfd); > + close(ctx->devfd); > + free(ctx); > +} > + > +static void udmabuf_memcpy_to_device(struct memory_buffer *dst, size_t off, > + void *src, int n) > +{ > + struct dma_buf_sync sync = {}; > + > + sync.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_WRITE; > + ioctl(dst->fd, DMA_BUF_IOCTL_SYNC, &sync); > + > + memcpy(dst->buf_mem + off, src, n); > + > + sync.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_WRITE; > + ioctl(dst->fd, DMA_BUF_IOCTL_SYNC, &sync); > +} > + > +static void udmabuf_memcpy_from_device(void *dst, struct memory_buffer *src, > + size_t off, int n) > +{ > + struct dma_buf_sync sync = {}; > + > + sync.flags = DMA_BUF_SYNC_START; > + ioctl(src->fd, DMA_BUF_IOCTL_SYNC, &sync); > + > + memcpy(dst, src->buf_mem + off, n); > + > + sync.flags = DMA_BUF_SYNC_END; > + ioctl(src->fd, DMA_BUF_IOCTL_SYNC, &sync); > } > > -void print_nonzero_bytes(void *ptr, size_t size) > +static struct memory_provider udmabuf_memory_provider = { > + .alloc = udmabuf_alloc, > + .free = udmabuf_free, > + .memcpy_to_device = udmabuf_memcpy_to_device, > + .memcpy_from_device = udmabuf_memcpy_from_device, > +}; > + > +static struct memory_provider *provider = &udmabuf_memory_provider; > + > +static void print_nonzero_bytes(void *ptr, size_t size) > { > unsigned char *p = ptr; > unsigned int i; > @@ -201,42 +302,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd, > return -1; > } > > -static void create_udmabuf(int *devfd, int *memfd, int *buf, size_t dmabuf_size) > -{ > - struct udmabuf_create create; > - int ret; > - > - *devfd = open("/dev/udmabuf", O_RDWR); > - if (*devfd < 0) { > - error(70, 0, > - "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n", > - TEST_PREFIX); > - } > - > - *memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING); > - if (*memfd < 0) > - error(70, 0, "%s: [skip,no-memfd]\n", TEST_PREFIX); > - > - /* Required for udmabuf */ > - ret = fcntl(*memfd, F_ADD_SEALS, F_SEAL_SHRINK); > - if (ret < 0) > - error(73, 0, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); > - > - ret = ftruncate(*memfd, dmabuf_size); > - if (ret == -1) > - error(74, 0, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); > - > - memset(&create, 0, sizeof(create)); > - > - create.memfd = *memfd; > - create.offset = 0; > - create.size = dmabuf_size; > - *buf = ioctl(*devfd, UDMABUF_CREATE, &create); > - if (*buf < 0) > - error(75, 0, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX); > -} > - > -int do_server(void) > +int do_server(struct memory_buffer *mem) > { > char ctrl_data[sizeof(int) * 20000]; > struct netdev_queue_id *queues; > @@ -244,23 +310,18 @@ int do_server(void) > struct sockaddr_in client_addr; > struct sockaddr_in server_sin; > size_t page_aligned_frags = 0; > - int devfd, memfd, buf, ret; > size_t total_received = 0; > socklen_t client_addr_len; > bool is_devmem = false; > - char *buf_mem = NULL; > + char *tmp_mem = NULL; > struct ynl_sock *ys; > - size_t dmabuf_size; > char iobuf[819200]; > char buffer[256]; > int socket_fd; > int client_fd; > size_t i = 0; > int opt = 1; > - > - dmabuf_size = getpagesize() * NUM_PAGES; > - > - create_udmabuf(&devfd, &memfd, &buf, dmabuf_size); > + int ret; > > if (reset_flow_steering()) > error(1, 0, "Failed to reset flow steering\n"); > @@ -284,13 +345,12 @@ int do_server(void) > queues[i].id = start_queue + i; > } > > - if (bind_rx_queue(ifindex, buf, queues, num_queues, &ys)) > + if (bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) > error(1, 0, "Failed to bind\n"); > > - buf_mem = mmap(NULL, dmabuf_size, PROT_READ | PROT_WRITE, MAP_SHARED, > - buf, 0); > - if (buf_mem == MAP_FAILED) > - error(1, 0, "mmap()"); > + tmp_mem = malloc(mem->size); > + if (!tmp_mem) > + error(1, ENOMEM, "malloc failed"); > > server_sin.sin_family = AF_INET; > server_sin.sin_port = htons(atoi(port)); > @@ -341,7 +401,6 @@ int do_server(void) > struct iovec iov = { .iov_base = iobuf, > .iov_len = sizeof(iobuf) }; > struct dmabuf_cmsg *dmabuf_cmsg = NULL; > - struct dma_buf_sync sync = { 0 }; > struct cmsghdr *cm = NULL; > struct msghdr msg = { 0 }; > struct dmabuf_token token; > @@ -410,22 +469,17 @@ int do_server(void) > else > page_aligned_frags++; > > - sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_START; > - ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync); > + provider->memcpy_from_device(tmp_mem, mem, > + dmabuf_cmsg->frag_offset, > + dmabuf_cmsg->frag_size); > > if (do_validation) > validate_buffer( > - ((unsigned char *)buf_mem) + > + ((unsigned char *)tmp_mem) + > dmabuf_cmsg->frag_offset, > dmabuf_cmsg->frag_size); > else > - print_nonzero_bytes( > - ((unsigned char *)buf_mem) + > - dmabuf_cmsg->frag_offset, > - dmabuf_cmsg->frag_size); > - > - sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_END; > - ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync); > + print_nonzero_bytes(tmp_mem, dmabuf_cmsg->frag_size); > > ret = setsockopt(client_fd, SOL_SOCKET, > SO_DEVMEM_DONTNEED, &token, > @@ -450,12 +504,9 @@ int do_server(void) > > cleanup: > > - munmap(buf_mem, dmabuf_size); > + free(tmp_mem); > close(client_fd); > close(socket_fd); > - close(buf); > - close(memfd); > - close(devfd); > ynl_sock_destroy(ys); > > return 0; > @@ -464,14 +515,11 @@ int do_server(void) > void run_devmem_tests(void) > { > struct netdev_queue_id *queues; > - int devfd, memfd, buf; > + struct memory_buffer *mem; > struct ynl_sock *ys; > - size_t dmabuf_size; > size_t i = 0; > > - dmabuf_size = getpagesize() * NUM_PAGES; > - > - create_udmabuf(&devfd, &memfd, &buf, dmabuf_size); > + mem = provider->alloc(getpagesize() * NUM_PAGES); > There is some weirdness here where run_devmem_tests() allocates its own provider, but do_server() uses a provider allocated in main(). Any reason these are not symmetric? I would marginally prefer do_server() to allocate its own provider just like run_devmem_tests(), or at least make them both match, if possible.
On 10/03, Mina Almasry wrote: > On Mon, Sep 30, 2024 at 10:18 AM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > So we can plug the other ones in the future if needed. > > > > Cc: Mina Almasry <almasrymina@google.com> > > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> > > Feedback is minor, so with or without it: > > Reviewed-by: Mina Almasry <almasrymina@google.com> > > > --- > > tools/testing/selftests/net/ncdevmem.c | 215 +++++++++++++++---------- > > 1 file changed, 134 insertions(+), 81 deletions(-) > > > > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c > > index 9245d3f158dd..557175c3bf02 100644 > > --- a/tools/testing/selftests/net/ncdevmem.c > > +++ b/tools/testing/selftests/net/ncdevmem.c > > @@ -71,17 +71,118 @@ static char *ifname = "eth1"; > > static unsigned int ifindex; > > static unsigned int dmabuf_id; > > > > -void print_bytes(void *ptr, size_t size) > > +struct memory_buffer { > > + int fd; > > + size_t size; > > + > > + int devfd; > > + int memfd; > > + char *buf_mem; > > +}; > > + > > +struct memory_provider { > > + struct memory_buffer *(*alloc)(size_t size); > > + void (*free)(struct memory_buffer *ctx); > > + void (*memcpy_to_device)(struct memory_buffer *dst, size_t off, > > + void *src, int n); > > AFAICT all this code in this api and its implementation is unused. I'm > guessing this is for the future TX path testing? > > If you get the chance to slice these changes out of this patch and > punting them for when the TX changes are ready, please do. Just to > minimize dead code until TX path is added, but since these are tests, > dead code is not a huge deal. Ah, yes, I'll remove it for now. > > + void (*memcpy_from_device)(void *dst, struct memory_buffer *src, > > + size_t off, int n); > > +}; > > + > > +static struct memory_buffer *udmabuf_alloc(size_t size) > > { > > - unsigned char *p = ptr; > > - int i; > > + struct udmabuf_create create; > > + struct memory_buffer *ctx; > > + int ret; > > > > - for (i = 0; i < size; i++) > > - printf("%02hhX ", p[i]); > > - printf("\n"); > > + ctx = malloc(sizeof(*ctx)); > > + if (!ctx) > > + error(1, ENOMEM, "malloc failed"); > > + > > + ctx->size = size; > > + > > + ctx->devfd = open("/dev/udmabuf", O_RDWR); > > + if (ctx->devfd < 0) > > + error(1, errno, > > + "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n", > > + TEST_PREFIX); > > + > > + ctx->memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING); > > + if (ctx->memfd < 0) > > + error(1, errno, "%s: [skip,no-memfd]\n", TEST_PREFIX); > > + > > + ret = fcntl(ctx->memfd, F_ADD_SEALS, F_SEAL_SHRINK); > > + if (ret < 0) > > + error(1, errno, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); > > + > > + ret = ftruncate(ctx->memfd, size); > > + if (ret == -1) > > + error(1, errno, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); > > + > > + memset(&create, 0, sizeof(create)); > > + > > + create.memfd = ctx->memfd; > > + create.offset = 0; > > + create.size = size; > > + ctx->fd = ioctl(ctx->devfd, UDMABUF_CREATE, &create); > > + if (ctx->fd < 0) > > + error(1, errno, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX); > > + > > + ctx->buf_mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, > > + ctx->fd, 0); > > + if (ctx->buf_mem == MAP_FAILED) > > + error(1, errno, "%s: [FAIL, map udmabuf]\n", TEST_PREFIX); > > + > > + return ctx; > > +} > > + > > +static void udmabuf_free(struct memory_buffer *ctx) > > +{ > > + munmap(ctx->buf_mem, ctx->size); > > + close(ctx->fd); > > + close(ctx->memfd); > > + close(ctx->devfd); > > + free(ctx); > > +} > > + > > +static void udmabuf_memcpy_to_device(struct memory_buffer *dst, size_t off, > > + void *src, int n) > > +{ > > + struct dma_buf_sync sync = {}; > > + > > + sync.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_WRITE; > > + ioctl(dst->fd, DMA_BUF_IOCTL_SYNC, &sync); > > + > > + memcpy(dst->buf_mem + off, src, n); > > + > > + sync.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_WRITE; > > + ioctl(dst->fd, DMA_BUF_IOCTL_SYNC, &sync); > > +} > > + > > +static void udmabuf_memcpy_from_device(void *dst, struct memory_buffer *src, > > + size_t off, int n) > > +{ > > + struct dma_buf_sync sync = {}; > > + > > + sync.flags = DMA_BUF_SYNC_START; > > + ioctl(src->fd, DMA_BUF_IOCTL_SYNC, &sync); > > + > > + memcpy(dst, src->buf_mem + off, n); > > + > > + sync.flags = DMA_BUF_SYNC_END; > > + ioctl(src->fd, DMA_BUF_IOCTL_SYNC, &sync); > > } > > > > -void print_nonzero_bytes(void *ptr, size_t size) > > +static struct memory_provider udmabuf_memory_provider = { > > + .alloc = udmabuf_alloc, > > + .free = udmabuf_free, > > + .memcpy_to_device = udmabuf_memcpy_to_device, > > + .memcpy_from_device = udmabuf_memcpy_from_device, > > +}; > > + > > +static struct memory_provider *provider = &udmabuf_memory_provider; > > + > > +static void print_nonzero_bytes(void *ptr, size_t size) > > { > > unsigned char *p = ptr; > > unsigned int i; > > @@ -201,42 +302,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd, > > return -1; > > } > > > > -static void create_udmabuf(int *devfd, int *memfd, int *buf, size_t dmabuf_size) > > -{ > > - struct udmabuf_create create; > > - int ret; > > - > > - *devfd = open("/dev/udmabuf", O_RDWR); > > - if (*devfd < 0) { > > - error(70, 0, > > - "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n", > > - TEST_PREFIX); > > - } > > - > > - *memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING); > > - if (*memfd < 0) > > - error(70, 0, "%s: [skip,no-memfd]\n", TEST_PREFIX); > > - > > - /* Required for udmabuf */ > > - ret = fcntl(*memfd, F_ADD_SEALS, F_SEAL_SHRINK); > > - if (ret < 0) > > - error(73, 0, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); > > - > > - ret = ftruncate(*memfd, dmabuf_size); > > - if (ret == -1) > > - error(74, 0, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); > > - > > - memset(&create, 0, sizeof(create)); > > - > > - create.memfd = *memfd; > > - create.offset = 0; > > - create.size = dmabuf_size; > > - *buf = ioctl(*devfd, UDMABUF_CREATE, &create); > > - if (*buf < 0) > > - error(75, 0, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX); > > -} > > - > > -int do_server(void) > > +int do_server(struct memory_buffer *mem) > > { > > char ctrl_data[sizeof(int) * 20000]; > > struct netdev_queue_id *queues; > > @@ -244,23 +310,18 @@ int do_server(void) > > struct sockaddr_in client_addr; > > struct sockaddr_in server_sin; > > size_t page_aligned_frags = 0; > > - int devfd, memfd, buf, ret; > > size_t total_received = 0; > > socklen_t client_addr_len; > > bool is_devmem = false; > > - char *buf_mem = NULL; > > + char *tmp_mem = NULL; > > struct ynl_sock *ys; > > - size_t dmabuf_size; > > char iobuf[819200]; > > char buffer[256]; > > int socket_fd; > > int client_fd; > > size_t i = 0; > > int opt = 1; > > - > > - dmabuf_size = getpagesize() * NUM_PAGES; > > - > > - create_udmabuf(&devfd, &memfd, &buf, dmabuf_size); > > + int ret; > > > > if (reset_flow_steering()) > > error(1, 0, "Failed to reset flow steering\n"); > > @@ -284,13 +345,12 @@ int do_server(void) > > queues[i].id = start_queue + i; > > } > > > > - if (bind_rx_queue(ifindex, buf, queues, num_queues, &ys)) > > + if (bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) > > error(1, 0, "Failed to bind\n"); > > > > - buf_mem = mmap(NULL, dmabuf_size, PROT_READ | PROT_WRITE, MAP_SHARED, > > - buf, 0); > > - if (buf_mem == MAP_FAILED) > > - error(1, 0, "mmap()"); > > + tmp_mem = malloc(mem->size); > > + if (!tmp_mem) > > + error(1, ENOMEM, "malloc failed"); > > > > server_sin.sin_family = AF_INET; > > server_sin.sin_port = htons(atoi(port)); > > @@ -341,7 +401,6 @@ int do_server(void) > > struct iovec iov = { .iov_base = iobuf, > > .iov_len = sizeof(iobuf) }; > > struct dmabuf_cmsg *dmabuf_cmsg = NULL; > > - struct dma_buf_sync sync = { 0 }; > > struct cmsghdr *cm = NULL; > > struct msghdr msg = { 0 }; > > struct dmabuf_token token; > > @@ -410,22 +469,17 @@ int do_server(void) > > else > > page_aligned_frags++; > > > > - sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_START; > > - ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync); > > + provider->memcpy_from_device(tmp_mem, mem, > > + dmabuf_cmsg->frag_offset, > > + dmabuf_cmsg->frag_size); > > > > if (do_validation) > > validate_buffer( > > - ((unsigned char *)buf_mem) + > > + ((unsigned char *)tmp_mem) + > > dmabuf_cmsg->frag_offset, > > dmabuf_cmsg->frag_size); > > else > > - print_nonzero_bytes( > > - ((unsigned char *)buf_mem) + > > - dmabuf_cmsg->frag_offset, > > - dmabuf_cmsg->frag_size); > > - > > - sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_END; > > - ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync); > > + print_nonzero_bytes(tmp_mem, dmabuf_cmsg->frag_size); > > > > ret = setsockopt(client_fd, SOL_SOCKET, > > SO_DEVMEM_DONTNEED, &token, > > @@ -450,12 +504,9 @@ int do_server(void) > > > > cleanup: > > > > - munmap(buf_mem, dmabuf_size); > > + free(tmp_mem); > > close(client_fd); > > close(socket_fd); > > - close(buf); > > - close(memfd); > > - close(devfd); > > ynl_sock_destroy(ys); > > > > return 0; > > @@ -464,14 +515,11 @@ int do_server(void) > > void run_devmem_tests(void) > > { > > struct netdev_queue_id *queues; > > - int devfd, memfd, buf; > > + struct memory_buffer *mem; > > struct ynl_sock *ys; > > - size_t dmabuf_size; > > size_t i = 0; > > > > - dmabuf_size = getpagesize() * NUM_PAGES; > > - > > - create_udmabuf(&devfd, &memfd, &buf, dmabuf_size); > > + mem = provider->alloc(getpagesize() * NUM_PAGES); > > > > There is some weirdness here where run_devmem_tests() allocates its > own provider, but do_server() uses a provider allocated in main(). Any > reason these are not symmetric? I would marginally prefer do_server() > to allocate its own provider just like run_devmem_tests(), or at least > make them both match, if possible. I wanted to keep them separate in case we end up adding more to the selftest part. For example, not sure what would happen now if we pass a udmabuf with just one page? Do we need some test for the drivers to make sure they handle this case?
On Thu, Oct 3, 2024 at 3:08 PM Stanislav Fomichev <stfomichev@gmail.com> wrote: > > > > @@ -464,14 +515,11 @@ int do_server(void) > > > void run_devmem_tests(void) > > > { > > > struct netdev_queue_id *queues; > > > - int devfd, memfd, buf; > > > + struct memory_buffer *mem; > > > struct ynl_sock *ys; > > > - size_t dmabuf_size; > > > size_t i = 0; > > > > > > - dmabuf_size = getpagesize() * NUM_PAGES; > > > - > > > - create_udmabuf(&devfd, &memfd, &buf, dmabuf_size); > > > + mem = provider->alloc(getpagesize() * NUM_PAGES); > > > > > > > There is some weirdness here where run_devmem_tests() allocates its > > own provider, but do_server() uses a provider allocated in main(). Any > > reason these are not symmetric? I would marginally prefer do_server() > > to allocate its own provider just like run_devmem_tests(), or at least > > make them both match, if possible. > > I wanted to keep them separate in case we end up adding more to > the selftest part. For example, not sure what would happen now if we pass > a udmabuf with just one page? Do we need some test for the drivers > to make sure they handle this case? The size of the udmabuf (or more generically, any dmabuf) is the address space for the page pool; if the dmabuf is too small, like 1 page, the rest of the pp allocations will return -ENOMEM.
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c index 9245d3f158dd..557175c3bf02 100644 --- a/tools/testing/selftests/net/ncdevmem.c +++ b/tools/testing/selftests/net/ncdevmem.c @@ -71,17 +71,118 @@ static char *ifname = "eth1"; static unsigned int ifindex; static unsigned int dmabuf_id; -void print_bytes(void *ptr, size_t size) +struct memory_buffer { + int fd; + size_t size; + + int devfd; + int memfd; + char *buf_mem; +}; + +struct memory_provider { + struct memory_buffer *(*alloc)(size_t size); + void (*free)(struct memory_buffer *ctx); + void (*memcpy_to_device)(struct memory_buffer *dst, size_t off, + void *src, int n); + void (*memcpy_from_device)(void *dst, struct memory_buffer *src, + size_t off, int n); +}; + +static struct memory_buffer *udmabuf_alloc(size_t size) { - unsigned char *p = ptr; - int i; + struct udmabuf_create create; + struct memory_buffer *ctx; + int ret; - for (i = 0; i < size; i++) - printf("%02hhX ", p[i]); - printf("\n"); + ctx = malloc(sizeof(*ctx)); + if (!ctx) + error(1, ENOMEM, "malloc failed"); + + ctx->size = size; + + ctx->devfd = open("/dev/udmabuf", O_RDWR); + if (ctx->devfd < 0) + error(1, errno, + "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n", + TEST_PREFIX); + + ctx->memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING); + if (ctx->memfd < 0) + error(1, errno, "%s: [skip,no-memfd]\n", TEST_PREFIX); + + ret = fcntl(ctx->memfd, F_ADD_SEALS, F_SEAL_SHRINK); + if (ret < 0) + error(1, errno, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); + + ret = ftruncate(ctx->memfd, size); + if (ret == -1) + error(1, errno, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); + + memset(&create, 0, sizeof(create)); + + create.memfd = ctx->memfd; + create.offset = 0; + create.size = size; + ctx->fd = ioctl(ctx->devfd, UDMABUF_CREATE, &create); + if (ctx->fd < 0) + error(1, errno, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX); + + ctx->buf_mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, + ctx->fd, 0); + if (ctx->buf_mem == MAP_FAILED) + error(1, errno, "%s: [FAIL, map udmabuf]\n", TEST_PREFIX); + + return ctx; +} + +static void udmabuf_free(struct memory_buffer *ctx) +{ + munmap(ctx->buf_mem, ctx->size); + close(ctx->fd); + close(ctx->memfd); + close(ctx->devfd); + free(ctx); +} + +static void udmabuf_memcpy_to_device(struct memory_buffer *dst, size_t off, + void *src, int n) +{ + struct dma_buf_sync sync = {}; + + sync.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_WRITE; + ioctl(dst->fd, DMA_BUF_IOCTL_SYNC, &sync); + + memcpy(dst->buf_mem + off, src, n); + + sync.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_WRITE; + ioctl(dst->fd, DMA_BUF_IOCTL_SYNC, &sync); +} + +static void udmabuf_memcpy_from_device(void *dst, struct memory_buffer *src, + size_t off, int n) +{ + struct dma_buf_sync sync = {}; + + sync.flags = DMA_BUF_SYNC_START; + ioctl(src->fd, DMA_BUF_IOCTL_SYNC, &sync); + + memcpy(dst, src->buf_mem + off, n); + + sync.flags = DMA_BUF_SYNC_END; + ioctl(src->fd, DMA_BUF_IOCTL_SYNC, &sync); } -void print_nonzero_bytes(void *ptr, size_t size) +static struct memory_provider udmabuf_memory_provider = { + .alloc = udmabuf_alloc, + .free = udmabuf_free, + .memcpy_to_device = udmabuf_memcpy_to_device, + .memcpy_from_device = udmabuf_memcpy_from_device, +}; + +static struct memory_provider *provider = &udmabuf_memory_provider; + +static void print_nonzero_bytes(void *ptr, size_t size) { unsigned char *p = ptr; unsigned int i; @@ -201,42 +302,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd, return -1; } -static void create_udmabuf(int *devfd, int *memfd, int *buf, size_t dmabuf_size) -{ - struct udmabuf_create create; - int ret; - - *devfd = open("/dev/udmabuf", O_RDWR); - if (*devfd < 0) { - error(70, 0, - "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n", - TEST_PREFIX); - } - - *memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING); - if (*memfd < 0) - error(70, 0, "%s: [skip,no-memfd]\n", TEST_PREFIX); - - /* Required for udmabuf */ - ret = fcntl(*memfd, F_ADD_SEALS, F_SEAL_SHRINK); - if (ret < 0) - error(73, 0, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); - - ret = ftruncate(*memfd, dmabuf_size); - if (ret == -1) - error(74, 0, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); - - memset(&create, 0, sizeof(create)); - - create.memfd = *memfd; - create.offset = 0; - create.size = dmabuf_size; - *buf = ioctl(*devfd, UDMABUF_CREATE, &create); - if (*buf < 0) - error(75, 0, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX); -} - -int do_server(void) +int do_server(struct memory_buffer *mem) { char ctrl_data[sizeof(int) * 20000]; struct netdev_queue_id *queues; @@ -244,23 +310,18 @@ int do_server(void) struct sockaddr_in client_addr; struct sockaddr_in server_sin; size_t page_aligned_frags = 0; - int devfd, memfd, buf, ret; size_t total_received = 0; socklen_t client_addr_len; bool is_devmem = false; - char *buf_mem = NULL; + char *tmp_mem = NULL; struct ynl_sock *ys; - size_t dmabuf_size; char iobuf[819200]; char buffer[256]; int socket_fd; int client_fd; size_t i = 0; int opt = 1; - - dmabuf_size = getpagesize() * NUM_PAGES; - - create_udmabuf(&devfd, &memfd, &buf, dmabuf_size); + int ret; if (reset_flow_steering()) error(1, 0, "Failed to reset flow steering\n"); @@ -284,13 +345,12 @@ int do_server(void) queues[i].id = start_queue + i; } - if (bind_rx_queue(ifindex, buf, queues, num_queues, &ys)) + if (bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) error(1, 0, "Failed to bind\n"); - buf_mem = mmap(NULL, dmabuf_size, PROT_READ | PROT_WRITE, MAP_SHARED, - buf, 0); - if (buf_mem == MAP_FAILED) - error(1, 0, "mmap()"); + tmp_mem = malloc(mem->size); + if (!tmp_mem) + error(1, ENOMEM, "malloc failed"); server_sin.sin_family = AF_INET; server_sin.sin_port = htons(atoi(port)); @@ -341,7 +401,6 @@ int do_server(void) struct iovec iov = { .iov_base = iobuf, .iov_len = sizeof(iobuf) }; struct dmabuf_cmsg *dmabuf_cmsg = NULL; - struct dma_buf_sync sync = { 0 }; struct cmsghdr *cm = NULL; struct msghdr msg = { 0 }; struct dmabuf_token token; @@ -410,22 +469,17 @@ int do_server(void) else page_aligned_frags++; - sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_START; - ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync); + provider->memcpy_from_device(tmp_mem, mem, + dmabuf_cmsg->frag_offset, + dmabuf_cmsg->frag_size); if (do_validation) validate_buffer( - ((unsigned char *)buf_mem) + + ((unsigned char *)tmp_mem) + dmabuf_cmsg->frag_offset, dmabuf_cmsg->frag_size); else - print_nonzero_bytes( - ((unsigned char *)buf_mem) + - dmabuf_cmsg->frag_offset, - dmabuf_cmsg->frag_size); - - sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_END; - ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync); + print_nonzero_bytes(tmp_mem, dmabuf_cmsg->frag_size); ret = setsockopt(client_fd, SOL_SOCKET, SO_DEVMEM_DONTNEED, &token, @@ -450,12 +504,9 @@ int do_server(void) cleanup: - munmap(buf_mem, dmabuf_size); + free(tmp_mem); close(client_fd); close(socket_fd); - close(buf); - close(memfd); - close(devfd); ynl_sock_destroy(ys); return 0; @@ -464,14 +515,11 @@ int do_server(void) void run_devmem_tests(void) { struct netdev_queue_id *queues; - int devfd, memfd, buf; + struct memory_buffer *mem; struct ynl_sock *ys; - size_t dmabuf_size; size_t i = 0; - dmabuf_size = getpagesize() * NUM_PAGES; - - create_udmabuf(&devfd, &memfd, &buf, dmabuf_size); + mem = provider->alloc(getpagesize() * NUM_PAGES); /* Configure RSS to divert all traffic from our devmem queues */ if (configure_rss()) @@ -482,7 +530,7 @@ void run_devmem_tests(void) if (configure_headersplit(1)) error(1, 0, "Failed to configure header split\n"); - if (!bind_rx_queue(ifindex, buf, queues, num_queues, &ys)) + if (!bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) error(1, 0, "Binding empty queues array should have failed\n"); for (i = 0; i < num_queues; i++) { @@ -495,7 +543,7 @@ void run_devmem_tests(void) if (configure_headersplit(0)) error(1, 0, "Failed to configure header split\n"); - if (!bind_rx_queue(ifindex, buf, queues, num_queues, &ys)) + if (!bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) error(1, 0, "Configure dmabuf with header split off should have failed\n"); if (configure_headersplit(1)) @@ -508,7 +556,7 @@ void run_devmem_tests(void) queues[i].id = start_queue + i; } - if (bind_rx_queue(ifindex, buf, queues, num_queues, &ys)) + if (bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) error(1, 0, "Failed to bind\n"); /* Deactivating a bound queue should not be legal */ @@ -517,11 +565,15 @@ void run_devmem_tests(void) /* Closing the netlink socket does an implicit unbind */ ynl_sock_destroy(ys); + + provider->free(mem); } int main(int argc, char *argv[]) { + struct memory_buffer *mem; int is_server = 0, opt; + int ret; while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:")) != -1) { switch (opt) { @@ -562,8 +614,9 @@ int main(int argc, char *argv[]) run_devmem_tests(); - if (is_server) - return do_server(); + mem = provider->alloc(getpagesize() * NUM_PAGES); + ret = is_server ? do_server(mem) : 1; + provider->free(mem); - return 0; + return ret; }
So we can plug the other ones in the future if needed. Cc: Mina Almasry <almasrymina@google.com> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> --- tools/testing/selftests/net/ncdevmem.c | 215 +++++++++++++++---------- 1 file changed, 134 insertions(+), 81 deletions(-)