Hi Harald and Geert,

I looked at it and it is quite confusing what it does. Not quite simple
to understand. I think it is not uncommon for dnsmasq code, but this
brings more knobs to make it more complicated.

I tried to rewrite it hopefully with less repetitions and cleaner code.
However when I did this, I have realized current algorihm is quite bad.

When dnsmasq would have 40 client records for dhcp-host, it would have
to walk throught that list about 4*40 times for every DHCP request. It
seems to me smarter sorting of dhcp_configs would allow single or two
passes. Just push the most specific records with client ids and hwaddr
first, then just hostnames.

Anyway, I think some unit test would be needed. Any opinions on my
patches. Have not yet tested them in real network.

Cheers,
Petr

On 4/30/20 11:07 AM, Harald Jensås wrote:
> On Wed, 2020-04-29 at 16:18 +0200, Harald Jensås wrote:
>> Hi,
>>
>> We discovered an issue introduced in this commit:
>> 137286e9baecf6a3ba97722ef1b49c851b531810
>>
>> Prior to this commit one could have two dhcp-host entries, one for
>> IPv4
>> and another for IPv6, for example:
>>
>>  dhcp-host=52:54:00:bc:c3:fd,172.20.0.11,host2
>>  dhcp-host=52:54:00:bc:c3:fd,[fd12:3456:789a:1::aadd],host2
>>
>> This no longer works. In the above example dhcpv6 client succedes,
>> but
>> the dhcpv4 client get 'no address available'. Swapping the order of
>> the
>> two entries in the config file allow the dhcpv4 client to succeed,
>> but
>> then the dhcpv5 client fails.
>>
>> Alternative configurations that do work in 2.81:
>>
>>  dhcp-host=52:54:00:bc:c3:fd,172.20.0.11,host2
>>  dhcp-
>> host=tag:dhcpv6,52:54:00:bc:c3:fd,[fd12:3456:789a:1::aadd],host2
>>
>> or: 
>>
>>  dhcp-
>> host=52:54:00:bc:c3:fd,172.20.0.11,[fd12:3456:789a:1::aadd],host2
>>
>>
>> I'm not sure using two dhcp-host entries was ever intended to work,
>> as
>> the manual page states that both ipv4 and ipv6 address can be defined
>> in a single entry.
>>
>> A first tought for possible fix would be to internally set the
>> 'tag:dhcpv6' for any dhcp-host entry with only ipv6 addresse(s).
>>
> 
> An attempt for a fix, preferring dhcp-host entries with CONFIG_ADDR or
> CONFIG_ADDR6 flag set. With this change I can use these two configs
> successfully:
> 
>   dhcp-host=52:54:00:bc:c3:fd,172.20.0.11,[fd12:3456:789a:1::aadd],host2
> or
>   dhcp-host=52:54:00:bc:c3:fd,[fd12:3456:789a:1::aadd],host2
>   dhcp-host=52:54:00:bc:c3:fd,172.20.0.11,host2
> 
> (The latter works in 2.80, but fails in 2.81)
> 
> 
> 
> From 13de4c3346462146ce3826a7fc0546fdb5a569b0 Mon Sep 17 00:00:00 2001
> From: Harald Jensås <hjen...@redhat.com>
> Date: Thu, 30 Apr 2020 10:42:07 +0200
> Subject: [PATCH 1/1] Tag filtering in find_config, filter on IP version
> 
> Adds the possibility to filter on a config flag when
> looking up configuration.
> 
> Update the DHCP and DHCPv6 code to prefer dhcp-host
> entries in config with CONFIG_ADDR/CONFIG_ADDR6 flag
> set.
> 
> This fixes a regression in 2.81 where using one dhcp-host
> entry for IPv4 and another for IPv6 no longer work.
> 
> Related: rhbz#1829448
> ---
>  src/dhcp-common.c | 48 +++++++++++++++++++++++++++++++++++++----------
>  src/dnsmasq.h     |  3 ++-
>  src/lease.c       |  2 +-
>  src/rfc2131.c     |  6 +++---
>  src/rfc3315.c     |  6 +++---
>  5 files changed, 47 insertions(+), 18 deletions(-)
> 
> diff --git a/src/dhcp-common.c b/src/dhcp-common.c
> index eae9886..1277fa6 100644
> --- a/src/dhcp-common.c
> +++ b/src/dhcp-common.c
> @@ -192,6 +192,18 @@ int match_netid(struct dhcp_netid *check, struct 
> dhcp_netid *pool, int tagnotnee
>    return 1;
>  }
>  
> +/* Is the config flag set. */
> +int match_flag(unsigned int flags, unsigned int flag, int flagnotneeded)
> +{
> +  if (flagnotneeded)
> +    return 1;
> +  
> +  if (flags & flag)
> +    return 1;
> +  
> +  return 0;
> +}
> +
>  /* return domain or NULL if none. */
>  char *strip_hostname(char *hostname)
>  {
> @@ -318,7 +330,8 @@ static struct dhcp_config *find_config_match(struct 
> dhcp_config *configs,
>                                            unsigned char *clid, int clid_len,
>                                            unsigned char *hwaddr, int hw_len, 
>                                            int hw_type, char *hostname,
> -                                          struct dhcp_netid *tags, int 
> tag_not_needed)
> +                                          struct dhcp_netid *tags, int 
> tag_not_needed,
> +               unsigned int flag, int flag_not_needed)
>  {
>    int count, new;
>    struct dhcp_config *config, *candidate; 
> @@ -331,7 +344,8 @@ static struct dhcp_config *find_config_match(struct 
> dhcp_config *configs,
>         if (config->clid_len == clid_len && 
>             memcmp(config->clid, clid, clid_len) == 0 &&
>             is_config_in_context(context, config) &&
> -           match_netid(config->filter, tags, tag_not_needed))
> +           match_netid(config->filter, tags, tag_not_needed) &&
> +        match_flag(config->flags, flag, flag_not_needed))
>           
>           return config;
>         
> @@ -341,7 +355,8 @@ static struct dhcp_config *find_config_match(struct 
> dhcp_config *configs,
>         if ((!context || !(context->flags & CONTEXT_V6)) && *clid == 0 && 
> config->clid_len == clid_len-1  &&
>             memcmp(config->clid, clid+1, clid_len-1) == 0 &&
>             is_config_in_context(context, config) &&
> -           match_netid(config->filter, tags, tag_not_needed))
> +           match_netid(config->filter, tags, tag_not_needed) &&
> +        match_flag(config->flags, flag, flag_not_needed))
>           return config;
>       }
>    
> @@ -350,7 +365,8 @@ static struct dhcp_config *find_config_match(struct 
> dhcp_config *configs,
>      for (config = configs; config; config = config->next)
>        if (config_has_mac(config, hwaddr, hw_len, hw_type) &&
>         is_config_in_context(context, config) &&
> -       match_netid(config->filter, tags, tag_not_needed))
> +       match_netid(config->filter, tags, tag_not_needed) &&
> +    match_flag(config->flags, flag, flag_not_needed))
>       return config;
>    
>    if (hostname && context)
> @@ -358,7 +374,8 @@ static struct dhcp_config *find_config_match(struct 
> dhcp_config *configs,
>        if ((config->flags & CONFIG_NAME) && 
>         hostname_isequal(config->hostname, hostname) &&
>         is_config_in_context(context, config) &&
> -       match_netid(config->filter, tags, tag_not_needed))
> +       match_netid(config->filter, tags, tag_not_needed) &&
> +    match_flag(config->flags, flag, flag_not_needed))
>       return config;
>  
>    
> @@ -368,7 +385,8 @@ static struct dhcp_config *find_config_match(struct 
> dhcp_config *configs,
>    /* use match with fewest wildcard octets */
>    for (candidate = NULL, count = 0, config = configs; config; config = 
> config->next)
>      if (is_config_in_context(context, config) &&
> -     match_netid(config->filter, tags, tag_not_needed))
> +     match_netid(config->filter, tags, tag_not_needed) &&
> +  match_flag(config->flags, flag, flag_not_needed))
>        for (conf_addr = config->hwaddr; conf_addr; conf_addr = 
> conf_addr->next)
>       if (conf_addr->wildcard_mask != 0 &&
>           conf_addr->hwaddr_len == hw_len &&  
> @@ -382,17 +400,27 @@ static struct dhcp_config *find_config_match(struct 
> dhcp_config *configs,
>    return candidate;
>  }
>  
> -/* Find tagged configs first. */
> +/* Find tagged configs with flags first. */
>  struct dhcp_config *find_config(struct dhcp_config *configs,
>                               struct dhcp_context *context,
>                               unsigned char *clid, int clid_len,
>                               unsigned char *hwaddr, int hw_len, 
> -                             int hw_type, char *hostname, struct dhcp_netid 
> *tags)
> +                             int hw_type, char *hostname, struct dhcp_netid 
> *tags, unsigned int flag)
>  {
> -  struct dhcp_config *ret = find_config_match(configs, context, clid, 
> clid_len, hwaddr, hw_len, hw_type, hostname, tags, 0);
> +  /* Find tagged config with flags */
> +  struct dhcp_config *ret = find_config_match(configs, context, clid, 
> clid_len, hwaddr, hw_len, hw_type, hostname, tags, 0, flag, 0);
> +
> +  /* Find tagged config without flags */
> +  if (!ret)
> +    ret = find_config_match(configs, context, clid, clid_len, hwaddr, 
> hw_len, hw_type, hostname, tags, 0, flag, 1);
> +
> +  /* Find untagged config with flags */
> +  if (!ret)
> +    ret = find_config_match(configs, context, clid, clid_len, hwaddr, 
> hw_len, hw_type, hostname, tags, 1, flag, 0);
>  
> +  /* Find untagged config without flags */
>    if (!ret)
> -    ret = find_config_match(configs, context, clid, clid_len, hwaddr, 
> hw_len, hw_type, hostname, tags, 1);
> +    ret = find_config_match(configs, context, clid, clid_len, hwaddr, 
> hw_len, hw_type, hostname, tags, 1, flag, 1);
>  
>    return ret;
>  }
> diff --git a/src/dnsmasq.h b/src/dnsmasq.h
> index 18c381e..bdb085b 100644
> --- a/src/dnsmasq.h
> +++ b/src/dnsmasq.h
> @@ -1571,7 +1571,8 @@ struct dhcp_config *find_config(struct dhcp_config 
> *configs,
>                               unsigned char *clid, int clid_len,
>                               unsigned char *hwaddr, int hw_len, 
>                               int hw_type, char *hostname,
> -                             struct dhcp_netid *filter);
> +                             struct dhcp_netid *filter,
> +        unsigned int flag);
>  int config_has_mac(struct dhcp_config *config, unsigned char *hwaddr, int 
> len, int type);
>  #ifdef HAVE_LINUX_NETWORK
>  char *whichdevice(void);
> diff --git a/src/lease.c b/src/lease.c
> index 23e6fe0..917f7f3 100644
> --- a/src/lease.c
> +++ b/src/lease.c
> @@ -230,7 +230,7 @@ void lease_update_from_configs(void)
>      if (lease->flags & (LEASE_TA | LEASE_NA))
>        continue;
>      else if ((config = find_config(daemon->dhcp_conf, NULL, lease->clid, 
> lease->clid_len, 
> -                                lease->hwaddr, lease->hwaddr_len, 
> lease->hwaddr_type, NULL, NULL)) && 
> +                                lease->hwaddr, lease->hwaddr_len, 
> lease->hwaddr_type, NULL, NULL, 0)) && 
>            (config->flags & CONFIG_NAME) &&
>            (!(config->flags & CONFIG_ADDR) || config->addr.s_addr == 
> lease->addr.s_addr))
>        lease_set_hostname(lease, config->hostname, 1, 
> get_domain(lease->addr), NULL);
> diff --git a/src/rfc2131.c b/src/rfc2131.c
> index fc54aab..22c2d63 100644
> --- a/src/rfc2131.c
> +++ b/src/rfc2131.c
> @@ -504,7 +504,7 @@ size_t dhcp_reply(struct dhcp_context *context, char 
> *iface_name, int int_index,
>    mess->op = BOOTREPLY;
>    
>    config = find_config(daemon->dhcp_conf, context, clid, clid_len, 
> -                    mess->chaddr, mess->hlen, mess->htype, NULL, 
> run_tag_if(netid));
> +                    mess->chaddr, mess->hlen, mess->htype, NULL, 
> run_tag_if(netid), CONFIG_ADDR);
>  
>    /* set "known" tag for known hosts */
>    if (config)
> @@ -514,7 +514,7 @@ size_t dhcp_reply(struct dhcp_context *context, char 
> *iface_name, int int_index,
>        netid = &known_id;
>      }
>    else if (find_config(daemon->dhcp_conf, NULL, clid, clid_len, 
> -                    mess->chaddr, mess->hlen, mess->htype, NULL, 
> run_tag_if(netid)))
> +                    mess->chaddr, mess->hlen, mess->htype, NULL, 
> run_tag_if(netid), CONFIG_ADDR))
>      {
>        known_id.net = "known-othernet";
>        known_id.next = netid;
> @@ -781,7 +781,7 @@ size_t dhcp_reply(struct dhcp_context *context, char 
> *iface_name, int int_index,
>                to avoid impersonation by name. */
>             struct dhcp_config *new = find_config(daemon->dhcp_conf, context, 
> NULL, 0,
>                                                   mess->chaddr, mess->hlen, 
> -                                                 mess->htype, hostname, 
> run_tag_if(netid));
> +                                                 mess->htype, hostname, 
> run_tag_if(netid), CONFIG_ADDR);
>             if (new && !have_config(new, CONFIG_CLID) && !new->hwaddr)
>               {
>                 config = new;
> diff --git a/src/rfc3315.c b/src/rfc3315.c
> index b3f0a0a..27b0611 100644
> --- a/src/rfc3315.c
> +++ b/src/rfc3315.c
> @@ -527,7 +527,7 @@ static int dhcp6_no_relay(struct state *state, int 
> msg_type, void *inbuff, size_
>    
>    if (state->clid &&
>        (config = find_config(daemon->dhcp_conf, state->context, state->clid, 
> state->clid_len,
> -                         state->mac, state->mac_len, state->mac_type, NULL, 
> run_tag_if(state->tags))) &&
> +                         state->mac, state->mac_len, state->mac_type, NULL, 
> run_tag_if(state->tags), CONFIG_ADDR6)) &&
>        have_config(config, CONFIG_NAME))
>      {
>        state->hostname = config->hostname;
> @@ -547,7 +547,7 @@ static int dhcp6_no_relay(struct state *state, int 
> msg_type, void *inbuff, size_
>             /* Search again now we have a hostname. 
>                Only accept configs without CLID here, (it won't match)
>                to avoid impersonation by name. */
> -           struct dhcp_config *new = find_config(daemon->dhcp_conf, 
> state->context, NULL, 0, NULL, 0, 0, state->hostname, 
> run_tag_if(state->tags));
> +           struct dhcp_config *new = find_config(daemon->dhcp_conf, 
> state->context, NULL, 0, NULL, 0, 0, state->hostname, 
> run_tag_if(state->tags), CONFIG_ADDR6);
>             if (new && !have_config(new, CONFIG_CLID) && !new->hwaddr)
>               config = new;
>           }
> @@ -574,7 +574,7 @@ static int dhcp6_no_relay(struct state *state, int 
> msg_type, void *inbuff, size_
>      }
>    else if (state->clid &&
>          find_config(daemon->dhcp_conf, NULL, state->clid, state->clid_len,
> -                    state->mac, state->mac_len, state->mac_type, NULL, 
> run_tag_if(state->tags)))
> +                    state->mac, state->mac_len, state->mac_type, NULL, 
> run_tag_if(state->tags), CONFIG_ADDR6))
>      {
>        known_id.net = "known-othernet";
>        known_id.next = state->tags;
> 

-- 
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
>From 046c87efa78fcfa7bf7de5a36bf496993a13696a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Fri, 1 May 2020 01:50:03 +0200
Subject: [PATCH 2/2] Add flags to find_config and match address family

Check also address family when looking for configuration for given
lease. Have the most important tagged configuration. Then try for
different address family and retry the same on common configuration
without tags.
---
 src/dhcp-common.c | 42 ++++++++++++++++++++++++++----------------
 src/dnsmasq.h     |  3 ++-
 src/lease.c       |  2 +-
 src/rfc2131.c     |  7 ++++---
 src/rfc3315.c     |  6 +++---
 5 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/src/dhcp-common.c b/src/dhcp-common.c
index bba34f5..4c74f05 100644
--- a/src/dhcp-common.c
+++ b/src/dhcp-common.c
@@ -313,23 +313,27 @@ static int is_config_in_context(struct dhcp_context *context, struct dhcp_config
   return 0;
 }
 
-/** Structure to help find_config_match reduce code repetitions */
+/** Structure to help find_config_match reduce code repetitions.
+ *
+ * It should improve also readibility. */
 struct dhcp_config_match_data {
   struct dhcp_context *context;
   struct dhcp_netid *tags;
   int tag_needed;
+  int flag_needed;
 };
 
 static int is_config_matching(struct dhcp_config_match_data *data, struct dhcp_config *config)
 {
   return (is_config_in_context(data->context, config) &&
-	  match_netid(config->filter, data->tags, !data->tag_needed));
+	  match_netid(config->filter, data->tags, !data->tag_needed) &&
+	  (!data->flag_needed || (config->flags & data->flag_needed)!=0));
 }
 
 static int is_config_matching_clid(struct dhcp_config_match_data *data, struct dhcp_config *config,
 				   unsigned char *clid, int clid_len)
 {
-  if (!(config->flags & CONFIG_CLID))
+  if ((config->flags & CONFIG_CLID) == 0)
     return 0;
 
   if (config->clid_len == clid_len &&
@@ -362,30 +366,28 @@ static int is_config_matching_hostname(struct dhcp_config_match_data *data, stru
 }
 
 static struct dhcp_config *find_config_match(struct dhcp_config *configs,
-					     struct dhcp_context *context,
 					     unsigned char *clid, int clid_len,
 					     unsigned char *hwaddr, int hw_len, 
 					     int hw_type, char *hostname,
-					     struct dhcp_netid *tags, int tag_not_needed)
+					     struct dhcp_config_match_data *data)
 {
   int count, new;
   struct dhcp_config *config, *candidate; 
   struct hwaddr_config *conf_addr;
-  struct dhcp_config_match_data data = { context, tags, !tag_not_needed };
 
   if (clid)
     for (config = configs; config; config = config->next)
-      if (is_config_matching_clid(&data, config, clid, clid_len))
+      if (clid && is_config_matching_clid(data, config, clid, clid_len))
 	return config;
 
   if (hwaddr)
     for (config = configs; config; config = config->next)
-      if (is_config_matching_hwaddr(&data, config, hwaddr, hw_len, hw_type))
+      if (is_config_matching_hwaddr(data, config, hwaddr, hw_len, hw_type))
 	return config;
   
-  if (hostname && context)
+  if (hostname && data->context)
     for (config = configs; config; config = config->next)
-      if (is_config_matching_hostname(&data, config, hostname))
+      if (is_config_matching_hostname(data, config, hostname))
 	return config;
   
   if (!hwaddr)
@@ -393,7 +395,7 @@ static struct dhcp_config *find_config_match(struct dhcp_config *configs,
 
   /* use match with fewest wildcard octets */
   for (candidate = NULL, count = 0, config = configs; config; config = config->next)
-    if (is_config_matching(&data, config))
+    if (is_config_matching(data, config))
       for (conf_addr = config->hwaddr; conf_addr; conf_addr = conf_addr->next)
 	if (conf_addr->wildcard_mask != 0 &&
 	    conf_addr->hwaddr_len == hw_len &&	
@@ -412,12 +414,20 @@ struct dhcp_config *find_config(struct dhcp_config *configs,
 				struct dhcp_context *context,
 				unsigned char *clid, int clid_len,
 				unsigned char *hwaddr, int hw_len, 
-				int hw_type, char *hostname, struct dhcp_netid *tags)
+				int hw_type, char *hostname, struct dhcp_netid *tags, int flags)
 {
-  struct dhcp_config *ret = find_config_match(configs, context, clid, clid_len, hwaddr, hw_len, hw_type, hostname, tags, 0);
-
-  if (!ret)
-    ret = find_config_match(configs, context, clid, clid_len, hwaddr, hw_len, hw_type, hostname, tags, 1);
+  struct dhcp_config_match_data data[] = {
+	{ context, tags, 1, flags }, // first try tagged host with the same address type
+	{ context, tags, 1, 0 },     // then tagged host with different address type
+	{ context, tags, 0, flags }, // find common host with the same address type
+	{ context, tags, 0, 0},      // find common host with different address as last resort
+  };
+  struct dhcp_config *ret = NULL;
+  const size_t variants = (sizeof(data)/sizeof(data[0]));
+  size_t i;
+
+  for (i=0; i<variants && !ret; i++)
+    ret = find_config_match(configs, clid, clid_len, hwaddr, hw_len, hw_type, hostname, &data[i]);
 
   return ret;
 }
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 18c381e..4769031 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1571,7 +1571,8 @@ struct dhcp_config *find_config(struct dhcp_config *configs,
 				unsigned char *clid, int clid_len,
 				unsigned char *hwaddr, int hw_len, 
 				int hw_type, char *hostname,
-				struct dhcp_netid *filter);
+				struct dhcp_netid *filter,
+				int flags);
 int config_has_mac(struct dhcp_config *config, unsigned char *hwaddr, int len, int type);
 #ifdef HAVE_LINUX_NETWORK
 char *whichdevice(void);
diff --git a/src/lease.c b/src/lease.c
index 23e6fe0..917f7f3 100644
--- a/src/lease.c
+++ b/src/lease.c
@@ -230,7 +230,7 @@ void lease_update_from_configs(void)
     if (lease->flags & (LEASE_TA | LEASE_NA))
       continue;
     else if ((config = find_config(daemon->dhcp_conf, NULL, lease->clid, lease->clid_len, 
-				   lease->hwaddr, lease->hwaddr_len, lease->hwaddr_type, NULL, NULL)) && 
+				   lease->hwaddr, lease->hwaddr_len, lease->hwaddr_type, NULL, NULL, 0)) && 
 	     (config->flags & CONFIG_NAME) &&
 	     (!(config->flags & CONFIG_ADDR) || config->addr.s_addr == lease->addr.s_addr))
       lease_set_hostname(lease, config->hostname, 1, get_domain(lease->addr), NULL);
