Index | Thread | Search

From:
Lloyd <ng2d68@proton.me>
Subject:
Re: wg(4) logging enhancement - revised patch
To:
Vitaliy Makkoveev <otto@bsdbox.dev>
Cc:
Stuart Henderson <stu@spacehopper.org>, "tech@openbsd.org" <tech@openbsd.org>
Date:
Thu, 02 Jan 2025 01:26:55 +0000

Download raw body.

Thread
Vitaliy Makkoveev wrote:

> 1. peer->p_endpoint should be protected with p_endpoint_mtx mutex(9).
> 
> 2. Do we need to output ip addresses? I mean outside wg(4) too. If
> so, isn’t it better to teach kprintf() something like '%pI{4,6}’
> format?

Thank you for the feedback. I have added the mutex calls to protect
p_endpoint and cleaned up the source to comply with style(9).

The reason for swapping out the macro for individual code blocks is
each logging instance is handled a bit differently than the others
if it contains an IP address, calling wg_log_ip() and addlog().

I think having IP address support in printf(9) is a good long-term
goal to simplify code across a few different network devices and
pf, but until that exists I wouldn't want perfect to stand in the
way of good enough for logging.

--- if_wg.c.orig	Thu Oct 31 12:33:11 2024
+++ if_wg.c	Thu Jan  2 01:07:53 2025
@@ -30,6 +30,8 @@
 #include <sys/percpu.h>
 #include <sys/ioctl.h>
 #include <sys/mbuf.h>
+#include <sys/syslog.h>
+#include <sys/time.h>
 
 #include <net/if.h>
 #include <net/if_var.h>
@@ -70,9 +72,6 @@
 #define NEW_HANDSHAKE_TIMEOUT	(REKEY_TIMEOUT + KEEPALIVE_TIMEOUT)
 #define UNDERLOAD_TIMEOUT	1
 
-#define DPRINTF(sc, str, ...) do { if (ISSET((sc)->sc_if.if_flags, IFF_DEBUG))\
-    printf("%s: " str, (sc)->sc_if.if_xname, ##__VA_ARGS__); } while (0)
-
 #define CONTAINER_OF(ptr, type, member) ({			\
 	const __typeof( ((type *)0)->member ) *__mptr = (ptr);	\
 	(type *)( (char *)__mptr - offsetof(type,member) );})
@@ -371,6 +370,7 @@ void	wg_down(struct wg_softc *);
 int	wg_clone_create(struct if_clone *, int);
 int	wg_clone_destroy(struct ifnet *);
 void	wgattach(int);
+void	wg_log_ip(struct wg_endpoint *);
 
 uint64_t	peer_counter = 0;
 struct pool	wg_aip_pool;
@@ -448,7 +448,10 @@ wg_peer_create(struct wg_softc *sc, uint8_t public[WG_
 	sc->sc_peer_num++;
 	rw_exit_write(&sc->sc_peer_lock);
 
-	DPRINTF(sc, "Peer %llu created\n", peer->p_id);
+	if (ISSET(sc->sc_if.if_flags, IFF_DEBUG))
+		log(LOG_INFO, "%s: Peer %llu created\n", sc->sc_if.if_xname,
+		    peer->p_id);
+
 	return peer;
 }
 
@@ -530,7 +533,10 @@ wg_peer_destroy(struct wg_peer *peer)
 	if (!mq_empty(&peer->p_stage_queue))
 		mq_purge(&peer->p_stage_queue);
 
-	DPRINTF(sc, "Peer %llu destroyed\n", peer->p_id);
+	if (ISSET(sc->sc_if.if_flags, IFF_DEBUG))
+		log(LOG_INFO, "%s: Peer %llu destroyed\n", sc->sc_if.if_xname,
+		    peer->p_id);
+	
 	explicit_bzero(peer, sizeof(*peer));
 	pool_put(&wg_peer_pool, peer);
 }
