Pavel Šimerda <c...@simerda.eu> [2021-01-12 04:36:36]:
Hi,
The module requires libteam's teamd and teamdctl commands.
This looks like an alien to the OpenWrt ecosystem, basically you're using
netifd just as a launcher for teamd, teamdctl without any proper error
handling instead of ubus for configuration etc.
Signed-off-by: Pavel Šimerda <c...@simerda.eu>
---
CMakeLists.txt | 2 +-
team.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 179 insertions(+), 1 deletion(-)
create mode 100644 team.c
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9d19817..351e303 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -19,7 +19,7 @@ SET(SOURCES
main.c utils.c system.c tunnel.c handler.c
interface.c interface-ip.c interface-event.c
iprule.c proto.c proto-static.c proto-shell.c
- config.c device.c bridge.c veth.c vlan.c alias.c
+ config.c device.c bridge.c team.c veth.c vlan.c alias.c
macvlan.c ubus.c vlandev.c wireless.c)
diff --git a/team.c b/team.c
new file mode 100644
index 0000000..9b67566
--- /dev/null
+++ b/team.c
@@ -0,0 +1,178 @@
+/*
+ * netifd - network interface daemon
+ * Copyright (C) 2021 Pavel Šimerda <c...@simerda.eu>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <signal.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "netifd.h"
+#include "device.h"
+#include "interface.h"
+#include "system.h"
+
+enum {
+ TEAM_ATTR_IFNAME,
+ __TEAM_ATTR_MAX
+};
+
+static const struct blobmsg_policy team_attrs[__TEAM_ATTR_MAX] = {
+ [TEAM_ATTR_IFNAME] = { "ifname", BLOBMSG_TYPE_ARRAY },
+};
+
+static const struct uci_blob_param_info team_attr_info[__TEAM_ATTR_MAX] = {
+ [TEAM_ATTR_IFNAME] = { .type = BLOBMSG_TYPE_STRING },
+};
+
+static const struct uci_blob_param_list team_attr_list = {
+ .n_params = __TEAM_ATTR_MAX,
+ .params = team_attrs,
+ .info = team_attr_info,
+
+ .n_next = 1,
+ .next = { &device_attr_list },
+};
+
+struct team_device {
+ struct device dev;
+ device_state_cb set_state;
+
+ struct blob_attr *config_data;
+ struct blob_attr *ifnames;
+
+ int pid;
+};
+
+static int
+team_set_state(struct device *dev, bool up)
+{
+ struct team_device *teamdev = container_of(dev, struct team_device,
dev);
+
+ if (up) {
+ int pid;
+ struct blob_attr *cur;
+ int rem;
+ char buffer[64];
+
+ printf("TEAM start teamd\n");
if this is needed at all, take a look around and use proper debug logging
+ pid = fork();
+ if (pid == -1)
+ return -errno;
+ if (pid == 0)
+ execlp("teamd", "teamd", "-t", dev->ifname, NULL);
this is external dependency and you lack any check for that
+ teamdev->pid = pid;
+ // TODO: Better handling of newly created devices.
better? there is no handling of anything
+ sleep(1);
+ if (teamdev->ifnames) {
+ printf("TEAM port init\n");
+ blobmsg_for_each_attr(cur, teamdev->ifnames, rem) {
+ printf("TEAM one port init\n");
+ snprintf(buffer, sizeof buffer,
+ "teamdctl %s port add %s",
+ dev->ifname,
+ blobmsg_get_string(cur));
+ system(buffer);
puting aside usage of system() for service configuration this smells, you're
passing possibly malicious input directly to system() in the service running
as root, what could go wrong?
+ }
+ }
+ teamdev->set_state(dev, up);
+ return 0;
+ } else {
+ printf("TEAM: killing %d\n", teamdev->pid);
+ if (teamdev->pid) {
+ kill(teamdev->pid, SIGTERM);
+ waitpid(teamdev->pid, NULL, 0);
+ teamdev->pid = 0;
+ }
+ return 0;
+ }
+}
+
+static enum dev_change_type
+team_reload(struct device *dev, struct blob_attr *attr)
+{
+ struct team_device *teamdev = container_of(dev, struct team_device,
dev);
+ struct blob_attr *tb_tm[__TEAM_ATTR_MAX];
+
+ attr = blob_memdup(attr);
+
+ blobmsg_parse(team_attrs, __TEAM_ATTR_MAX, tb_tm, blob_data(attr),
blob_len(attr));
+ teamdev->ifnames = tb_tm[TEAM_ATTR_IFNAME];
+
+ if (teamdev->pid) {
+ // TODO: More fine-grained reconfiguration
+ team_set_state(dev, false);
+ team_set_state(dev, true);
+ }
+
+ free(teamdev->config_data);
+ teamdev->config_data = attr;
+ return DEV_CONFIG_APPLIED;
+}
+
+static struct device *
+team_create(const char *name, struct device_type *devtype,
+ struct blob_attr *attr)
+{
+ struct team_device *teamdev;
+ struct device *dev = NULL;
+
+ teamdev = calloc(1, sizeof(*teamdev));
+ if (!teamdev)
+ return NULL;
+ dev = &teamdev->dev;
+
+ if (device_init(dev, devtype, name) < 0) {
+ device_cleanup(dev);
+ free(teamdev);
+ return NULL;
+ }
+
+ teamdev->set_state = dev->set_state;
+ dev->set_state = team_set_state;
+
+ device_set_present(dev, true);
+ team_reload(dev, attr);
+
+ return dev;
+}
+
+static void
+team_free(struct device *dev)
+{
+ struct team_device *teamdev = container_of(dev, struct team_device,
dev);
+
+ free(teamdev->config_data);
+ free(teamdev);
+}
+
+static struct device_type team_device_type = {
+ .name = "team",
+ .config_params = &team_attr_list,
+
+ .bridge_capability = true,
+ .name_prefix = "tm",
+
+ .create = team_create,
+ .reload = team_reload,
+ .free = team_free,
+};
+
+static void __init team_device_type_init(void)
+{
+ device_type_add(&team_device_type);
+}
--
2.29.2