diff --git a/src/rfc2131.c b/src/rfc2131.c
index fc54aab..6b55c83 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -504,7 +504,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
   mess->op = BOOTREPLY;
   
   config = find_config(daemon->dhcp_conf, context, clid, clid_len, 
-		       mess->chaddr, mess->hlen, mess->htype, NULL, run_tag_if(netid));
+		       mess->chaddr, mess->hlen, mess->htype, NULL, run_tag_if(netid), CONFIG_ADDR);
 
   /* set "known" tag for known hosts */
   if (config)
@@ -514,7 +514,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
       netid = &known_id;
     }
   else if (find_config(daemon->dhcp_conf, NULL, clid, clid_len, 
-		       mess->chaddr, mess->hlen, mess->htype, NULL, run_tag_if(netid)))
+		       mess->chaddr, mess->hlen, mess->htype, NULL, run_tag_if(netid), CONFIG_ADDR))
     {
       known_id.net = "known-othernet";
       known_id.next = netid;
@@ -781,7 +781,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 		 to avoid impersonation by name. */
 	      struct dhcp_config *new = find_config(daemon->dhcp_conf, context, NULL, 0,
 						    mess->chaddr, mess->hlen, 
-						    mess->htype, hostname, run_tag_if(netid));
+						    mess->htype, hostname, run_tag_if(netid),
+						    CONFIG_ADDR);
 	      if (new && !have_config(new, CONFIG_CLID) && !new->hwaddr)
 		{
 		  config = new;
diff --git a/src/rfc3315.c b/src/rfc3315.c
index b3f0a0a..27b0611 100644
--- a/src/rfc3315.c
+++ b/src/rfc3315.c
@@ -527,7 +527,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, void *inbuff, size_
   
   if (state->clid &&
       (config = find_config(daemon->dhcp_conf, state->context, state->clid, state->clid_len,
-			    state->mac, state->mac_len, state->mac_type, NULL, run_tag_if(state->tags))) &&
+			    state->mac, state->mac_len, state->mac_type, NULL, run_tag_if(state->tags), CONFIG_ADDR6)) &&
       have_config(config, CONFIG_NAME))
     {
       state->hostname = config->hostname;
@@ -547,7 +547,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, void *inbuff, size_
 	      /* Search again now we have a hostname. 
 		 Only accept configs without CLID here, (it won't match)
 		 to avoid impersonation by name. */
-	      struct dhcp_config *new = find_config(daemon->dhcp_conf, state->context, NULL, 0, NULL, 0, 0, state->hostname, run_tag_if(state->tags));
+	      struct dhcp_config *new = find_config(daemon->dhcp_conf, state->context, NULL, 0, NULL, 0, 0, state->hostname, run_tag_if(state->tags), CONFIG_ADDR6);
 	      if (new && !have_config(new, CONFIG_CLID) && !new->hwaddr)
 		config = new;
 	    }
