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

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

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 1885 bytes --]

---
 src/netconfig.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/netconfig.c b/src/netconfig.c
index 285d5fef..51d06ce1 100644
--- a/src/netconfig.c
+++ b/src/netconfig.c
@@ -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;
 	}
 
-- 
2.13.6

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

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 1526 bytes --]

---
 src/netconfig.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/netconfig.c b/src/netconfig.c
index 51d06ce1..7513c5b9 100644
--- a/src/netconfig.c
+++ b/src/netconfig.c
@@ -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;
-- 
2.13.6

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

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2507 bytes --]

Hi Tim,

On 10/3/19 3:48 PM, Tim Kourt wrote:
> ---
>   src/netconfig.c | 37 +++++++++++++++++++++++++++++--------
>   1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/src/netconfig.c b/src/netconfig.c
> index 285d5fef..51d06ce1 100644
> --- a/src/netconfig.c
> +++ b/src/netconfig.c
> @@ -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.

>   
> -	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);

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...

> +
> +			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;
>   	}
>   
> 

Anyway, applied.

Regards,
-Denis