@@ -882,8 +888,9 @@ retry:
 		}
 	} else {
 		ret = wg_send(sc, e, m);
-		if (ret != 0)
-			DPRINTF(sc, "Unable to send packet\n");
+		if (ret != 0 && ISSET(sc->sc_if.if_flags, IFF_DEBUG))
+			log(LOG_DEBUG, "%s: Unable to send packet\n",
+			    sc->sc_if.if_xname);
 	}
 }
 
@@ -1149,19 +1156,33 @@ wg_timers_run_retry_handshake(void *_t)
 	if (t->t_handshake_retries <= MAX_TIMER_HANDSHAKES) {
 		t->t_handshake_retries++;
 		mtx_leave(&t->t_handshake_mtx);
+	
+		if (ISSET(peer->p_sc->sc_if.if_flags, IFF_DEBUG)) {
+			log(LOG_INFO, "%s: Handshake for peer %llu (",
+			    peer->p_sc->sc_if.if_xname, peer->p_id);
+			mtx_enter(&peer->p_endpoint_mtx);
+			wg_log_ip(&peer->p_endpoint);
+			mtx_leave(&peer->p_endpoint_mtx);
+			addlog(") did not complete after %d seconds, retrying "
+			    "(try %d)\n", REKEY_TIMEOUT,
+			    t->t_handshake_retries + 1);
+		}
 
-		DPRINTF(peer->p_sc, "Handshake for peer %llu did not complete "
-		    "after %d seconds, retrying (try %d)\n", peer->p_id,
-		    REKEY_TIMEOUT, t->t_handshake_retries + 1);
 		wg_peer_clear_src(peer);
 		wg_timers_run_send_initiation(t, 1);
 	} else {
 		mtx_leave(&t->t_handshake_mtx);
+		
+		if (ISSET(peer->p_sc->sc_if.if_flags, IFF_DEBUG)) {
+			log(LOG_INFO, "%s: Handshake for peer %llu (",
+			    peer->p_sc->sc_if.if_xname, peer->p_id);
+			mtx_enter(&peer->p_endpoint_mtx);
+			wg_log_ip(&peer->p_endpoint);
+			mtx_leave(&peer->p_endpoint_mtx);
+			addlog(") did not complete after %d retries, giving up"
+			    "\n", MAX_TIMER_HANDSHAKES + 2);
+		}
 
-		DPRINTF(peer->p_sc, "Handshake for peer %llu did not complete "
-		    "after %d retries, giving up\n", peer->p_id,
-		    MAX_TIMER_HANDSHAKES + 2);
-
 		timeout_del(&t->t_send_keepalive);
 		mq_purge(&peer->p_stage_queue);
 		if (!timeout_pending(&t->t_zero_key_material))
@@ -1188,10 +1209,17 @@ wg_timers_run_new_handshake(void *_t)
 {
 	struct wg_timers *t = _t;
 	struct wg_peer	 *peer = CONTAINER_OF(t, struct wg_peer, p_timers);
-
-	DPRINTF(peer->p_sc, "Retrying handshake with peer %llu because we "
-	    "stopped hearing back after %d seconds\n",
-	    peer->p_id, NEW_HANDSHAKE_TIMEOUT);
+	    
+	if (ISSET(peer->p_sc->sc_if.if_flags, IFF_DEBUG)) {
+		log(LOG_INFO, "%s: Retrying handshake with peer %llu (",
+		    peer->p_sc->sc_if.if_xname, peer->p_id);
+		mtx_enter(&peer->p_endpoint_mtx);
+		wg_log_ip(&peer->p_endpoint);
+		mtx_leave(&peer->p_endpoint_mtx);
+		addlog(") because we stopped hearing back after %d seconds\n",
+		    NEW_HANDSHAKE_TIMEOUT);
+	}
+	    
 	wg_peer_clear_src(peer);
 
 	wg_timers_run_send_initiation(t, 0);
@@ -1203,7 +1231,14 @@ wg_timers_run_zero_key_material(void *_t)
 	struct wg_timers *t = _t;
 	struct wg_peer	 *peer = CONTAINER_OF(t, struct wg_peer, p_timers);
 
-	DPRINTF(peer->p_sc, "Zeroing out keys for peer %llu\n", peer->p_id);
+	if (ISSET(peer->p_sc->sc_if.if_flags, IFF_DEBUG)) {
+		log(LOG_INFO, "%s: Zeroing out keys for peer %llu (",
+		    peer->p_sc->sc_if.if_xname, peer->p_id);
+		mtx_enter(&peer->p_endpoint_mtx);
+		wg_log_ip(&peer->p_endpoint);
+		mtx_leave(&peer->p_endpoint_mtx);
+		addlog(")\n");
+	}
 	task_add(wg_handshake_taskq, &peer->p_clear_secrets);
 }
 
@@ -1237,10 +1272,16 @@ wg_send_initiation(void *_peer)
 
 	if (wg_timers_check_handshake_last_sent(&peer->p_timers) != ETIMEDOUT)
 		return;
+	    
+	if (ISSET(peer->p_sc->sc_if.if_flags, IFF_DEBUG)) {
+		log(LOG_INFO, "%s: Sending handshake initiation to peer %llu (",
+		    peer->p_sc->sc_if.if_xname, peer->p_id);
+		mtx_enter(&peer->p_endpoint_mtx);
+		wg_log_ip(&peer->p_endpoint);
+		mtx_leave(&peer->p_endpoint_mtx);
+		addlog(")\n");
+	}
 
-	DPRINTF(peer->p_sc, "Sending handshake initiation to peer %llu\n",
-	    peer->p_id);
-
 	if (noise_create_initiation(&peer->p_remote, &pkt.s_idx, pkt.ue, pkt.es,
 				    pkt.ets) != 0)
 		return;
@@ -1255,10 +1296,17 @@ void
 wg_send_response(struct wg_peer *peer)
 {
 	struct wg_pkt_response	 pkt;
+	
+	if (ISSET(peer->p_sc->sc_if.if_flags, IFF_DEBUG)) {
+		log(LOG_INFO, "%s: Sending handshake response to peer %llu (",
+		    peer->p_sc->sc_if.if_xname, peer->p_id);
+		mtx_enter(&peer->p_endpoint_mtx);
+		wg_log_ip(&peer->p_endpoint);
+		mtx_leave(&peer->p_endpoint_mtx);
+		addlog(")\n");
+	}
+	
 
-	DPRINTF(peer->p_sc, "Sending handshake response to peer %llu\n",
-	    peer->p_id);
-
 	if (noise_create_response(&peer->p_remote, &pkt.s_idx, &pkt.r_idx,
 				  pkt.ue, pkt.en) != 0)
 		return;
@@ -1277,9 +1325,11 @@ wg_send_cookie(struct wg_softc *sc, struct cookie_macs
     struct wg_endpoint *e)
 {
 	struct wg_pkt_cookie	pkt;
+	
+	if (ISSET(sc->sc_if.if_flags, IFF_DEBUG))
+		log(LOG_DEBUG, "%s: Sending cookie response for denied "
+		    "handshake message\n", sc->sc_if.if_xname);
 
-	DPRINTF(sc, "Sending cookie response for denied handshake message\n");
-
 	pkt.t = WG_PKT_COOKIE;
 	pkt.r_idx = idx;
 
@@ -1363,30 +1413,50 @@ wg_handshake(struct wg_softc *sc, struct mbuf *m)
 				underload, &t->t_endpoint.e_remote.r_sa);
 
 		if (res == EINVAL) {
-			DPRINTF(sc, "Invalid initiation MAC\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_DEBUG, "%s: Invalid initiation MAC "
+				    "from ", sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		} else if (res == ECONNREFUSED) {
-			DPRINTF(sc, "Handshake ratelimited\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_DEBUG, "%s: Handshake ratelimited "
+				    "from ", sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		} else if (res == EAGAIN) {
 			wg_send_cookie(sc, &init->m, init->s_idx,
 			    &t->t_endpoint);
 			goto error;
 		} else if (res != 0) {
-			panic("unexpected response: %d", res);
+			panic("wg: Unexpected response: %d", res);
 		}
 
 		if (noise_consume_initiation(&sc->sc_local, &remote,
 		    init->s_idx, init->ue, init->es, init->ets) != 0) {
-			DPRINTF(sc, "Invalid handshake initiation\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_DEBUG, "%s: Invalid handshake "
+				    "initiation from ", sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		}
 
 		peer = CONTAINER_OF(remote, struct wg_peer, p_remote);
+		    
+		if (ISSET(peer->p_sc->sc_if.if_flags, IFF_DEBUG)) {
+			log(LOG_INFO, "%s: Receiving handshake initiation from"
+			    " peer %llu (", peer->p_sc->sc_if.if_xname,
+			    peer->p_id);
+			wg_log_ip(&t->t_endpoint);
+			addlog(")\n");
+		}
 
-		DPRINTF(sc, "Receiving handshake initiation from peer %llu\n",
-		    peer->p_id);
-
 		wg_peer_counters_add(peer, 0, sizeof(*init));
 		wg_peer_set_endpoint_from_tag(peer, t);
 		wg_send_response(peer);
@@ -1397,37 +1467,61 @@ wg_handshake(struct wg_softc *sc, struct mbuf *m)
 		res = cookie_checker_validate_macs(&sc->sc_cookie, &resp->m,
 				resp, sizeof(*resp) - sizeof(resp->m),
 				underload, &t->t_endpoint.e_remote.r_sa);
-
 		if (res == EINVAL) {
-			DPRINTF(sc, "Invalid response MAC\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_DEBUG, "%s: Invalid response MAC from ",
+				    sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		} else if (res == ECONNREFUSED) {
-			DPRINTF(sc, "Handshake ratelimited\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_DEBUG, "%s: Handshake ratelimited from ",
+				    sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		} else if (res == EAGAIN) {
 			wg_send_cookie(sc, &resp->m, resp->s_idx,
 			    &t->t_endpoint);
 			goto error;
 		} else if (res != 0) {
-			panic("unexpected response: %d", res);
+			panic("wg: Unexpected response: %d", res);
 		}
 
 		if ((remote = wg_index_get(sc, resp->r_idx)) == NULL) {
-			DPRINTF(sc, "Unknown handshake response\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_DEBUG, "%s: Unknown handshake response "
+				    "from ", sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		}
 
 		peer = CONTAINER_OF(remote, struct wg_peer, p_remote);
-
 		if (noise_consume_response(remote, resp->s_idx, resp->r_idx,
 					   resp->ue, resp->en) != 0) {
-			DPRINTF(sc, "Invalid handshake response\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_INFO, "%s: Invalid handshake response "
+				    "from ", sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		}
+				
+		if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+			log(LOG_INFO, "%s: Receiving handshake response from "
+			    "peer %llu (", sc->sc_if.if_xname, peer->p_id);
+			mtx_enter(&peer->p_endpoint_mtx);
+			wg_log_ip(&peer->p_endpoint);
+			mtx_leave(&peer->p_endpoint_mtx);
+			addlog(")\n");
+		}		
 
-		DPRINTF(sc, "Receiving handshake response from peer %llu\n",
-				peer->p_id);
-
 		wg_peer_counters_add(peer, 0, sizeof(*resp));
 		wg_peer_set_endpoint_from_tag(peer, t);
 		if (noise_remote_begin_session(&peer->p_remote) == 0) {
@@ -1439,7 +1533,12 @@ wg_handshake(struct wg_softc *sc, struct mbuf *m)
 		cook = mtod(m, struct wg_pkt_cookie *);
 
 		if ((remote = wg_index_get(sc, cook->r_idx)) == NULL) {
-			DPRINTF(sc, "Unknown cookie index\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_DEBUG, "%s: Unknown cookie index from ",
+				    sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		}
 
@@ -1447,14 +1546,24 @@ wg_handshake(struct wg_softc *sc, struct mbuf *m)
 
 		if (cookie_maker_consume_payload(&peer->p_cookie,
 		    cook->nonce, cook->ec) != 0) {
-			DPRINTF(sc, "Could not decrypt cookie response\n");
+			if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+				log(LOG_DEBUG, "%s: Could not decrypt cookie "
+				    "response from ", sc->sc_if.if_xname);
+				wg_log_ip(&t->t_endpoint);
+				addlog("\n");
+			}
 			goto error;
 		}
 
-		DPRINTF(sc, "Receiving cookie response\n");
+		if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+			log(LOG_DEBUG, "%s: Receiving cookie response from ",
+			    sc->sc_if.if_xname);
+			wg_log_ip(&t->t_endpoint);
+			addlog("\n");
+		}
 		goto error;
 	default:
-		panic("invalid packet in handshake queue");
+		panic("wg: Invalid packet in handshake queue");
 	}
 
 	wg_timers_event_any_authenticated_packet_received(&peer->p_timers);
@@ -1505,6 +1614,8 @@ wg_encap(struct wg_softc *sc, struct mbuf *m)
 	struct mbuf		*mc;
 	size_t			 padding_len, plaintext_len, out_len;
 	uint64_t		 nonce;
+	static struct timeval	 lastkeepalivemsg;
+	struct timeval		 errint = { 300, 0 };
 
 	t = wg_tag_get(m);
 	peer = t->t_peer;
@@ -1557,10 +1668,17 @@ wg_encap(struct wg_softc *sc, struct mbuf *m)
 	}
 
 	/* A packet with length 0 is a keepalive packet */
-	if (__predict_false(m->m_pkthdr.len == 0))
-		DPRINTF(sc, "Sending keepalive packet to peer %llu\n",
-		    peer->p_id);
-
+	if (__predict_false(m->m_pkthdr.len == 0) &&
+	    ISSET(sc->sc_if.if_flags, IFF_DEBUG) &&
+	    ratecheck(&lastkeepalivemsg, &errint)) {
+		log(LOG_DEBUG, "%s: Sending keepalive packet to peer "
+		    "%llu (", sc->sc_if.if_xname, peer->p_id);
+		mtx_enter(&peer->p_endpoint_mtx);
+		wg_log_ip(&peer->p_endpoint);
+		mtx_leave(&peer->p_endpoint_mtx);
+		addlog(")\n");
+	}
+	
 	mc->m_pkthdr.ph_loopcnt = m->m_pkthdr.ph_loopcnt;
 	mc->m_flags &= ~(M_MCAST | M_BCAST);
 	mc->m_pkthdr.len = mc->m_len = out_len;
@@ -1590,6 +1708,8 @@ wg_decap(struct wg_softc *sc, struct mbuf *m)
 	struct wg_tag		*t;
 	size_t			 payload_len;
 	uint64_t		 nonce;
+	static struct timeval	 lastkeepalivemsg;
+	struct timeval		 errint = { 300, 0 };
 
 	t = wg_tag_get(m);
 	peer = t->t_peer;
@@ -1631,8 +1751,15 @@ wg_decap(struct wg_softc *sc, struct mbuf *m)
 
 	/* A packet with length 0 is a keepalive packet */
 	if (__predict_false(m->m_pkthdr.len == 0)) {
-		DPRINTF(sc, "Receiving keepalive packet from peer "
-		    "%llu\n", peer->p_id);
+		if (ISSET(sc->sc_if.if_flags, IFF_DEBUG) &&
+		    ratecheck(&lastkeepalivemsg, &errint)) {
+			log(LOG_DEBUG, "%s: Receiving keepalive packet from "
+			    "peer %llu (", sc->sc_if.if_xname, peer->p_id);
+			mtx_enter(&peer->p_endpoint_mtx);
+			wg_log_ip(&peer->p_endpoint);
+			mtx_leave(&peer->p_endpoint_mtx);
+			addlog(")\n");
+		}
 		goto done;
 	}
 
@@ -1670,14 +1797,27 @@ wg_decap(struct wg_softc *sc, struct mbuf *m)
 		allowed_peer = wg_aip_lookup(sc->sc_aip6, &ip6->ip6_src);
 #endif
 	} else {
-		DPRINTF(sc, "Packet is neither ipv4 nor ipv6 from "
-		    "peer %llu\n", peer->p_id);
+		if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+			log(LOG_WARNING, "%s: Packet is neither IPv4 nor IPv6 "
+			    "from peer %llu (", sc->sc_if.if_xname, peer->p_id);
+			mtx_enter(&peer->p_endpoint_mtx);
+			wg_log_ip(&peer->p_endpoint);
+			mtx_leave(&peer->p_endpoint_mtx);
+			addlog(")\n");
+		}
+		    
 		goto error;
 	}
 
 	if (__predict_false(peer != allowed_peer)) {
-		DPRINTF(sc, "Packet has unallowed src IP from peer "
-		    "%llu\n", peer->p_id);
+		if (ISSET(sc->sc_if.if_flags, IFF_DEBUG)) {
+			log(LOG_WARNING, "%s: Packet has unallowed src IP from"
+			    " peer %llu (", sc->sc_if.if_xname, peer->p_id);
+			mtx_enter(&peer->p_endpoint_mtx);			    
+			wg_log_ip(&peer->p_endpoint);
+			mtx_leave(&peer->p_endpoint_mtx);
+			addlog(")\n");
+		}
 		goto error;
 	}
 
@@ -2058,8 +2198,11 @@ wg_input(void *_sc, struct mbuf *m, struct ip *ip, str
 	    (m->m_pkthdr.len == sizeof(struct wg_pkt_cookie) &&
 		*mtod(m, uint32_t *) == WG_PKT_COOKIE)) {
 
-		if (mq_enqueue(&sc->sc_handshake_queue, m) != 0)
-			DPRINTF(sc, "Dropping handshake packet\n");
+		if (mq_enqueue(&sc->sc_handshake_queue, m) != 0 &&
+		    ISSET(sc->sc_if.if_flags, IFF_DEBUG))
+			log(LOG_DEBUG, "%s: Dropping handshake packet\n",
+			    sc->sc_if.if_xname);
+		
 		task_add(wg_handshake_taskq, &sc->sc_handshake);
 
 	} else if (m->m_pkthdr.len >= sizeof(struct wg_pkt_data) +
@@ -2130,10 +2273,12 @@ int
 wg_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *sa,
     struct rtentry *rt)
 {
-	struct wg_softc	*sc = ifp->if_softc;
-	struct wg_peer	*peer;
-	struct wg_tag	*t;
-	int		 af, ret = EINVAL;
+	struct wg_softc		*sc = ifp->if_softc;
+	struct wg_peer		*peer;
+	struct wg_tag		*t;
+	int		 	 af, ret = EINVAL;
+	static struct timeval	 lasterr, looperr, delayerr;
+	struct timeval		 errint = { 300, 0 };
 
 	NET_ASSERT_LOCKED();
 
@@ -2166,17 +2311,22 @@ wg_output(struct ifnet *ifp, struct mbuf *m, struct so
 		ret = ENETUNREACH;
 		goto error;
 	}
-
 	af = peer->p_endpoint.e_remote.r_sa.sa_family;
-	if (af != AF_INET && af != AF_INET6) {
-		DPRINTF(sc, "No valid endpoint has been configured or "
-				"discovered for peer %llu\n", peer->p_id);
+	if (af != AF_INET && af != AF_INET6) {	
+		if (ISSET(sc->sc_if.if_flags, IFF_DEBUG) &&
+		    ratecheck(&lasterr, &errint))
+			log(LOG_DEBUG, "%s: No valid endpoint has been "
+			    "configured or discovered for peer %llu\n",
+			    sc->sc_if.if_xname, peer->p_id);
 		ret = EDESTADDRREQ;
 		goto error;
 	}
 
 	if (m->m_pkthdr.ph_loopcnt++ > M_MAXLOOP) {
-		DPRINTF(sc, "Packet looped\n");
+		if (ISSET(sc->sc_if.if_flags, IFF_DEBUG) &&
+		    ratecheck(&looperr, &errint))
+			log(LOG_DEBUG, "%s: Packet looped\n",
+			    sc->sc_if.if_xname);			
 		ret = ELOOP;
 		goto error;
 	}
@@ -2189,7 +2339,10 @@ wg_output(struct ifnet *ifp, struct mbuf *m, struct so
 	 * another aip_lookup in wg_qstart, or refcnting as mentioned before.
 	 */
 	if (m->m_pkthdr.pf.delay > 0) {
-		DPRINTF(sc, "PF delay unsupported\n");
+		if (ISSET(sc->sc_if.if_flags, IFF_DEBUG) &&
+		    ratecheck(&delayerr, &errint))
+			log(LOG_DEBUG, "%s: PF delay unsupported\n",
+			    sc->sc_if.if_xname);
 		ret = EOPNOTSUPP;
 		goto error;
 	}
@@ -2701,9 +2854,9 @@ wg_clone_create(struct if_clone *ifc, int unit)
 #if NBPFILTER > 0
 	bpfattach(&ifp->if_bpf, ifp, DLT_LOOP, sizeof(uint32_t));
 #endif
-
-	DPRINTF(sc, "Interface created\n");
-
+	
+	if (ISSET(sc->sc_if.if_flags, IFF_DEBUG))
+		log(LOG_INFO, "%s: Interface created\n", sc->sc_if.if_xname);
 	return 0;
 ret_05:
 	hashfree(sc->sc_peer, HASHTABLE_PEER_SIZE, M_DEVBUF);
@@ -2744,9 +2897,10 @@ wg_clone_destroy(struct ifnet *ifp)
 		wg_handshake_taskq = NULL;
 		wg_crypt_taskq = NULL;
 	}
+	
+	if (ISSET(sc->sc_if.if_flags, IFF_DEBUG))
+		log(LOG_INFO, "%s: Interface destroyed\n", sc->sc_if.if_xname);
 
-	DPRINTF(sc, "Destroyed interface\n");
-
 	hashfree(sc->sc_index, HASHTABLE_INDEX_SIZE, M_DEVBUF);
 	hashfree(sc->sc_peer, HASHTABLE_PEER_SIZE, M_DEVBUF);
 #ifdef INET6
@@ -2773,4 +2927,64 @@ wgattach(int nwg)
 			IPL_NET, 0, "wgpeer", NULL);
 	pool_init(&wg_ratelimit_pool, sizeof(struct ratelimit_entry), 0,
 			IPL_NET, 0, "wgratelimit", NULL);
+}
+
+void
+wg_log_ip(struct wg_endpoint *ep)
+{
+	u_int32_t	addr;
+#ifdef INET6
+	struct in6_addr	addr6;
+#endif
+
+	switch(ep->e_remote.r_sa.sa_family) {
+		case AF_INET:
+			addr = ntohl(ep->e_remote.r_sin.sin_addr.s_addr);
+			addlog("%u.%u.%u.%u", (addr>>24)&255, (addr>>16)&255,
+			    (addr>>8)&255, addr&255);
+			break;
+#ifdef INET6
+		case AF_INET6:
+			addr6 = ep->e_remote.r_sin6.sin6_addr;
+			u_int16_t	b;
+			u_int8_t	i, curstart, curend, maxstart, maxend;
+			curstart = curend = maxstart = maxend = 255;
+			for (i = 0; i < 8; i++) {
+				if (!addr6.s6_addr16[i]) {
+					if (curstart == 255)
+						curstart = i;
+					curend = i;
+				}
+				else {
+					if ((curend - curstart) >
+					    (maxend - maxstart)) {
+						maxstart = curstart;
+						maxend = curend;
+					}
+					curstart = curend = 255;
+				}
+			}
+			if ((curend - curstart) > (maxend - maxstart)) {
+				maxstart = curstart;
+				maxend = curend;
+			}
+			for (i = 0; i < 8; i++) {
+				if (i >= maxstart && i <= maxend) {
+					if (i == 0)
+						addlog(":");
+					if (i == maxend)
+						addlog(":");
+				}
+				else {
+					b = ntohs(addr6.s6_addr16[i]);
+					addlog("%x", b);
+					if (i < 7)
+						addlog(":");
+				}
+			}
+			break;			
+#endif
+		default:
+			break;
+	}
 }