Merge pull request #17798 from yuwata/ipv4ll-follow-ups

network: improve debug logs and add tests for IPv4LL

src/libsystemd-network/sd-ipv4acd.c

@@ -16,10 +16,12 @@
 #include "ether-addr-util.h"
 #include "event-util.h"
 #include "fd-util.h"
+#include "format-util.h"
 #include "in-addr-util.h"
-#include "list.h"
+#include "log-link.h"
 #include "random-util.h"
 #include "siphash24.h"
+#include "string-table.h"
 #include "string-util.h"
 #include "time-util.h"
 
@@ -54,6 +56,7 @@ struct sd_ipv4acd {
         int ifindex;
         int fd;
 
+        char ifname[IF_NAMESIZE + 1];
         unsigned n_iteration;
         unsigned n_conflict;
 
@@ -72,13 +75,30 @@ struct sd_ipv4acd {
         void* userdata;
 };
 
-#define log_ipv4acd_errno(acd, error, fmt, ...) log_internal(LOG_DEBUG, error, PROJECT_FILE, __LINE__, __func__, "IPV4ACD: " fmt, ##__VA_ARGS__)
-#define log_ipv4acd(acd, fmt, ...) log_ipv4acd_errno(acd, 0, fmt, ##__VA_ARGS__)
+#define log_ipv4acd_errno(acd, error, fmt, ...)                         \
+        log_interface_full_errno(sd_ipv4acd_get_ifname(acd), LOG_DEBUG, error, "IPV4ACD: " fmt, ##__VA_ARGS__)
+#define log_ipv4acd(acd, fmt, ...)                      \
+        log_ipv4acd_errno(acd, 0, fmt, ##__VA_ARGS__)
+
+static const char * const ipv4acd_state_table[_IPV4ACD_STATE_MAX] = {
+        [IPV4ACD_STATE_INIT]             = "init",
+        [IPV4ACD_STATE_STARTED]          = "started",
+        [IPV4ACD_STATE_WAITING_PROBE]    = "waiting-probe",
+        [IPV4ACD_STATE_PROBING]          = "probing",
+        [IPV4ACD_STATE_WAITING_ANNOUNCE] = "waiting-announce",
+        [IPV4ACD_STATE_ANNOUNCING]       = "announcing",
+        [IPV4ACD_STATE_RUNNING]          = "running",
+};
+
+DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(ipv4acd_state, IPv4ACDState);
 
 static void ipv4acd_set_state(sd_ipv4acd *acd, IPv4ACDState st, bool reset_counter) {
         assert(acd);
         assert(st < _IPV4ACD_STATE_MAX);
 
+        if (st != acd->state)
+                log_ipv4acd(acd, "%s -> %s", ipv4acd_state_to_string(acd->state), ipv4acd_state_to_string(st));
+
         if (st == acd->state && !reset_counter)
                 acd->n_iteration++;
         else {
@@ -378,15 +398,35 @@ fail:
 }
 
 int sd_ipv4acd_set_ifindex(sd_ipv4acd *acd, int ifindex) {
+        char ifname[IF_NAMESIZE + 1];
+
         assert_return(acd, -EINVAL);
         assert_return(ifindex > 0, -EINVAL);
         assert_return(acd->state == IPV4ACD_STATE_INIT, -EBUSY);
 
+        if (!format_ifname(ifindex, ifname))
+                return -ENODEV;
+
+        strcpy(acd->ifname, ifname);
         acd->ifindex = ifindex;
 
         return 0;
 }
 
+int sd_ipv4acd_get_ifindex(sd_ipv4acd *acd) {
+        if (!acd)
+                return -EINVAL;
+
+        return acd->ifindex;
+}
+
+const char *sd_ipv4acd_get_ifname(sd_ipv4acd *acd) {
+        if (!acd)
+                return NULL;
+
+        return empty_to_null(acd->ifname);
+}
+
 int sd_ipv4acd_set_mac(sd_ipv4acd *acd, const struct ether_addr *addr) {
         assert_return(acd, -EINVAL);
         assert_return(addr, -EINVAL);

src/libsystemd-network/sd-ipv4ll.c

@@ -15,7 +15,7 @@
 #include "alloc-util.h"
 #include "ether-addr-util.h"
 #include "in-addr-util.h"
-#include "list.h"
+#include "log-link.h"
 #include "random-util.h"
 #include "siphash24.h"
 #include "sparse-endian.h"
@@ -49,8 +49,10 @@ struct sd_ipv4ll {
         void* userdata;
 };
 
-#define log_ipv4ll_errno(ll, error, fmt, ...) log_internal(LOG_DEBUG, error, PROJECT_FILE, __LINE__, __func__, "IPV4LL: " fmt, ##__VA_ARGS__)
-#define log_ipv4ll(ll, fmt, ...) log_ipv4ll_errno(ll, 0, fmt, ##__VA_ARGS__)
+#define log_ipv4ll_errno(ll, error, fmt, ...)                           \
+        log_interface_full_errno(sd_ipv4ll_get_ifname(ll), LOG_DEBUG, error, "IPV4LL: " fmt, ##__VA_ARGS__)
+#define log_ipv4ll(ll, fmt, ...)                        \
+        log_ipv4ll_errno(ll, 0, fmt, ##__VA_ARGS__)
 
 static void ipv4ll_on_acd(sd_ipv4acd *ll, int event, void *userdata);
 
@@ -103,6 +105,20 @@ int sd_ipv4ll_set_ifindex(sd_ipv4ll *ll, int ifindex) {
         return sd_ipv4acd_set_ifindex(ll->acd, ifindex);
 }
 
+int sd_ipv4ll_get_ifindex(sd_ipv4ll *ll) {
+        if (!ll)
+                return -EINVAL;
+
+        return sd_ipv4acd_get_ifindex(ll->acd);
+}
+
+const char *sd_ipv4ll_get_ifname(sd_ipv4ll *ll) {
+        if (!ll)
+                return NULL;
+
+        return sd_ipv4acd_get_ifname(ll->acd);
+}
+
 int sd_ipv4ll_set_mac(sd_ipv4ll *ll, const struct ether_addr *addr) {
         int r;
 

src/libsystemd-network/test-ipv4ll.c

@@ -126,7 +126,6 @@ static void test_public_api_setters(sd_event *e) {
         assert_se(sd_ipv4ll_set_ifindex(ll, -1) == -EINVAL);
         assert_se(sd_ipv4ll_set_ifindex(ll, -99) == -EINVAL);
         assert_se(sd_ipv4ll_set_ifindex(ll, 1) == 0);
-        assert_se(sd_ipv4ll_set_ifindex(ll, 99) == 0);
 
         assert_se(sd_ipv4ll_ref(ll) == ll);
         assert_se(sd_ipv4ll_unref(ll) == NULL);

src/network/networkd-dhcp4.c

@@ -1277,6 +1277,8 @@ static int dhcp4_set_request_address(Link *link) {
         if (!a)
                 return 0;
 
+        log_link_debug(link, "DHCP4 CLIENT: requesting " IPV4_ADDRESS_FMT_STR, IPV4_ADDRESS_FMT_VAL(a->in_addr.in));
+
         return sd_dhcp_client_set_request_address(link->dhcp_client, &a->in_addr.in);
 }
 

src/shared/log-link.h

@@ -3,6 +3,13 @@
 
 #include "log.h"
 
+#define log_interface_full_errno(ifname, level, error, ...)             \
+        ({                                                              \
+                const char *_ifname = (ifname);                         \
+                _ifname ? log_object_internal(level, error, PROJECT_FILE, __LINE__, __func__, "INTERFACE=", _ifname, NULL, NULL, ##__VA_ARGS__) : \
+                        log_internal(level, error, PROJECT_FILE, __LINE__, __func__, ##__VA_ARGS__); \
+        })
+
 /*
  * The following macros append INTERFACE= to the message.
  * The macros require a struct named 'Link' which contains 'char *ifname':
@@ -17,9 +24,8 @@
 #define log_link_full_errno(link, level, error, ...)                    \
         ({                                                              \
                 const Link *_l = (link);                                \
-                (_l && _l->ifname) ? log_object_internal(level, error, PROJECT_FILE, __LINE__, __func__, "INTERFACE=", _l->ifname, NULL, NULL, ##__VA_ARGS__) : \
-                        log_internal(level, error, PROJECT_FILE, __LINE__, __func__, ##__VA_ARGS__); \
-        })                                                              \
+                log_interface_full_errno(_l ? _l->ifname : NULL, level, error, ##__VA_ARGS__); \
+        })
 
 #define log_link_full(link, level, ...) (void) log_link_full_errno(link, level, 0, __VA_ARGS__)
 

src/systemd/sd-ipv4acd.h

@@ -43,6 +43,8 @@ int sd_ipv4acd_get_address(sd_ipv4acd *acd, struct in_addr *address);
 int sd_ipv4acd_set_callback(sd_ipv4acd *acd, sd_ipv4acd_callback_t cb, void *userdata);
 int sd_ipv4acd_set_mac(sd_ipv4acd *acd, const struct ether_addr *addr);
 int sd_ipv4acd_set_ifindex(sd_ipv4acd *acd, int interface_index);
+int sd_ipv4acd_get_ifindex(sd_ipv4acd *acd);
+const char *sd_ipv4acd_get_ifname(sd_ipv4acd *acd);
 int sd_ipv4acd_set_address(sd_ipv4acd *acd, const struct in_addr *address);
 int sd_ipv4acd_is_running(sd_ipv4acd *acd);
 int sd_ipv4acd_start(sd_ipv4acd *acd, bool reset_conflicts);

src/systemd/sd-ipv4ll.h

@@ -43,6 +43,8 @@ int sd_ipv4ll_get_address(sd_ipv4ll *ll, struct in_addr *address);
 int sd_ipv4ll_set_callback(sd_ipv4ll *ll, sd_ipv4ll_callback_t cb, void *userdata);
 int sd_ipv4ll_set_mac(sd_ipv4ll *ll, const struct ether_addr *addr);
 int sd_ipv4ll_set_ifindex(sd_ipv4ll *ll, int interface_index);
+int sd_ipv4ll_get_ifindex(sd_ipv4ll *ll);
+const char *sd_ipv4ll_get_ifname(sd_ipv4ll *ll);
 int sd_ipv4ll_set_address(sd_ipv4ll *ll, const struct in_addr *address);
 int sd_ipv4ll_set_address_seed(sd_ipv4ll *ll, uint64_t seed);
 int sd_ipv4ll_is_running(sd_ipv4ll *ll);