@@ -574,7 +574,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, void *inbuff, size_
     }
   else if (state->clid &&
 	   find_config(daemon->dhcp_conf, NULL, state->clid, state->clid_len,
-		       state->mac, state->mac_len, state->mac_type, NULL, run_tag_if(state->tags)))
+		       state->mac, state->mac_len, state->mac_type, NULL, run_tag_if(state->tags), CONFIG_ADDR6))
     {
       known_id.net = "known-othernet";
       known_id.next = state->tags;
-- 
2.21.1

>From 0faeca909690fef5e83c4ff7704f4742f69d9407 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Fri, 1 May 2020 00:49:31 +0200
Subject: [PATCH 1/2] Break find_config_match into more functions

Reduce code repetitions and make main body shorter.
Reveals more obvious reiterating over the configs list again and again.
Better algorithm should be used for best candidate.
---
 src/dhcp-common.c | 83 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 54 insertions(+), 29 deletions(-)

diff --git a/src/dhcp-common.c b/src/dhcp-common.c
index eae9886..bba34f5 100644
--- a/src/dhcp-common.c
+++ b/src/dhcp-common.c
@@ -313,6 +313,54 @@ static int is_config_in_context(struct dhcp_context *context, struct dhcp_config
   return 0;
 }
 
+/** Structure to help find_config_match reduce code repetitions */
+struct dhcp_config_match_data {
+  struct dhcp_context *context;
+  struct dhcp_netid *tags;
+  int tag_needed;
+};
+
+static int is_config_matching(struct dhcp_config_match_data *data, struct dhcp_config *config)
+{
+  return (is_config_in_context(data->context, config) &&
+	  match_netid(config->filter, data->tags, !data->tag_needed));
+}
+
+static int is_config_matching_clid(struct dhcp_config_match_data *data, struct dhcp_config *config,
+				   unsigned char *clid, int clid_len)
+{
+  if (!(config->flags & CONFIG_CLID))
+    return 0;
+
+  if (config->clid_len == clid_len &&
+      memcmp(config->clid, clid, clid_len) == 0 &&
+      is_config_matching(data, config))
+
+    return 1;
+
+  /* dhcpcd prefixes ASCII client IDs by zero which is wrong, but we try and
+     cope with that here. This is IPv4 only. context==NULL implies IPv4,
+     see lease_update_from_configs() */
+  return ((!data->context || !(data->context->flags & CONTEXT_V6)) && *clid == 0 && config->clid_len == clid_len-1  &&
+	  memcmp(config->clid, clid+1, clid_len-1) == 0 &&
+	  is_config_matching(data, config));
+}
+
+static int is_config_matching_hwaddr(struct dhcp_config_match_data *data, struct dhcp_config *config,
+				   unsigned char *hwaddr, int hw_len, int hw_type)
+{
+  return (config_has_mac(config, hwaddr, hw_len, hw_type) &&
+	  is_config_matching(data, config));
+}
+
+static int is_config_matching_hostname(struct dhcp_config_match_data *data, struct dhcp_config *config,
+					const char *hostname)
+{
+  return ((config->flags & CONFIG_NAME) &&
+	  hostname_isequal(config->hostname, hostname) &&
+	  is_config_matching(data, config));
+}
+
 static struct dhcp_config *find_config_match(struct dhcp_config *configs,
 					     struct dhcp_context *context,
 					     unsigned char *clid, int clid_len,
@@ -323,52 +371,29 @@ static struct dhcp_config *find_config_match(struct dhcp_config *configs,
   int count, new;
   struct dhcp_config *config, *candidate; 
   struct hwaddr_config *conf_addr;
+  struct dhcp_config_match_data data = { context, tags, !tag_not_needed };
 
   if (clid)
     for (config = configs; config; config = config->next)
-      if (config->flags & CONFIG_CLID)
-	{
-	  if (config->clid_len == clid_len && 
-	      memcmp(config->clid, clid, clid_len) == 0 &&
-	      is_config_in_context(context, config) &&
-	      match_netid(config->filter, tags, tag_not_needed))
-	    
-	    return config;
-	  
-	  /* dhcpcd prefixes ASCII client IDs by zero which is wrong, but we try and
-	     cope with that here. This is IPv4 only. context==NULL implies IPv4, 
-	     see lease_update_from_configs() */
-	  if ((!context || !(context->flags & CONTEXT_V6)) && *clid == 0 && config->clid_len == clid_len-1  &&
-	      memcmp(config->clid, clid+1, clid_len-1) == 0 &&
-	      is_config_in_context(context, config) &&
-	      match_netid(config->filter, tags, tag_not_needed))
-	    return config;
-	}
-  
+      if (is_config_matching_clid(&data, config, clid, clid_len))
+	return config;
 
   if (hwaddr)
     for (config = configs; config; config = config->next)
-      if (config_has_mac(config, hwaddr, hw_len, hw_type) &&
-	  is_config_in_context(context, config) &&
-	  match_netid(config->filter, tags, tag_not_needed))
+      if (is_config_matching_hwaddr(&data, config, hwaddr, hw_len, hw_type))
 	return config;
   
   if (hostname && context)
     for (config = configs; config; config = config->next)
-      if ((config->flags & CONFIG_NAME) && 
-	  hostname_isequal(config->hostname, hostname) &&
-	  is_config_in_context(context, config) &&
-	  match_netid(config->filter, tags, tag_not_needed))
+      if (is_config_matching_hostname(&data, config, hostname))
 	return config;
-
   
   if (!hwaddr)
     return NULL;
 
   /* use match with fewest wildcard octets */
   for (candidate = NULL, count = 0, config = configs; config; config = config->next)
-    if (is_config_in_context(context, config) &&
-	match_netid(config->filter, tags, tag_not_needed))
+    if (is_config_matching(&data, config))
       for (conf_addr = config->hwaddr; conf_addr; conf_addr = conf_addr->next)
 	if (conf_addr->wildcard_mask != 0 &&
 	    conf_addr->hwaddr_len == hw_len &&	
-- 
2.21.1

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Reply via email to