-
Notifications
You must be signed in to change notification settings - Fork 20
Stability fixes #1574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Stability fixes #1574
Changes from all commits
8c001ac
122a59b
77ec084
dae75a9
f2f9e55
b9c2c7a
8533bde
2cb3d28
fac92bc
87a06f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # From GitHub release | ||
| sha256 9f194fa0b6e34fd915054394ef5b820a4f6b1755ace5ed1011bfba6df550accf mdns-alias-1.2.tar.gz | ||
| sha256 8186f0758f184cbdcab1033e4945117a587356c323e53bcdd19d47911ee2567b mdns-alias-1.3.tar.gz | ||
|
|
||
| # Locally generated | ||
| sha256 3d6f910b5e198f3daab48047b8ee6949040f7abee3927daf2e231f265faf7d91 LICENSE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| #set DEBUG=1 | ||
| service name:statd [12345] <pid/confd> statd -f -p /run/statd.pid -n -- Status daemon | ||
| service name:statd [12345] <pid/confd> statd -- Status daemon | ||
|
troglobit marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -507,13 +507,26 @@ static int eth_gen_del(struct lyd_node *dif, FILE *ip) | |
| return 0; | ||
| } | ||
|
|
||
| static int link_gen_del(struct lyd_node *dif, FILE *ip) | ||
| /* | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This we have talked multiple times about, i am fine with this solution, but i remember @wkz had some good examples why this was not good.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not exactly sure what example I was thinking about. My main concerns about this is (1) that we mask the sources of errors (i.e., if there is a problem in removing an interface, we won't see it until we try to configure the next instance of it - or worse: never); and (2) that we lull the user into thinking we support things (rolling back a partially applied config) that we do not (i.e., we might still be leaving addresses, routes, containers, etc. around from the previous config). Unless we have that sorted out through some other means? |
||
| * Tolerate the interface already being gone, e.g., leftover state from | ||
| * an earlier, partially applied generation -- a failed teardown would | ||
| * abort the whole generation. | ||
| */ | ||
| static int link_gen_del(struct dagger *net, struct lyd_node *dif) | ||
| { | ||
| fprintf(ip, "link del dev %s\n", lydx_get_cattr(dif, "name")); | ||
| const char *ifname = lydx_get_cattr(dif, "name"); | ||
| FILE *sh; | ||
|
|
||
| sh = dagger_fopen_net_exit(net, ifname, NETDAG_EXIT, "exit-del.sh"); | ||
| if (!sh) | ||
| return -EIO; | ||
|
|
||
| fprintf(sh, "ip link del dev %s 2>/dev/null || true\n", ifname); | ||
| fclose(sh); | ||
| return 0; | ||
| } | ||
|
|
||
| static int veth_gen_del(struct lyd_node *dif, FILE *sh) | ||
| static int veth_gen_del(struct dagger *net, struct lyd_node *dif) | ||
| { | ||
| if (!veth_is_primary(dif)) | ||
| return 0; | ||
|
|
@@ -526,7 +539,7 @@ static int veth_gen_del(struct lyd_node *dif, FILE *sh) | |
| if (lydx_get_child(dif, "container-network")) | ||
| return 0; | ||
|
|
||
| return link_gen_del(dif, sh); | ||
| return link_gen_del(net, dif); | ||
| } | ||
|
|
||
| static int netdag_gen_iface_del(struct dagger *net, struct lyd_node *dif, | ||
|
|
@@ -538,10 +551,6 @@ static int netdag_gen_iface_del(struct dagger *net, struct lyd_node *dif, | |
|
|
||
| DEBUG_IFACE(dif, ""); | ||
|
|
||
| ip = dagger_fopen_net_exit(net, ifname, NETDAG_EXIT, "exit.ip"); | ||
| if (!ip) | ||
| return -EIO; | ||
|
|
||
| type = iftype_from_iface(dif); | ||
| if (type == IFT_UNKNOWN) | ||
| /* The interface is still in running, so we need to | ||
|
|
@@ -554,11 +563,14 @@ static int netdag_gen_iface_del(struct dagger *net, struct lyd_node *dif, | |
| switch (type) { | ||
| case IFT_ETH: | ||
| case IFT_LO: | ||
| ip = dagger_fopen_net_exit(net, ifname, NETDAG_EXIT, "exit.ip"); | ||
| if (!ip) | ||
| return -EIO; | ||
| eth_gen_del(dif, ip); | ||
| fclose(ip); | ||
| break; | ||
| case IFT_VETH: | ||
| veth_gen_del(dif, ip); | ||
| break; | ||
| return veth_gen_del(net, dif); | ||
| case IFT_WIFI: | ||
| wifi_del_iface(dif, net); | ||
| break; | ||
|
|
@@ -571,11 +583,9 @@ static int netdag_gen_iface_del(struct dagger *net, struct lyd_node *dif, | |
| case IFT_VXLAN: | ||
| case IFT_WIREGUARD: | ||
| case IFT_UNKNOWN: | ||
| link_gen_del(dif, ip); | ||
| break; | ||
| return link_gen_del(net, dif); | ||
| } | ||
|
|
||
| fclose(ip); | ||
| return 0; | ||
| } | ||
|
|
||
|
|
@@ -617,6 +627,49 @@ static sr_error_t netdag_gen_iface_timeout(struct dagger *net, const char *ifnam | |
| return SR_ERR_OK; | ||
| } | ||
|
|
||
| /* | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to discuss this next week. What should happen when something fails, should we handle it or should it never fail? |
||
| * A netlink-created interface may linger from an earlier, partially | ||
| * applied generation, causing our `link add` to fail with EEXIST and | ||
| * abort the whole generation. Remove any leftover before creating. | ||
| */ | ||
| static int netdag_gen_ensure_absent(struct dagger *net, struct lyd_node *cif) | ||
| { | ||
| const char *ifname = lydx_get_cattr(cif, "name"); | ||
| const char *peer = NULL; | ||
| FILE *sh; | ||
|
|
||
| switch (iftype_from_iface(cif)) { | ||
| case IFT_BRIDGE: | ||
| case IFT_DUMMY: | ||
| case IFT_GRE: | ||
| case IFT_GRETAP: | ||
| case IFT_LAG: | ||
| case IFT_VLAN: | ||
| case IFT_VXLAN: | ||
| case IFT_WIREGUARD: | ||
| break; | ||
| case IFT_VETH: | ||
| /* primary's `link add` creates both ends */ | ||
| if (!veth_is_primary(cif)) | ||
| return 0; | ||
| peer = lydx_get_cattr(lydx_get_child(cif, "veth"), "peer"); | ||
| break; | ||
| default: | ||
| return 0; | ||
| } | ||
|
|
||
| sh = dagger_fopen_net_init(net, ifname, NETDAG_INIT_PRE, "ensure-absent.sh"); | ||
| if (!sh) | ||
| return -EIO; | ||
|
|
||
| fprintf(sh, "ip link del dev %s 2>/dev/null || true\n", ifname); | ||
| if (peer) | ||
| fprintf(sh, "ip link del dev %s 2>/dev/null || true\n", peer); | ||
| fclose(sh); | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| static sr_error_t netdag_gen_iface(sr_session_ctx_t *session, struct dagger *net, | ||
| struct lyd_node *dif, struct lyd_node *cif) | ||
| { | ||
|
|
@@ -683,7 +736,8 @@ static sr_error_t netdag_gen_iface(sr_session_ctx_t *session, struct dagger *net | |
| } | ||
|
|
||
| if (op == LYDX_OP_CREATE) { | ||
| err = netdag_gen_afspec_add(session, net, dif, cif, ip); | ||
| err = netdag_gen_ensure_absent(net, cif); | ||
| err = err ? : netdag_gen_afspec_add(session, net, dif, cif, ip); | ||
| if (err) | ||
| goto err_close_ip; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't a better solution to move the installation of the ttyd.conf to the ttyd package instead?