Skip to contentSkip to conversation

netconfig: Allow to override IPv4 DHCP DNSs with static addresses

PATCH In review Tim Kourt 2 participants 3 messages Oct 3, 2019 – Oct 4, 2019

lore
Share & tools
lore

Tim Kourt

author
#0 lore raw

PATCH 1/2 netconfig: Allow to override IPv4 DHCP DNSs with static addresses

---
 src/netconfig.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)
src/netconfig.c +29 -8
index 285d5fef..51d06ce1 100644
@@ -207,14 +207,37 @@ static char *netconfig_ipv4_get_gateway(struct netconfig *netconfig)
 static char **netconfig_ipv4_get_dns(struct netconfig *netconfig, uint8_t proto)
 {
 	const struct l_dhcp_lease *lease;
+	struct in_addr in_addr;
+	char **dns_list;
+	char **p;
 
-	switch (proto) {
-	case RTPROT_STATIC:
-
-		return l_settings_get_string_list(netconfig->active_settings,
+	p = dns_list = l_settings_get_string_list(netconfig->active_settings,
 							"IPv4", "dns", ' ');
+	if (dns_list && *dns_list) {
+		for (; *p; p++) {
+			if (inet_pton(AF_INET, *p, &in_addr) == 1)
+				continue;
 
-	case RTPROT_DHCP:
+			l_error("netconfig: Invalid IPv4 DNS address '%s' is "
+				"provided in network configuration file.", *p);
+
+			l_strv_free(dns_list);
+
+			return NULL;
+		}
+
+		/* Allow to override the DHCP DNSs with static addressing. */
+		return dns_list;
+	} else if (dns_list) {
+		l_error("netconfig: No IPv4 DNS address is provided in network "
+							"configuration file.");
+
+		l_strv_free(dns_list);
+
+		return NULL;
+	}
+
+	if (proto == RTPROT_DHCP) {
 		lease = l_dhcp_client_get_lease(netconfig->dhcp_client);
 		if (!lease)
 			return NULL;

@@ -662,9 +685,7 @@ static void netconfig_ipv4_ifaddr_add_cmd_cb(int error, uint16_t type,
 
 	dns = netconfig_ipv4_get_dns(netconfig, netconfig->rtm_protocol);
 	if (!dns) {
-		l_error("netconfig: Failed to obtain DNS addresses from %s.",
-				netconfig->rtm_protocol == RTPROT_STATIC ?
-				"setting file" : "DHCPv4 lease");
+		l_error("netconfig: Failed to obtain DNS addresses.");
 		goto done;
 	}
Signature · 1 line
2.13.6
author
#1 lore raw

PATCH 2/2 netconfig: Allow to override IPv6 DHCP DNSs with static addresses

---
 src/netconfig.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)
src/netconfig.c +13 -13
index 51d06ce1..7513c5b9 100644
@@ -346,23 +346,14 @@ static char **netconfig_ipv6_get_dns(struct netconfig *netconfig, uint8_t proto)
 	char **dns_list;
 	char **p;
 
-	switch (proto) {
-	case RTPROT_STATIC:
-		p = dns_list =
-			l_settings_get_string_list(netconfig->active_settings,
+	p = dns_list = l_settings_get_string_list(netconfig->active_settings,
 							"IPv6", "dns", ' ');
-
-		if (!dns_list || !*dns_list) {
-			l_strv_free(dns_list);
-
-			return NULL;
-		}
-
+	if (dns_list && *dns_list) {
 		for (; *p; p++) {
 			if (inet_pton(AF_INET6, *p, &in6_addr) == 1)
 				continue;
 
-			l_error("netconfig: Invalid IPv6 DNS address %s is "
+			l_error("netconfig: Invalid IPv6 DNS address '%s' is "
 				"provided in network configuration file.", *p);
 
 			l_strv_free(dns_list);

@@ -370,9 +361,18 @@ static char **netconfig_ipv6_get_dns(struct netconfig *netconfig, uint8_t proto)
 			return NULL;
 		}
 
+		/* Allow to override the DHCP DNSs with static addressing. */
 		return dns_list;
+	} else if (dns_list) {
+		l_error("netconfig: No IPv6 DNS address is provided in network "
+							"configuration file.");
 
-	case RTPROT_DHCP:
+		l_strv_free(dns_list);
+
+		return NULL;
+	}
+
+	if (proto == RTPROT_DHCP) {
 		/* TODO */
 
 		return NULL;
Signature · 1 line
2.13.6
#2 lore raw
In reply to Tim Kourt

Hi Tim,

Quoted 9 lines · Tim Kourt
---
  src/netconfig.c | 37 +++++++++++++++++++++++++++++--------
  1 file changed, 29 insertions(+), 8 deletions(-)
src/netconfig.c +0 -0
index 285d5fef..51d06ce1 100644

src/netconfig.c static char *netconfig_ipv4_get_gateway(struct netconfig *netconfig) · line 212

Quoted 6 lines
src/netconfig.c +3 -0
@@ -207,14 +207,37 @@ static char *netconfig_ipv4_get_gateway(struct netconfig *netconfig)
  static char **netconfig_ipv4_get_dns(struct netconfig *netconfig, uint8_t proto)
  {
  	const struct l_dhcp_lease *lease;
+	struct in_addr in_addr;
+	char **dns_list;
+	char **p;

In general it is always a good idea to limit the scope of variables. So char **p could be defined in the opening if() below.

Quoted 7 lines
src/netconfig.c +1 -4
-	switch (proto) {
-	case RTPROT_STATIC:
-
-		return l_settings_get_string_list(netconfig->active_settings,
+	p = dns_list = l_settings_get_string_list(netconfig->active_settings,
  							"IPv4", "dns", ' ');

src/netconfig.c static char *netconfig_ipv4_get_gateway(struct netconfig *netconfig) · line 224

Quoted 10 lines
src/netconfig.c +8 -1
+	if (dns_list && *dns_list) {
+		for (; *p; p++) {
+			if (inet_pton(AF_INET, *p, &in_addr) == 1)
+				continue;
 
-	case RTPROT_DHCP:
+			l_error("netconfig: Invalid IPv4 DNS address '%s' is "
+				"provided in network configuration file.", *p);
+
+			l_strv_free(dns_list);

So the way you have it implemented now, it might be nice to fall back to DHCP-provided DNS if parsing the override fails and DHCP is enabled.

I still would like to get the parameter validation done up-front so that such policy decisions can be made there...

Quoted 21 lines
src/netconfig.c +16 -0
+
+			return NULL;
+		}
+
+		/* Allow to override the DHCP DNSs with static addressing. */
+		return dns_list;
+	} else if (dns_list) {
+		l_error("netconfig: No IPv4 DNS address is provided in network "
+							"configuration file.");
+
+		l_strv_free(dns_list);
+
+		return NULL;
+	}
+
+	if (proto == RTPROT_DHCP) {
  		lease = l_dhcp_client_get_lease(netconfig->dhcp_client);
  		if (!lease)
  			return NULL;

src/netconfig.c static void netconfig_ipv4_ifaddr_add_cmd_cb(int error, uint16_t type, · line 691

Quoted 10 lines
src/netconfig.c +1 -3
@@ -662,9 +685,7 @@ static void netconfig_ipv4_ifaddr_add_cmd_cb(int error, uint16_t type,
 
  	dns = netconfig_ipv4_get_dns(netconfig, netconfig->rtm_protocol);
  	if (!dns) {
-		l_error("netconfig: Failed to obtain DNS addresses from %s.",
-				netconfig->rtm_protocol == RTPROT_STATIC ?
-				"setting file" : "DHCPv4 lease");
+		l_error("netconfig: Failed to obtain DNS addresses.");
  		goto done;
  	}

Anyway, applied.

Regards,
-Denis