From 9af7a13fe4872344b02c7898e1aeb74ea9a7c2de Mon Sep 17 00:00:00 2001 From: Octavian Purdila Date: Wed, 17 Aug 2016 00:11:00 +0300 Subject: [PATCH 1/2] lkl tools: virtio net fd: fix error handling in lkl_register_netdev_fd Signed-off-by: Octavian Purdila --- tools/lkl/lib/virtio_net_fd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/lkl/lib/virtio_net_fd.c b/tools/lkl/lib/virtio_net_fd.c index 37066c8dc79be4..d48a6a70dfab59 100644 --- a/tools/lkl/lib/virtio_net_fd.c +++ b/tools/lkl/lib/virtio_net_fd.c @@ -195,6 +195,7 @@ struct lkl_netdev *lkl_register_netdev_fd(int fd) close(nd->pipe[0]); close(nd->pipe[1]); lkl_unregister_netdev_fd(&nd->dev); + return NULL; } nd->dev.ops = &fd_net_ops; From a7af721ecbbabbe7447bbcfeb518abc8e9f8a9b9 Mon Sep 17 00:00:00 2001 From: Octavian Purdila Date: Tue, 16 Aug 2016 23:52:49 +0300 Subject: [PATCH 2/2] lkl tools: virtio net: add lkl_netdev_free Right now we free the lkl_netdev structure in lkl_netdev_remove. This has two issues: it does more then its counterpart lkl_netdev_alloc and it relies on the facts that the structure is allocated with lkl_ops.alloc_mem and that lkl_netdev is at the beggining of the backend structure. This patch implements a consistent netdev deallocation by adding lkl_netdev_free and converting the close netdev operation into two other operations: poll_hup and free. poll_hup is used to stop the polling thread while free is used to free the backends resources and the lkl_netdev structure. Signed-off-by: Octavian Purdila --- tools/lkl/include/lkl.h | 7 +++++++ tools/lkl/include/lkl_host.h | 13 +++++++++---- tools/lkl/lib/hijack/init.c | 5 ++++- tools/lkl/lib/virtio_net.c | 13 ++++++++----- tools/lkl/lib/virtio_net_dpdk.c | 13 +++++++++++-- tools/lkl/lib/virtio_net_fd.c | 30 +++++++++++++++--------------- tools/lkl/lib/virtio_net_fd.h | 8 -------- tools/lkl/lib/virtio_net_vde.c | 16 +++++++++++++--- 8 files changed, 67 insertions(+), 38 deletions(-) diff --git a/tools/lkl/include/lkl.h b/tools/lkl/include/lkl.h index 2a791a9c5fb404..9d55adffbdf76b 100644 --- a/tools/lkl/include/lkl.h +++ b/tools/lkl/include/lkl.h @@ -279,6 +279,13 @@ int lkl_netdev_add(struct lkl_netdev *nd, struct lkl_netdev_args* args); */ void lkl_netdev_remove(int id); +/** + * lkl_netdev_free - frees a network device + * + * @nd - the network device to free + */ +void lkl_netdev_free(struct lkl_netdev *nd); + /** * lkl_netdev_get_ifindex - retrieve the interface index for a given network * device id diff --git a/tools/lkl/include/lkl_host.h b/tools/lkl/include/lkl_host.h index b845ee2e2c0f45..03ebb04c50fb6f 100644 --- a/tools/lkl/include/lkl_host.h +++ b/tools/lkl/include/lkl_host.h @@ -97,12 +97,17 @@ struct lkl_dev_net_ops { int (*poll)(struct lkl_netdev *nd); /* - * Closes a net device. + * Make poll wakeup and return LKL_DEV_NET_POLL_HUP. + */ + void (*poll_hup)(struct lkl_netdev *nd); + + /* + * Frees a network device. * - * Implementation must release its resources and poll must wakeup and - * return LKL_DEV_NET_POLL_HUP. + * Implementation must release its resources and free the network device + * structure. */ - void (*close)(struct lkl_netdev *nd); + void (*free)(struct lkl_netdev *nd); }; #ifdef __cplusplus diff --git a/tools/lkl/lib/hijack/init.c b/tools/lkl/lib/hijack/init.c index 9ca6745f393606..63ff47de78c768 100644 --- a/tools/lkl/lib/hijack/init.c +++ b/tools/lkl/lib/hijack/init.c @@ -179,6 +179,7 @@ static void PinToFirstCpu(const cpu_set_t* cpus) int lkl_debug, lkl_running; static int nd_id = -1; +static struct lkl_netdev *nd; void __attribute__((constructor(102))) hijack_init(void) @@ -199,7 +200,6 @@ hijack_init(void) char *gateway6 = getenv("LKL_HIJACK_NET_GATEWAY6"); char *debug = getenv("LKL_HIJACK_DEBUG"); char *mount = getenv("LKL_HIJACK_MOUNT"); - struct lkl_netdev *nd = NULL; struct lkl_netdev_args nd_args; char *neigh_entries = getenv("LKL_HIJACK_NET_NEIGHBOR"); /* single_cpu mode: @@ -436,5 +436,8 @@ hijack_fini(void) if (nd_id >= 0) lkl_netdev_remove(nd_id); + if (nd) + lkl_netdev_free(nd); + lkl_sys_halt(); } diff --git a/tools/lkl/lib/virtio_net.c b/tools/lkl/lib/virtio_net.c index d0a34ae484cb9a..7e670aab69359f 100644 --- a/tools/lkl/lib/virtio_net.c +++ b/tools/lkl/lib/virtio_net.c @@ -275,6 +275,9 @@ void lkl_netdev_remove(int id) dev = registered_devs[id]; + dev->nd->ops->poll_hup(dev->nd); + lkl_host_ops.thread_join(dev->poll_tid); + ret = lkl_netdev_get_ifindex(id); if (ret < 0) { lkl_printf("%s: failed to get ifindex for id %d: %s\n", @@ -289,13 +292,13 @@ void lkl_netdev_remove(int id) return; } - dev->nd->ops->close(dev->nd); - - lkl_host_ops.thread_join(dev->poll_tid); - virtio_dev_cleanup(&dev->dev); - lkl_host_ops.mem_free(dev->nd); free_queue_locks(dev->queue_locks, NUM_QUEUES); lkl_host_ops.mem_free(dev); } + +void lkl_netdev_free(struct lkl_netdev *nd) +{ + nd->ops->free(nd); +} diff --git a/tools/lkl/lib/virtio_net_dpdk.c b/tools/lkl/lib/virtio_net_dpdk.c index 1e73c356e6535a..ab59d51748d8fb 100644 --- a/tools/lkl/lib/virtio_net_dpdk.c +++ b/tools/lkl/lib/virtio_net_dpdk.c @@ -170,7 +170,7 @@ static int net_poll(struct lkl_netdev *nd) return LKL_DEV_NET_POLL_RX | LKL_DEV_NET_POLL_TX; } -static void net_close(struct lkl_netdev *nd) +static void net_poll_hup(struct lkl_netdev *nd) { struct lkl_netdev_dpdk *nd_dpdk = container_of(nd, struct lkl_netdev_dpdk, dev); @@ -178,11 +178,20 @@ static void net_close(struct lkl_netdev *nd) nd_dpdk->close = 1; } +static void net_free(struct lkl_netdev *nd) +{ + struct lkl_netdev_dpdk *nd_dpdk = + container_of(nd, struct lkl_netdev_dpdk, dev); + + free(nd_dpdk); +} + struct lkl_dev_net_ops dpdk_net_ops = { .tx = net_tx, .rx = net_rx, .poll = net_poll, - .close = net_close, + .poll_hup = net_poll_hup, + .free = net_free, }; diff --git a/tools/lkl/lib/virtio_net_fd.c b/tools/lkl/lib/virtio_net_fd.c index d48a6a70dfab59..0fcf6ae7742335 100644 --- a/tools/lkl/lib/virtio_net_fd.c +++ b/tools/lkl/lib/virtio_net_fd.c @@ -152,22 +152,31 @@ static int fd_net_poll(struct lkl_netdev *nd) return ret; } -static void fd_net_close(struct lkl_netdev *nd) +static void fd_net_poll_hup(struct lkl_netdev *nd) { struct lkl_netdev_fd *nd_fd = container_of(nd, struct lkl_netdev_fd, dev); - /* this will cause a POLLHUP in the poll function */ - close(nd_fd->pipe[1]); + /* this will cause a POLLHUP / POLLNVAL in the poll function */ close(nd_fd->pipe[0]); + close(nd_fd->pipe[1]); +} + +static void fd_net_free(struct lkl_netdev *nd) +{ + struct lkl_netdev_fd *nd_fd = + container_of(nd, struct lkl_netdev_fd, dev); + close(nd_fd->fd); + free(nd_fd); } struct lkl_dev_net_ops fd_net_ops = { .tx = fd_net_tx, .rx = fd_net_rx, .poll = fd_net_poll, - .close = fd_net_close, + .poll_hup = fd_net_poll_hup, + .free = fd_net_free, }; struct lkl_netdev *lkl_register_netdev_fd(int fd) @@ -186,7 +195,7 @@ struct lkl_netdev *lkl_register_netdev_fd(int fd) nd->fd = fd; if (pipe(nd->pipe) < 0) { perror("pipe"); - lkl_unregister_netdev_fd(&nd->dev); + free(nd); return NULL; } @@ -194,19 +203,10 @@ struct lkl_netdev *lkl_register_netdev_fd(int fd) perror("fnctl"); close(nd->pipe[0]); close(nd->pipe[1]); - lkl_unregister_netdev_fd(&nd->dev); + free(nd); return NULL; } nd->dev.ops = &fd_net_ops; return &nd->dev; } - -void lkl_unregister_netdev_fd(struct lkl_netdev *nd) -{ - struct lkl_netdev_fd *nd_fd = - container_of(nd, struct lkl_netdev_fd, dev); - - fd_net_close(nd); - free(nd_fd); -} diff --git a/tools/lkl/lib/virtio_net_fd.h b/tools/lkl/lib/virtio_net_fd.h index a4105e5563ecf9..5c1921b99a7921 100644 --- a/tools/lkl/lib/virtio_net_fd.h +++ b/tools/lkl/lib/virtio_net_fd.h @@ -13,14 +13,6 @@ struct ifreq; struct lkl_netdev *lkl_register_netdev_fd(int fd); -/** - * lkl_unregister_netdev_linux_fdnet - unregister a file descriptor-based - * network device as a NIC - * - * @nd - a struct lkl_netdev_linux_fdnet entry to be unregistered - */ -void lkl_unregister_netdev_fd(struct lkl_netdev *nd); - /** * lkl_netdev_tap_init - initialize tap related structure fot lkl_netdev. * diff --git a/tools/lkl/lib/virtio_net_vde.c b/tools/lkl/lib/virtio_net_vde.c index f41ad1d33c1992..95fadaa01950cb 100644 --- a/tools/lkl/lib/virtio_net_vde.c +++ b/tools/lkl/lib/virtio_net_vde.c @@ -22,13 +22,15 @@ static int net_vde_tx(struct lkl_netdev *nd, struct lkl_dev_buf *iov, int cnt); static int net_vde_rx(struct lkl_netdev *nd, struct lkl_dev_buf *iov, int cnt); static int net_vde_poll_with_timeout(struct lkl_netdev *nd, int timeout); static int net_vde_poll(struct lkl_netdev *nd); -static int net_vde_close(struct lkl_netdev *nd); +static void net_vde_poll_hup(struct lkl_netdev *nd); +static void net_vde_free(struct lkl_netdev *nd); struct lkl_dev_net_ops vde_net_ops = { .tx = net_vde_tx, .rx = net_vde_rx, .poll = net_vde_poll, - .close = net_vde_close, + .poll_hup = net_vde_poll_hup, + .free = net_vde_free, }; int net_vde_tx(struct lkl_netdev *nd, struct lkl_dev_buf *iov, int cnt) @@ -107,7 +109,7 @@ int net_vde_poll(struct lkl_netdev *nd) return net_vde_poll_with_timeout(nd, -1); } -void net_vde_close(struct lkl_netdev *nd) +void net_vde_poll_hup(struct lkl_netdev *nd) { struct lkl_netdev_vde *nd_vde = container_of(nd, struct lkl_netdev_vde, dev); @@ -115,6 +117,14 @@ void net_vde_close(struct lkl_netdev *nd) vde_close(nd_vde->conn); } +void net_vde_free(struct lkl_netdev *nd) +{ + struct lkl_netdev_vde *nd_vde = + container_of(nd, struct lkl_netdev_vde, dev); + + free(nd_vde); +} + struct lkl_netdev *lkl_netdev_vde_create(char const *switch_path) { struct lkl_netdev_vde *nd;