[Dnsmasq-discuss] [PATCH] Regression in 2.81 related to support for multiple IPv6 addresses
Harald Jensas
hjensas at redhat.com
Fri May 1 15:52:56 BST 2020
On Fri, 1 May 2020, 16:28 Petr Menšík, <pemensik at redhat.com> wrote:
> 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
>
Hi Petr,
I looked at the patches, I think the refactoring looks good.
I am slightly worried about the sorting, I know openstack neutron does
sorting to ensure IPv4 records come before records without address (IPv6
stateless). Look at the docstring[1] for details. In the future I would
like to use tag:dhcpv6 in neutron instead of sorting, but that is'nt really
possible before most distros have a version of dnsmasq with support for
tags in host definitions.
Today is a public holiday for me, and I am away from computers. I will do
some testing as soon as I am back home.
[1]
https://opendev.org/openstack/neutron/src/branch/master/neutron/agent/linux/dhcp.py#L572
//
Harald
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 <hjensas at 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: pemensik at redhat.com
> PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
> _______________________________________________
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss at lists.thekelleys.org.uk
> http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20200501/e61eaf8b/attachment-0001.html>
More information about the Dnsmasq-discuss
mailing list