network: implement refcounting for SR-IOV objects

link_request_sr_iov_vfs() queued each request with a raw SRIOV* as
userdata and a NULL free_func, so the request did not own the object.
The SRIOV is owned by link->network->sr_iov_by_section, which is freed
by network_free() on reload while the RTM_SETLINK reply may still be in
flight (the floating netlink slot keeps the request alive but does not
keep the userdata alive). A later reply, or the 25s netlink timeout,
then dispatches sr_iov_handler() which reads the freed SRIOV.

Follow-up for cb8453cc51a9d49e094e746af2e074669a71cc4a

diff --git a/src/network/networkd-sriov.c b/src/network/networkd-sriov.c
index 466442597916ffd19995f445c277fcb785e2d101..d35e108a4a95635ad3c433ae0881a666642d3008 100644 (file)
--- a/src/network/networkd-sriov.c
+++ b/src/network/networkd-sriov.c
@@ -98,11 +98,12 @@ int link_request_sr_iov_vfs(Link *link) {
                         if (!sr_iov_has_config(sr_iov, attr))
                                 continue;
 
+                        /* The request takes a reference on the configuration. */
                         r = link_queue_request_safe(
                                         link,
                                         _REQUEST_TYPE_SRIOV_BASE + attr,
                                         sr_iov,
-                                        NULL,
+                                        sr_iov_unref,
                                         sr_iov_hash_func,
                                         sr_iov_compare_func,
                                         sr_iov_process_request,
@@ -113,6 +114,8 @@ int link_request_sr_iov_vfs(Link *link) {
                                 return log_link_warning_errno(link, r,
                                                               "Failed to request to set up %s for SR-IOV virtual function %"PRIu32": %m",
                                                               sr_iov_attribute_to_string(attr), sr_iov->vf);
+                        if (r > 0)
+                                sr_iov_ref(sr_iov);
                 }
         }
 

diff --git a/src/shared/netif-sriov.c b/src/shared/netif-sriov.c
index f5df7fb392ac6e59190fd61ee47fcdb534752500..79fd86c4f67d7ac3d59484ae78865b96235fb08c 100644 (file)
--- a/src/shared/netif-sriov.c
+++ b/src/shared/netif-sriov.c
@@ -16,24 +16,42 @@
 #include "string-table.h"
 #include "string-util.h"
 
+static SRIOV* sr_iov_detach_impl(SRIOV *sr_iov) {
+        assert(sr_iov);
+
+        if (!sr_iov->sr_iov_by_section)
+                return NULL;
+
+        assert(sr_iov->section);
+        ordered_hashmap_remove(sr_iov->sr_iov_by_section, sr_iov->section);
+        sr_iov->sr_iov_by_section = NULL;
+
+        return sr_iov;
+}
+
 static SRIOV* sr_iov_free(SRIOV *sr_iov) {
         if (!sr_iov)
                 return NULL;
 
-        if (sr_iov->sr_iov_by_section && sr_iov->section)
-                ordered_hashmap_remove(sr_iov->sr_iov_by_section, sr_iov->section);
+        sr_iov_detach_impl(sr_iov);
 
         config_section_free(sr_iov->section);
 
         return mfree(sr_iov);
 }
 
-DEFINE_SECTION_CLEANUP_FUNCTIONS(SRIOV, sr_iov_free);
+DEFINE_TRIVIAL_REF_UNREF_FUNC(SRIOV, sr_iov, sr_iov_free);
+DEFINE_SECTION_CLEANUP_FUNCTIONS(SRIOV, sr_iov_unref);
+
+static void sr_iov_detach(SRIOV *sr_iov) {
+        assert(sr_iov);
+        sr_iov_unref(sr_iov_detach_impl(sr_iov));
+}
 
 DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
                 sr_iov_hash_ops_by_section,
                 ConfigSection, config_section_hash_func, config_section_compare_func,
-                SRIOV, sr_iov_free);
+                SRIOV, sr_iov_detach);
 
 static int sr_iov_new(SRIOV **ret) {
         SRIOV *sr_iov;
@@ -45,6 +63,7 @@ static int sr_iov_new(SRIOV **ret) {
                 return -ENOMEM;
 
         *sr_iov = (SRIOV) {
+                  .n_ref = 1,
                   .vf = UINT32_MAX,
                   .vlan_proto = ETH_P_8021Q,
                   .vf_spoof_check_setting = -1,
@@ -60,7 +79,7 @@ static int sr_iov_new(SRIOV **ret) {
 
 static int sr_iov_new_static(OrderedHashmap **sr_iov_by_section, const char *filename, unsigned section_line, SRIOV **ret) {
         _cleanup_(config_section_freep) ConfigSection *n = NULL;
-        _cleanup_(sr_iov_freep) SRIOV *sr_iov = NULL;
+        _cleanup_(sr_iov_unrefp) SRIOV *sr_iov = NULL;
         SRIOV *existing = NULL;
         int r;
 
@@ -368,7 +387,7 @@ int sr_iov_drop_invalid_sections(uint32_t num_vfs, OrderedHashmap *sr_iov_by_sec
                 SRIOV *dup;
 
                 if (sr_iov_section_verify(num_vfs, sr_iov) < 0) {
-                        sr_iov_free(sr_iov);
+                        sr_iov_detach(sr_iov);
                         continue;
                 }
 
@@ -378,7 +397,7 @@ int sr_iov_drop_invalid_sections(uint32_t num_vfs, OrderedHashmap *sr_iov_by_sec
                                     "dropping the [SR-IOV] section specified at line %u.",
                                     dup->section->filename, sr_iov->section->line,
                                     dup->section->line, dup->section->line);
-                        sr_iov_free(dup);
+                        sr_iov_detach(dup);
                 }
 
                 r = set_ensure_put(&set, &sr_iov_hash_ops, sr_iov);
@@ -402,7 +421,7 @@ int config_parse_sr_iov_uint32(
                 void *data,
                 void *userdata) {
 
-        _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL;
+        _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL;
         OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data);
         uint32_t k;
         int r;
@@ -469,7 +488,7 @@ int config_parse_sr_iov_vlan_proto(
                 void *data,
                 void *userdata) {
 
-        _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL;
+        _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL;
         OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data);
         int r;
 
@@ -507,7 +526,7 @@ int config_parse_sr_iov_link_state(
                 void *data,
                 void *userdata) {
 
-        _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL;
+        _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL;
         OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data);
         int r;
 
@@ -558,7 +577,7 @@ int config_parse_sr_iov_boolean(
                 void *data,
                 void *userdata) {
 
-        _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL;
+        _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL;
         OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data);
         int r;
 
@@ -615,7 +634,7 @@ int config_parse_sr_iov_mac(
                 void *data,
                 void *userdata) {
 
-        _cleanup_(sr_iov_free_or_set_invalidp) SRIOV *sr_iov = NULL;
+        _cleanup_(sr_iov_unref_or_set_invalidp) SRIOV *sr_iov = NULL;
         OrderedHashmap **sr_iov_by_section = ASSERT_PTR(data);
         int r;
 

diff --git a/src/shared/netif-sriov.h b/src/shared/netif-sriov.h
index cd2befc634084bc051530c16f9945da20461d109..578a7ea579f12a1135b6ac71591ef7cf06633717 100644 (file)
--- a/src/shared/netif-sriov.h
+++ b/src/shared/netif-sriov.h
@@ -27,6 +27,8 @@ typedef enum SRIOVLinkState {
 } SRIOVLinkState;
 
 typedef struct SRIOV {
+        unsigned n_ref;
+
         ConfigSection *section;
         OrderedHashmap *sr_iov_by_section;
 
@@ -43,6 +45,8 @@ typedef struct SRIOV {
 
 DECLARE_STRING_TABLE_LOOKUP_TO_STRING(sr_iov_attribute, SRIOVAttribute);
 
+DECLARE_TRIVIAL_REF_UNREF_FUNC(SRIOV, sr_iov);
+
 void sr_iov_hash_func(const SRIOV *sr_iov, struct siphash *state);
 int sr_iov_compare_func(const SRIOV *s1, const SRIOV *s2);
 bool sr_iov_has_config(SRIOV *sr_iov, SRIOVAttribute attr);