lldp: add proper ref counting to sd_lldp object and a separate call for setting the ifindex

Let's make sd-lldp a bit more like sd-ndisc ant the other APIs, and add proper
ref counting and a separate call for setting the ifindex.

This also adds a new lldp_reset() call we can use at various places to close
all fds. This is also similar to how sd-ndisc already does it.

5 files changed, 70 insertions, 31 deletions

diff --git a/src/libsystemd-network/lldp-internal.h b/src/libsystemd-network/lldp-internal.h
index 7592bc4305..becc162fab 100644
--- a/src/libsystemd-network/lldp-internal.h
+++ b/src/libsystemd-network/lldp-internal.h
@@ -28,6 +28,8 @@
 #include "prioq.h"
 
 struct sd_lldp {
+        unsigned n_ref;
+
         int ifindex;
         int fd;
 
diff --git a/src/libsystemd-network/sd-lldp.c b/src/libsystemd-network/sd-lldp.c
index 223a5ac02f..9e9907ca37 100644
--- a/src/libsystemd-network/sd-lldp.c
+++ b/src/libsystemd-network/sd-lldp.c
@@ -234,10 +234,20 @@ static int lldp_receive_datagram(sd_event_source *s, int fd, uint32_t revents, v
         return lldp_handle_datagram(lldp, n);
 }
 
+static void lldp_reset(sd_lldp *lldp) {
+        assert(lldp);
+
+        lldp->timer_event_source = sd_event_source_unref(lldp->timer_event_source);
+        lldp->io_event_source = sd_event_source_unref(lldp->io_event_source);
+        lldp->fd = safe_close(lldp->fd);
+}
+
 _public_ int sd_lldp_start(sd_lldp *lldp) {
         int r;
 
         assert_return(lldp, -EINVAL);
+        assert_return(lldp->event, -EINVAL);
+        assert_return(lldp->ifindex > 0, -EINVAL);
 
         if (lldp->fd >= 0)
                 return 0;
@@ -248,24 +258,21 @@ _public_ int sd_lldp_start(sd_lldp *lldp) {
         if (lldp->fd < 0)
                 return lldp->fd;
 
-        if (lldp->event) {
-                r = sd_event_add_io(lldp->event, &lldp->io_event_source, lldp->fd, EPOLLIN, lldp_receive_datagram, lldp);
-                if (r < 0)
-                        goto fail;
+        r = sd_event_add_io(lldp->event, &lldp->io_event_source, lldp->fd, EPOLLIN, lldp_receive_datagram, lldp);
+        if (r < 0)
+                goto fail;
 
-                r = sd_event_source_set_priority(lldp->io_event_source, lldp->event_priority);
-                if (r < 0)
-                        goto fail;
+        r = sd_event_source_set_priority(lldp->io_event_source, lldp->event_priority);
+        if (r < 0)
+                goto fail;
 
-                (void) sd_event_source_set_description(lldp->io_event_source, "lldp-io");
-        }
+        (void) sd_event_source_set_description(lldp->io_event_source, "lldp-io");
 
+        log_lldp("Started LLDP client");
         return 1;
 
 fail:
-        lldp->io_event_source = sd_event_source_unref(lldp->io_event_source);
-        lldp->fd = safe_close(lldp->fd);
-
+        lldp_reset(lldp);
         return r;
 }
 
@@ -275,10 +282,9 @@ _public_ int sd_lldp_stop(sd_lldp *lldp) {
         if (lldp->fd < 0)
                 return 0;
 
-        lldp->timer_event_source = sd_event_source_unref(lldp->timer_event_source);
-        lldp->io_event_source = sd_event_source_unref(lldp->io_event_source);
-        lldp->fd = safe_close(lldp->fd);
+        log_lldp("Stopping LLDP client");
 
+        lldp_reset(lldp);
         lldp_flush_neighbors(lldp);
 
         return 1;
@@ -322,39 +328,60 @@ _public_ int sd_lldp_set_callback(sd_lldp *lldp, sd_lldp_callback_t cb, void *us
         return 0;
 }
 
+_public_ int sd_lldp_set_ifindex(sd_lldp *lldp, int ifindex) {
+        assert_return(lldp, -EINVAL);
+        assert_return(ifindex > 0, -EINVAL);
+        assert_return(lldp->fd < 0, -EBUSY);
+
+        lldp->ifindex = ifindex;
+        return 0;
+}
+
+_public_ sd_lldp* sd_lldp_ref(sd_lldp *lldp) {
+
+        if (!lldp)
+                return NULL;
+
+        assert(lldp->n_ref > 0);
+        lldp->n_ref++;
+
+        return lldp;
+}
+
 _public_ sd_lldp* sd_lldp_unref(sd_lldp *lldp) {
 
         if (!lldp)
                 return NULL;
 
+        assert(lldp->n_ref > 0);
+        lldp->n_ref --;
+
+        if (lldp->n_ref > 0)
+                return NULL;
+
+        lldp_reset(lldp);
+        sd_lldp_detach_event(lldp);
         lldp_flush_neighbors(lldp);
 
         hashmap_free(lldp->neighbor_by_id);
         prioq_free(lldp->neighbor_by_expiry);
-
-        sd_event_source_unref(lldp->io_event_source);
-        sd_event_source_unref(lldp->timer_event_source);
-        sd_event_unref(lldp->event);
-        safe_close(lldp->fd);
-
         free(lldp);
 
         return NULL;
 }
 
-_public_ int sd_lldp_new(sd_lldp **ret, int ifindex) {
+_public_ int sd_lldp_new(sd_lldp **ret) {
         _cleanup_(sd_lldp_unrefp) sd_lldp *lldp = NULL;
         int r;
 
         assert_return(ret, -EINVAL);
-        assert_return(ifindex > 0, -EINVAL);
 
         lldp = new0(sd_lldp, 1);
         if (!lldp)
                 return -ENOMEM;
 
+        lldp->n_ref = 1;
         lldp->fd = -1;
-        lldp->ifindex = ifindex;
         lldp->neighbors_max = LLDP_DEFAULT_NEIGHBORS_MAX;
         lldp->capability_mask = (uint16_t) -1;
 
diff --git a/src/libsystemd-network/test-lldp.c b/src/libsystemd-network/test-lldp.c
index 1aae2253c0..6bcd65de0a 100644
--- a/src/libsystemd-network/test-lldp.c
+++ b/src/libsystemd-network/test-lldp.c
@@ -54,11 +54,11 @@ static void lldp_handler(sd_lldp *lldp, sd_lldp_event event, sd_lldp_neighbor *n
 static int start_lldp(sd_lldp **lldp, sd_event *e, sd_lldp_callback_t cb, void *cb_data) {
         int r;
 
-        r = sd_lldp_new(lldp, 42);
+        r = sd_lldp_new(lldp);
         if (r < 0)
                 return r;
 
-        r = sd_lldp_attach_event(*lldp, e, 0);
+        r = sd_lldp_set_ifindex(*lldp, 42);
         if (r < 0)
                 return r;
 
@@ -66,6 +66,10 @@ static int start_lldp(sd_lldp **lldp, sd_event *e, sd_lldp_callback_t cb, void *
         if (r < 0)
                 return r;
 
+        r = sd_lldp_attach_event(*lldp, e, 0);
+        if (r < 0)
+                return r;
+
         r = sd_lldp_start(*lldp);
         if (r < 0)
                 return r;
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index ee52b1ce1e..90ed55d42c 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -2364,7 +2364,11 @@ static int link_configure(Link *link) {
         }
 
         if (link_lldp_rx_enabled(link)) {
-                r = sd_lldp_new(&link->lldp, link->ifindex);
+                r = sd_lldp_new(&link->lldp);
+                if (r < 0)
+                        return r;
+
+                r = sd_lldp_set_ifindex(link->lldp, link->ifindex);
                 if (r < 0)
                         return r;
 
diff --git a/src/systemd/sd-lldp.h b/src/systemd/sd-lldp.h
index 617e0f1e17..8f096c1b99 100644
--- a/src/systemd/sd-lldp.h
+++ b/src/systemd/sd-lldp.h
@@ -30,9 +30,6 @@
 
 _SD_BEGIN_DECLARATIONS;
 
-typedef struct sd_lldp sd_lldp;
-typedef struct sd_lldp_neighbor sd_lldp_neighbor;
-
 /* IEEE 802.3AB Clause 9: TLV Types */
 enum {
         SD_LLDP_TYPE_END                 = 0,
@@ -111,6 +108,9 @@ enum {
         SD_LLDP_OUI_802_1_SUBTYPE_LINK_AGGREGATION      = 7,
 };
 
+typedef struct sd_lldp sd_lldp;
+typedef struct sd_lldp_neighbor sd_lldp_neighbor;
+
 typedef enum sd_lldp_event {
         SD_LLDP_EVENT_ADDED     = 'a',
         SD_LLDP_EVENT_REMOVED   = 'r',
@@ -120,7 +120,8 @@ typedef enum sd_lldp_event {
 
 typedef void (*sd_lldp_callback_t)(sd_lldp *lldp, sd_lldp_event event, sd_lldp_neighbor *n, void *userdata);
 
-int sd_lldp_new(sd_lldp **ret, int ifindex);
+int sd_lldp_new(sd_lldp **ret);
+sd_lldp* sd_lldp_ref(sd_lldp *lldp);
 sd_lldp* sd_lldp_unref(sd_lldp *lldp);
 
 int sd_lldp_start(sd_lldp *lldp);
@@ -130,6 +131,7 @@ int sd_lldp_attach_event(sd_lldp *lldp, sd_event *event, int64_t priority);
 int sd_lldp_detach_event(sd_lldp *lldp);
 
 int sd_lldp_set_callback(sd_lldp *lldp, sd_lldp_callback_t cb, void *userdata);
+int sd_lldp_set_ifindex(sd_lldp *lldp, int ifindex);
 
 /* Controls how much and what to store in the neighbors database */
 int sd_lldp_set_neighbors_max(sd_lldp *lldp, uint64_t n);