diff options
| -rw-r--r-- | .github/workflows/run-tests.yml | 20 | ||||
| -rw-r--r-- | accel-pppd/ctrl/ipoe/ipoe.h | 3 | ||||
| -rw-r--r-- | accel-pppd/ctrl/ipoe/ipoe_netlink.c | 124 | ||||
| -rw-r--r-- | accel-pppd/libnetlink/genl.c | 8 | ||||
| -rw-r--r-- | drivers/ipoe/ipoe.c | 175 | ||||
| -rw-r--r-- | drivers/ipoe/ipoe.h | 1 | ||||
| -rw-r--r-- | tests/accel-pppd/ipoe/test_ipoe_link_ops.py | 82 | ||||
| -rw-r--r-- | tests/accel-pppd/ipoe/test_ipoe_stale_cleanup.py | 97 | ||||
| -rw-r--r-- | tests/common/ipoe_iface.py | 84 |
9 files changed, 572 insertions, 22 deletions
diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 9cd47a00..7ce55b6e 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -145,6 +145,7 @@ jobs: sudo dmesg" - name: Insert ipoe kernel module + if: ${{ always() }} run: > ssh -i ssh-key -p2222 user@localhost "cd accel-ppp && sudo insmod build/drivers/ipoe/driver/ipoe.ko && @@ -163,12 +164,14 @@ jobs: sudo dmesg" - name: Insert vlan_mon kernel module + if: ${{ always() }} run: > ssh -i ssh-key -p2222 user@localhost "cd accel-ppp && sudo insmod build/drivers/vlan_mon/driver/vlan_mon.ko && lsmod | grep vlan_mon" - name: Insert ppposeq kernel module + if: ${{ always() }} run: > ssh -i ssh-key -p2222 user@localhost "cd accel-ppp && sudo modprobe pppox && sudo insmod build/drivers/ppposeq/driver/ppposeq.ko && @@ -290,6 +293,7 @@ jobs: doas dmesg" - name: Insert ipoe kernel module + if: ${{ always() }} run: > ssh -i ssh-key -p2222 alpine@localhost "cd accel-ppp && doas insmod build/drivers/ipoe/driver/ipoe.ko && @@ -308,12 +312,14 @@ jobs: doas dmesg" - name: Insert vlan_mon kernel module + if: ${{ always() }} run: > ssh -i ssh-key -p2222 alpine@localhost "cd accel-ppp && doas insmod build/drivers/vlan_mon/driver/vlan_mon.ko && lsmod | grep vlan_mon" - name: Insert ppposeq kernel module + if: ${{ always() }} run: > ssh -i ssh-key -p2222 alpine@localhost "cd accel-ppp && doas modprobe pppox && doas insmod build/drivers/ppposeq/driver/ppposeq.ko && @@ -478,9 +484,17 @@ jobs: - name: Generate coverage reports (default(txt), csv, html) run: | mkdir -p tests/report - gcovr --config=tests/gcovr.conf --output=tests/report/accel-ppp.txt - gcovr --config=tests/gcovr.conf --csv --output=tests/report/accel-ppp.csv - gcovr --config=tests/gcovr.conf --html --html-details --output=tests/report/accel-ppp.html + # gcov reports negative branch counters now and then (gcc bug 68080). + # gcovr 6 and later stop with a parse error unless told to carry on, + # older ones do not know the option at all, so ask before using it. + IGNORE="" + if gcovr --help 2>&1 | grep -q negative_hits; then + IGNORE="--gcov-ignore-parse-errors=negative_hits.warn_once_per_file" + fi + echo "gcovr `gcovr --version | head -1`, extra options: '$IGNORE'" + gcovr --config=tests/gcovr.conf $IGNORE --output=tests/report/accel-ppp.txt + gcovr --config=tests/gcovr.conf $IGNORE --csv --output=tests/report/accel-ppp.csv + gcovr --config=tests/gcovr.conf $IGNORE --html --html-details --output=tests/report/accel-ppp.html - name: Show default coverage report run: cat tests/report/accel-ppp.txt diff --git a/accel-pppd/ctrl/ipoe/ipoe.h b/accel-pppd/ctrl/ipoe/ipoe.h index 37c26a63..2bd3a14f 100644 --- a/accel-pppd/ctrl/ipoe/ipoe.h +++ b/accel-pppd/ctrl/ipoe/ipoe.h @@ -164,7 +164,8 @@ void ipoe_nl_delete_interfaces(void); int ipoe_nl_create(); void ipoe_nl_delete(int ifindex); int ipoe_nl_modify(int ifindex, uint32_t peer_addr, uint32_t addr, uint32_t gw, int link_ifindex, uint8_t *hwaddr); -void ipoe_nl_get_sessions(struct list_head *list); +int ipoe_nl_get_sessions(struct list_head *list); +int ipoe_nl_flush_sessions(void); int ipoe_nl_add_exclude(uint32_t addr, int mask); void ipoe_nl_del_exclude(uint32_t addr); int ipoe_nl_add_net(uint32_t addr, int mask); diff --git a/accel-pppd/ctrl/ipoe/ipoe_netlink.c b/accel-pppd/ctrl/ipoe/ipoe_netlink.c index 7057c91e..df4dec67 100644 --- a/accel-pppd/ctrl/ipoe/ipoe_netlink.c +++ b/accel-pppd/ctrl/ipoe/ipoe_netlink.c @@ -46,6 +46,8 @@ int ipoe_nl_add_exclude(uint32_t addr, int mask) return -1; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -82,6 +84,8 @@ void ipoe_nl_del_exclude(uint32_t addr) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -114,6 +118,8 @@ int ipoe_nl_add_net(uint32_t addr, int mask) return -1; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -150,6 +156,8 @@ void ipoe_nl_del_net(uint32_t addr) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -181,6 +189,8 @@ void ipoe_nl_add_interface(int ifindex, uint8_t mode) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -213,6 +223,8 @@ void ipoe_nl_del_interface(int ifindex) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -253,6 +265,8 @@ int ipoe_nl_create() return -1; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -317,6 +331,8 @@ int ipoe_nl_modify(int ifindex, uint32_t peer_addr, uint32_t addr, uint32_t gw, return -1; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -394,17 +410,25 @@ static int dump_session(const struct sockaddr_nl *addr, struct nlmsghdr *n, void return 0; } -void ipoe_nl_get_sessions(struct list_head *list) +int ipoe_nl_get_sessions(struct list_head *list) { + struct rtnl_handle rth; struct nlmsghdr *nlh; struct genlmsghdr *ghdr; struct { struct nlmsghdr n; char buf[1024]; } req; + int ret; - if (rth.fd == -1) - return; + /* a private socket, so that the dump does not have to compete with + * the packet notifications delivered to the multicast one */ + if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC)) { + log_error("ipoe: cannot open generic netlink socket\n"); + return -1; + } + + memset(&req, 0, sizeof(req)); nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); @@ -416,11 +440,51 @@ void ipoe_nl_get_sessions(struct list_head *list) ghdr->cmd = IPOE_CMD_GET; if (rtnl_send(&rth, (char *)nlh, nlh->nlmsg_len) < 0) { - log_emerg("ipoe: failed to send dump request: %s\n", strerror(errno)); - return; + log_error("ipoe: failed to send dump request: %s\n", strerror(errno)); + rtnl_close(&rth); + return -1; } - rtnl_dump_filter(&rth, dump_session, list, NULL, NULL); + ret = rtnl_dump_filter(&rth, dump_session, list, NULL, NULL); + + rtnl_close(&rth); + + return ret; +} + +int ipoe_nl_flush_sessions(void) +{ + struct rtnl_handle rth; + struct nlmsghdr *nlh; + struct genlmsghdr *ghdr; + struct { + struct nlmsghdr n; + char buf[128]; + } req; + int ret = 0; + + if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC)) { + log_error("ipoe: cannot open generic netlink socket\n"); + return -EIO; + } + + memset(&req, 0, sizeof(req)); + + nlh = &req.n; + nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); + nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; + nlh->nlmsg_type = ipoe_genl_id; + + ghdr = NLMSG_DATA(&req.n); + ghdr->cmd = IPOE_CMD_FLUSH; + + errno = 0; + if (rtnl_talk(&rth, nlh, 0, 0, nlh, NULL, NULL, 0) < 0) + ret = errno ? -errno : -EIO; + + rtnl_close(&rth); + + return ret; } void ipoe_nl_delete(int ifindex) @@ -438,6 +502,8 @@ void ipoe_nl_delete(int ifindex) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -459,7 +525,10 @@ static void delete_sessions() struct ipoe_session_info *info; LIST_HEAD(ds_list); - ipoe_nl_get_sessions(&ds_list); + + if (ipoe_nl_get_sessions(&ds_list)) + log_error("ipoe: failed to enumerate sessions left by a previous" + " instance, some of them are not removed\n"); while (!list_empty(&ds_list)) { info = list_entry(ds_list.next, typeof(*info), entry); @@ -469,6 +538,24 @@ static void delete_sessions() } } +static void flush_sessions() +{ + int r = ipoe_nl_flush_sessions(); + + if (!r) + return; + + if (r == -EOPNOTSUPP) { + log_warn("ipoe: loaded ipoe module does not support IPOE_CMD_FLUSH," + " removing sessions one by one, reload the module to fix\n"); + delete_sessions(); + return; + } + + log_error("ipoe: failed to remove sessions left by a previous instance:" + " %s\n", strerror(-r)); +} + static void ipoe_up_handler(const struct sockaddr_nl *addr, struct nlmsghdr *h) { struct rtattr *tb[PKT_ATTR_MAX + 1]; @@ -628,23 +715,32 @@ static void init(void) log_warn("failed to load ipoe module\n"); mcg_id = genl_resolve_mcg(IPOE_GENL_NAME, IPOE_GENL_MCG_PKT, &ipoe_genl_id); + + if (!ipoe_genl_id) { + log_error("ipoe: cannot resolve netlink family, state left by a" + " previous instance is not removed\n"); + return; + } + + /* Drop everything a previous instance may have left in the kernel. + * The interfaces go first: while their rx handlers are still attached + * the module keeps reporting unclassified packets, and once we join + * the multicast group that traffic competes with our own replies. */ + ipoe_nl_delete_interfaces(); + flush_sessions(); + ipoe_nl_del_exclude(0); + ipoe_nl_del_net(0); + if (mcg_id == -1) { log_warn("ipoe: unclassified packet handling is disabled\n"); - rth.fd = -1; return; } if (rtnl_open_byproto(&rth, 1 << (mcg_id - 1), NETLINK_GENERIC)) { log_error("ipoe: cannot open generic netlink socket\n"); - rth.fd = -1; return; } - delete_sessions(); - ipoe_nl_del_exclude(0); - ipoe_nl_del_net(0); - ipoe_nl_delete_interfaces(); - fcntl(rth.fd, F_SETFL, O_NONBLOCK); fcntl(rth.fd, F_SETFD, fcntl(rth.fd, F_GETFD) | FD_CLOEXEC); diff --git a/accel-pppd/libnetlink/genl.c b/accel-pppd/libnetlink/genl.c index 0c758077..8cdfce89 100644 --- a/accel-pppd/libnetlink/genl.c +++ b/accel-pppd/libnetlink/genl.c @@ -82,12 +82,14 @@ int __export genl_resolve_mcg(const char *family, const char *name, int *fam_id) goto out; } - if (!tb[CTRL_ATTR_MCAST_GROUPS]) - goto out; - + /* report the family id even if the group lookup below fails, the + * caller may still have use for it */ if (fam_id) *fam_id = *(uint16_t *)(RTA_DATA(tb[CTRL_ATTR_FAMILY_ID])); + if (!tb[CTRL_ATTR_MCAST_GROUPS]) + goto out; + parse_rtattr_nested(tb2, GENL_MAX_FAM_GRPS, tb[CTRL_ATTR_MCAST_GROUPS]); for (i = 1; i < GENL_MAX_FAM_GRPS; i++) { diff --git a/drivers/ipoe/ipoe.c b/drivers/ipoe/ipoe.c index 2829e6ac..251fec96 100644 --- a/drivers/ipoe/ipoe.c +++ b/drivers/ipoe/ipoe.c @@ -22,6 +22,7 @@ #include <linux/version.h> #include <net/genetlink.h> +#include <net/rtnetlink.h> #include <net/route.h> #include <net/sock.h> #include <net/ip.h> @@ -104,6 +105,10 @@ struct ipoe_session { atomic_t refs; + /* set under ipoe_wlock by whoever starts tearing the session down, + * so that the genl and the rtnetlink path do not do it twice */ + unsigned int dying:1; + struct ipoe_stats __percpu *rx_stats; struct ipoe_stats __percpu *tx_stats; }; @@ -1116,6 +1121,83 @@ static const struct header_ops ipoe_hard_header_ops = { .cache_update = eth_header_cache_update, }; +/* Is anybody subscribed to our packet group, i.e. is a control daemon + * running at all? */ +static int ipoe_ctrl_attached(void) +{ +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0) && RHEL_MAJOR < 7 + return netlink_has_listeners(init_net.genl_sock, ipoe_nl_mcg.id); +#else + return genl_has_listeners(&ipoe_nl_family, &init_net, 0); +#endif +} + +/* Sessions are created through IPOE_CMD_CREATE, which also sets up the + * private state. A device made by rtnetlink would have none of it, so + * refuse 'ip link add ... type ipoe' explicitly. */ +#if LINUX_VERSION_CODE >= KERNEL_VERSION(6,13,0) +static int ipoe_newlink(struct net_device *dev, + struct rtnl_newlink_params *params, + struct netlink_ext_ack *extack) +#elif LINUX_VERSION_CODE >= KERNEL_VERSION(4,13,0) +static int ipoe_newlink(struct net *src_net, struct net_device *dev, + struct nlattr *tb[], struct nlattr *data[], + struct netlink_ext_ack *extack) +#else +static int ipoe_newlink(struct net *src_net, struct net_device *dev, + struct nlattr *tb[], struct nlattr *data[]) +#endif +{ + return -EOPNOTSUPP; +} + +/* Called by rtnetlink with rtnl held, so the teardown ipoe_nl_cmd_delete() + * does after dropping ipoe_wlock has to happen here as well. */ +static void ipoe_dellink(struct net_device *dev, struct list_head *head) +{ + struct ipoe_session *ses = netdev_priv(dev); + + down(&ipoe_wlock); + + if (ses->dying) { + /* the genl path is already removing it, it will unregister + * the device itself once it gets rtnl */ + up(&ipoe_wlock); + return; + } + + ses->dying = 1; + + if (ses->peer_addr) + list_del_rcu(&ses->entry); + list_del(&ses->entry2); + if (ses->u.hwaddr_u) + list_del_rcu(&ses->entry3); + + up(&ipoe_wlock); + + /* A session left behind by a dead daemon looks exactly like one that + * is still in use, so removing it can not be refused on state alone. + * Complain only if somebody is still subscribed to our multicast + * group, which means a daemon is around to be surprised by it. */ + if (ses->peer_addr && ipoe_ctrl_attached()) + pr_warn("ipoe: %s: removed through rtnetlink while bound to %pI4" + " and a control daemon is attached\n", + dev->name, &ses->peer_addr); + + synchronize_rcu(); + + while (atomic_read(&ses->refs)) + schedule_timeout_uninterruptible(1); + + if (ses->link_dev) { + dev_put(ses->link_dev); + ses->link_dev = NULL; + } + + unregister_netdevice_queue(dev, head); +} + static void ipoe_netdev_setup(struct net_device *dev) { dev->netdev_ops = &ipoe_netdev_ops; @@ -1145,6 +1227,14 @@ static void ipoe_netdev_setup(struct net_device *dev) dev->priv_flags &= ~IFF_XMIT_DST_RELEASE; } +static struct rtnl_link_ops ipoe_link_ops __read_mostly = { + .kind = "ipoe", + .priv_size = sizeof(struct ipoe_session), + .setup = ipoe_netdev_setup, + .newlink = ipoe_newlink, + .dellink = ipoe_dellink, +}; + static int ipoe_create(__be32 peer_addr, __be32 addr, __be32 gw, int ifindex, const __u8 *hwaddr) { struct ipoe_session *ses; @@ -1220,6 +1310,7 @@ static int ipoe_create(__be32 peer_addr, __be32 addr, __be32 gw, int ifindex, co }*/ dev->tx_queue_len = 100; + dev->rtnl_link_ops = &ipoe_link_ops; rtnl_lock(); r = register_netdevice(dev); @@ -1393,12 +1484,22 @@ static int ipoe_nl_cmd_delete(struct sk_buff *skb, struct genl_info *info) //pr_info("ipoe: delete %08x\n", ses->peer_addr); + if (ses->dying) { + /* already on its way out through ipoe_dellink() or a flush */ + ret = 0; + goto out_unlock; + } + + ses->dying = 1; + if (ses->peer_addr) list_del_rcu(&ses->entry); list_del(&ses->entry2); if (ses->u.hwaddr_u) list_del_rcu(&ses->entry3); + /* drop the lock before sleeping in synchronize_rcu() and taking rtnl + * in unregister_netdev() */ up(&ipoe_wlock); synchronize_rcu(); @@ -1411,13 +1512,61 @@ static int ipoe_nl_cmd_delete(struct sk_buff *skb, struct genl_info *info) unregister_netdev(ses->dev); - ret = 0; + return 0; out_unlock: up(&ipoe_wlock); return ret; } +static int ipoe_nl_cmd_flush(struct sk_buff *skb, struct genl_info *info) +{ + struct ipoe_session *ses; + LIST_HEAD(list); + LIST_HEAD(kill_list); + + down(&ipoe_wlock); + + list_splice_init(&ipoe_list2, &list); + + list_for_each_entry(ses, &list, entry2) { + ses->dying = 1; + + if (ses->peer_addr) + list_del_rcu(&ses->entry); + if (ses->u.hwaddr_u) + list_del_rcu(&ses->entry3); + } + + up(&ipoe_wlock); + + if (list_empty(&list)) + return 0; + + /* a single grace period covers the whole batch */ + synchronize_rcu(); + + list_for_each_entry(ses, &list, entry2) { + while (atomic_read(&ses->refs)) + schedule_timeout_uninterruptible(1); + + if (ses->link_dev) { + dev_put(ses->link_dev); + ses->link_dev = NULL; + } + } + + rtnl_lock(); + list_for_each_entry(ses, &list, entry2) + unregister_netdevice_queue(ses->dev, &kill_list); + unregister_netdevice_many(&kill_list); + rtnl_unlock(); + + /* the sessions are freed by now, do not touch 'list' again */ + + return 0; +} + static int ipoe_nl_cmd_modify(struct sk_buff *skb, struct genl_info *info) { int ret = -EINVAL, r = 0; @@ -1577,7 +1726,11 @@ static int ipoe_nl_cmd_dump_sessions(struct sk_buff *skb, struct netlink_callbac #else if (fill_info(skb, ses, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq) < 0) #endif + { + /* this one did not fit, resume from it next time */ + idx--; break; + } } up(&ipoe_wlock); @@ -1890,6 +2043,14 @@ static const struct genl_ops ipoe_nl_ops[] = { .policy = ipoe_nl_policy, #endif }, + { + .cmd = IPOE_CMD_FLUSH, + .doit = ipoe_nl_cmd_flush, + .flags = GENL_ADMIN_PERM, +#if LINUX_VERSION_CODE < KERNEL_VERSION(5,2,0) + .policy = ipoe_nl_policy, +#endif + }, }; #if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0) && RHEL_MAJOR < 7 @@ -1960,6 +2121,12 @@ static int __init ipoe_init(void) skb_queue_head_init(&ipoe_queue); INIT_WORK(&ipoe_queue_work, ipoe_process_queue); + err = rtnl_link_register(&ipoe_link_ops); + if (err < 0) { + printk(KERN_INFO "ipoe: can't register link operations\n"); + return err; + } + #if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0) && RHEL_MAJOR < 7 err = genl_register_family_with_ops(&ipoe_nl_family, ipoe_nl_ops, ARRAY_SIZE(ipoe_nl_ops)); #elif LINUX_VERSION_CODE < KERNEL_VERSION(4,10,0) @@ -1969,6 +2136,7 @@ static int __init ipoe_init(void) #endif if (err < 0) { printk(KERN_INFO "ipoe: can't register netlink interface\n"); + rtnl_link_unregister(&ipoe_link_ops); return err; } @@ -1977,6 +2145,7 @@ static int __init ipoe_init(void) if (err < 0) { printk(KERN_INFO "ipoe: can't register netlink multicast group\n"); genl_unregister_family(&ipoe_nl_family); + rtnl_link_unregister(&ipoe_link_ops); return err; } #endif @@ -1996,6 +2165,10 @@ static void __exit ipoe_fini(void) #endif genl_unregister_family(&ipoe_nl_family); + /* takes down whatever sessions are left through ipoe_dellink() and + * keeps rtnetlink from starting another one behind our back */ + rtnl_link_unregister(&ipoe_link_ops); + down(&ipoe_wlock); up(&ipoe_wlock); diff --git a/drivers/ipoe/ipoe.h b/drivers/ipoe/ipoe.h index 4097e2da..2d041c50 100644 --- a/drivers/ipoe/ipoe.h +++ b/drivers/ipoe/ipoe.h @@ -16,6 +16,7 @@ enum { IPOE_CMD_DEL_EXCLUDE, IPOE_CMD_ADD_NET, IPOE_CMD_DEL_NET, + IPOE_CMD_FLUSH, __IPOE_CMD_MAX, }; diff --git a/tests/accel-pppd/ipoe/test_ipoe_link_ops.py b/tests/accel-pppd/ipoe/test_ipoe_link_ops.py new file mode 100644 index 00000000..00d06af4 --- /dev/null +++ b/tests/accel-pppd/ipoe/test_ipoe_link_ops.py @@ -0,0 +1,82 @@ +import pytest +from common import process, ipoe_iface + + +@pytest.fixture() +def accel_pppd_config(veth_pair_netns): + return ( + """ + [modules] + connlimit + radius + ipoe + ippool + + [ip-pool] + gw-ip-address=192.0.2.1 + 192.0.2.2-255 + + [cli] + tcp=127.0.0.1:2001 + + [core] + log-error=/dev/stderr + + [log] + log-debug=/dev/stdout + log-file=/dev/stdout + log-emerg=/dev/stderr + level=5 + + [radius] + + [ipoe] + noauth=1 + shared=1 + gw-ip-address=192.0.2.1/24 + interface=re:.""" + + veth_pair_netns["veth_a"][1:] + ) + + +# sessions carry private state set up over generic netlink, a device made by +# rtnetlink would have none of it +@pytest.mark.dependency(depends=["ipoe_driver_loaded"], scope="session") +@pytest.mark.ipoe_driver +def test_ipoe_interface_cannot_be_created_by_iproute(): + (exit_code, out, err) = process.run( + ["ip", "link", "add", "ipoetest0", "type", "ipoe"] + ) + print("ip link add: exit=%d out=%s err=%s" % (exit_code, out, err)) + + # remove it again in case it got created anyway, so that the rest of the + # suite is not affected + process.run(["ip", "link", "del", "ipoetest0"]) + + assert exit_code != 0 + + +# stale interfaces have to be removable without restarting accel-pppd, which +# would drop every remaining session +@pytest.mark.dependency(depends=["ipoe_driver_loaded"], scope="session") +@pytest.mark.ipoe_driver +def test_ipoe_interface_can_be_deleted_by_iproute( + accel_pppd_instance, dhclient_instance, accel_cmd, veth_pair_netns +): + assert accel_pppd_instance, "accel-pppd did not start" + assert dhclient_instance["is_started"] + assert ipoe_iface.wait_for_session(accel_cmd, veth_pair_netns["veth_a"]) + + before = ipoe_iface.list_ipoe() + print("ipoe interfaces: " + str(before)) + assert len(before) > 0 + + (ifindex, name) = before[0] + + (exit_code, out, err) = process.run(["ip", "link", "del", name]) + print("ip link del %s: exit=%d out=%s err=%s" % (name, exit_code, out, err)) + assert exit_code == 0 + + after = ipoe_iface.list_ipoe() + print("ipoe interfaces after delete: " + str(after)) + assert (ifindex, name) not in after diff --git a/tests/accel-pppd/ipoe/test_ipoe_stale_cleanup.py b/tests/accel-pppd/ipoe/test_ipoe_stale_cleanup.py new file mode 100644 index 00000000..b7cfd533 --- /dev/null +++ b/tests/accel-pppd/ipoe/test_ipoe_stale_cleanup.py @@ -0,0 +1,97 @@ +import pytest +from common import accel_pppd_process, ipoe_iface + + +@pytest.fixture() +def accel_pppd_config(veth_pair_netns): + return ( + """ + [modules] + connlimit + radius + ipoe + ippool + + [ip-pool] + gw-ip-address=192.0.2.1 + 192.0.2.2-255 + + [cli] + tcp=127.0.0.1:2001 + + [core] + log-error=/dev/stderr + + [log] + log-debug=/dev/stdout + log-file=/dev/stdout + log-emerg=/dev/stderr + level=5 + + [radius] + + [ipoe] + noauth=1 + shared=1 + gw-ip-address=192.0.2.1/24 + interface=re:.""" + + veth_pair_netns["veth_a"][1:] + ) + + +# accel-pppd killed with SIGKILL has no chance to remove the session +# interfaces it created, so they are left in the kernel. The next instance is +# supposed to drop them while starting up. +@pytest.mark.dependency(depends=["ipoe_driver_loaded"], scope="session") +@pytest.mark.ipoe_driver +def test_ipoe_stale_interfaces_removed_after_sigkill( + accel_pppd_instance, + dhclient_instance, + accel_cmd, + accel_pppd, + accel_pppd_config_file, + veth_pair_netns, + pytestconfig, +): + assert accel_pppd_instance, "accel-pppd did not start" + assert dhclient_instance["is_started"] + assert ipoe_iface.wait_for_session(accel_cmd, veth_pair_netns["veth_a"]) + + # the session must have created an interface, otherwise there is nothing + # for this test to check + before = ipoe_iface.list_ipoe() + print("ipoe interfaces before the crash: " + str(before)) + assert len(before) > 0 + + assert ipoe_iface.kill_accel_pppd() > 0 + + # nothing removes them while no accel-pppd is running (not asserted, the + # kernel module is free to start doing it on its own one day) + print("ipoe interfaces after the crash: " + str(ipoe_iface.list_ipoe())) + + # start again, it replies to 'show version' only once the startup flush is + # done, so there is no need to wait for anything else + (is_started, thread, control) = accel_pppd_process.start( + accel_pppd, + ["-c" + accel_pppd_config_file], + accel_cmd, + pytestconfig.getoption("accel_pppd_max_wait_time"), + ) + + try: + assert is_started + + after = ipoe_iface.list_ipoe() + print("ipoe interfaces after the restart: " + str(after)) + + # compared by ifindex: dhclient may get a new session in the meantime, + # and the fresh interface would reuse the ipoe0 name + stale = set(before) & set(after) + assert not stale, "interfaces left over from the killed instance: " + str(stale) + finally: + accel_pppd_process.end( + thread, + control, + accel_cmd, + pytestconfig.getoption("accel_pppd_max_finish_time"), + ) diff --git a/tests/common/ipoe_iface.py b/tests/common/ipoe_iface.py new file mode 100644 index 00000000..62b26355 --- /dev/null +++ b/tests/common/ipoe_iface.py @@ -0,0 +1,84 @@ +from common import process +import os +import signal +import time + + +# (ifindex, name) of every ipoe session interface currently in the kernel. +# Matched by name rather than by 'ip link show type ipoe' so that the helper +# keeps working with a module that does not register rtnl_link_ops. +def list_ipoe(): + (exit_code, out, err) = process.run(["ip", "-o", "link", "show"]) + assert exit_code == 0, "ip link show failed: " + err + + ifaces = [] + for line in out.splitlines(): + fields = line.split(":") + if len(fields) < 2: + continue + try: + ifindex = int(fields[0].strip()) + except ValueError: + continue + name = fields[1].strip().split("@")[0] + if name.startswith("ipoe"): + ifaces.append((ifindex, name)) + + return ifaces + + +# pids of the running accel-pppd processes +def accel_pppd_pids(): + pids = [] + for entry in os.listdir("/proc"): + if not entry.isdigit(): + continue + try: + with open("/proc/" + entry + "/comm") as comm: + if comm.read().strip() == "accel-pppd": + pids.append(int(entry)) + except OSError: # process is gone, or not ours to look at + pass + + return pids + + +# SIGKILL every accel-pppd and wait until they are really gone. +# Returns the number of processes that were killed. +def kill_accel_pppd(max_wait_time=10.0): + pids = accel_pppd_pids() + for pid in pids: + print("kill_accel_pppd: SIGKILL to pid " + str(pid)) + try: + os.kill(pid, signal.SIGKILL) + except OSError: + pass + + sleep_time = 0.0 + while sleep_time < max_wait_time: + if not accel_pppd_pids(): + print("kill_accel_pppd: gone in (sec): " + str(sleep_time)) + break + time.sleep(0.1) + sleep_time += 0.1 + + return len(pids) + + +# wait until accel-pppd reports an active ipoe session on the given interface +def wait_for_session(accel_cmd, called_sid, max_wait_time=10.0): + sleep_time = 0.0 + out = "" + while sleep_time < max_wait_time: + (exit_code, out, err) = process.run( + [accel_cmd, "show sessions called-sid,ip,state"] + ) + assert exit_code == 0, "accel-cmd failed: " + err + if called_sid in out and "192.0.2." in out and "active" in out: + print("wait_for_session: session found in (sec): " + str(sleep_time)) + return True + time.sleep(0.1) + sleep_time += 0.1 + + print("wait_for_session: last accel-cmd out: " + out) + return False |