Re: [OpenWrt-Devel] [PATCH] dnsmasq: allow de-selecting features from -full variant.

2014-12-17 Thread Frank Schäfer
Hi yousong,

thanks for the patch.
A few issues/comments:

Am 17.12.2014 um 02:21 schrieb Yousong Zhou:
 Signed-off-by: Yousong Zhou yszhou4t...@gmail.com
 ---
  package/network/services/dnsmasq/Makefile  |   27 
 ++--
  .../network/services/dnsmasq/files/dnsmasq.init|5 
  2 files changed, 30 insertions(+), 2 deletions(-)

 diff --git a/package/network/services/dnsmasq/Makefile 
 b/package/network/services/dnsmasq/Makefile
 index a530225..2da593d 100644
 --- a/package/network/services/dnsmasq/Makefile
 +++ b/package/network/services/dnsmasq/Makefile
 @@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
  
  PKG_NAME:=dnsmasq
  PKG_VERSION:=2.72
 -PKG_RELEASE:=1
 +PKG_RELEASE:=2
  
  PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
  PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq
 @@ -72,6 +72,26 @@ define Package/dnsmasq/conffiles
  /etc/dnsmasq.conf
  endef
  
 +define Package/dnsmasq/config/Default
 +  if PACKAGE_$(1)-$(2)
 +  config PACKAGE_dnsmasq_$(2)_dhcpv6
 +bool Build with DHCPv6 support.
 +default y
This config needs to depend on IPV6

 +  config PACKAGE_dnsmasq_$(2)_dnssec
 +bool Build with DNSSEC support.
 +default y
This config needs to select PACKAGE_libnettle

 +  config PACKAGE_dnsmasq_$(2)_auth
 +bool Build with the facility to act as an authoritative DNS server.
 +default y
 +  config PACKAGE_dnsmasq_$(2)_ipset
 +bool Build with ipset support.
 +select PACKAGE_kmod-ipt-ipset
 +default y
 +  endif
 +endef
It would be nice if this would appear in a submenu and only if the
dnsmasq package is enabled.

 +
 +Package/dnsmasq-full/config=$(call 
 Package/dnsmasq/config/Default,dnsmasq,full)
 +
  Package/dnsmasq-dhcpv6/conffiles = $(Package/dnsmasq/conffiles)
  Package/dnsmasq-full/conffiles = $(Package/dnsmasq/conffiles)
  
 @@ -85,7 +105,10 @@ ifeq ($(BUILD_VARIANT),nodhcpv6)
  endif
  
  ifeq ($(BUILD_VARIANT),full)
 - COPTS += -DHAVE_DNSSEC
 + COPTS += $(if 
 $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_dhcpv6),,-DNO_DHCP6) \
 + $(if 
 $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_dnssec),-DHAVE_DNSSEC) \
 + $(if 
 $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_auth),,-DNO_AUTH) \
 + $(if 
 $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_ipset),,-DNO_IPSET)
   COPTS += $(if $(CONFIG_LIBNETTLE_MINI),-DNO_GMP,)
  else
   COPTS += -DNO_AUTH -DNO_IPSET
The installation section Package/dnsmasq-full/install should be
modified, too:
if DNSSEC is not enabled, there's no need to install trust-anchors.conf

---

The patch leaves the main problem unsolved:
you still have to enable the whole IPv6 stuff for DNS Auth / DNSSEC /
IPSET support.

I personally have no problems with migrating from package variants to
the configure at build time approach.
But I don't think they should be mixed together.
The dnsmasq-full package with only dhcpv6 enabled is exactly the same
as the dnsmasq-dhcpv6 package variant.
And as soon as you deselect one of the config options, dnsmasq-full
actually isn't a full package anymore, right ? ;-)
IMHO too much inconsistencies.
If we go this way, I would prefer having just a single (fully
configurable) dnsmasq package.

I'll send a patch later this evening.

Regards,
Frank

 diff --git a/package/network/services/dnsmasq/files/dnsmasq.init 
 b/package/network/services/dnsmasq/files/dnsmasq.init
 index 942acd7..209952b 100644
 --- a/package/network/services/dnsmasq/files/dnsmasq.init
 +++ b/package/network/services/dnsmasq/files/dnsmasq.init
 @@ -85,6 +85,10 @@ append_address() {
   xappend --address=$1
  }
  
 +append_ipset() {
 + xappend --ipset=$1
 +}
 +
  append_interface() {
   local ifname=$(uci_get_state network $1 ifname $1)
   xappend --interface=$ifname
 @@ -135,6 +139,7 @@ dnsmasq() {
   append_parm $cfg local --server
   config_list_foreach $cfg server append_server
   config_list_foreach $cfg address append_address
 + config_list_foreach $cfg ipset append_ipset
   config_list_foreach $cfg interface append_interface
   config_list_foreach $cfg notinterface append_notinterface
   config_list_foreach $cfg addnhosts append_addnhosts
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] dnsmasq: allow de-selecting features from -full variant.

2014-12-17 Thread Yousong Zhou
Hi, Frank.

On 18 December 2014 at 01:49, Frank Schäfer
fschaefer@googlemail.com wrote:
 Hi yousong,

 thanks for the patch.
 A few issues/comments:

 Am 17.12.2014 um 02:21 schrieb Yousong Zhou:
 Signed-off-by: Yousong Zhou yszhou4t...@gmail.com
 ---
  package/network/services/dnsmasq/Makefile  |   27 
 ++--
  .../network/services/dnsmasq/files/dnsmasq.init|5 
  2 files changed, 30 insertions(+), 2 deletions(-)

 diff --git a/package/network/services/dnsmasq/Makefile 
 b/package/network/services/dnsmasq/Makefile
 index a530225..2da593d 100644
 --- a/package/network/services/dnsmasq/Makefile
 +++ b/package/network/services/dnsmasq/Makefile
 @@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk

  PKG_NAME:=dnsmasq
  PKG_VERSION:=2.72
 -PKG_RELEASE:=1
 +PKG_RELEASE:=2

  PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
  PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq
 @@ -72,6 +72,26 @@ define Package/dnsmasq/conffiles
  /etc/dnsmasq.conf
  endef

 +define Package/dnsmasq/config/Default
 +  if PACKAGE_$(1)-$(2)
 +  config PACKAGE_dnsmasq_$(2)_dhcpv6
 +bool Build with DHCPv6 support.
 +default y
 This config needs to depend on IPV6

 +  config PACKAGE_dnsmasq_$(2)_dnssec
 +bool Build with DNSSEC support.
 +default y
 This config needs to select PACKAGE_libnettle

 +  config PACKAGE_dnsmasq_$(2)_auth
 +bool Build with the facility to act as an authoritative DNS server.
 +default y
 +  config PACKAGE_dnsmasq_$(2)_ipset
 +bool Build with ipset support.
 +select PACKAGE_kmod-ipt-ipset
 +default y
 +  endif
 +endef
 It would be nice if this would appear in a submenu and only if the
 dnsmasq package is enabled.

This currently only appear if dnsmasq-full is selected.


 +
 +Package/dnsmasq-full/config=$(call 
 Package/dnsmasq/config/Default,dnsmasq,full)
 +
  Package/dnsmasq-dhcpv6/conffiles = $(Package/dnsmasq/conffiles)
  Package/dnsmasq-full/conffiles = $(Package/dnsmasq/conffiles)

 @@ -85,7 +105,10 @@ ifeq ($(BUILD_VARIANT),nodhcpv6)
  endif

  ifeq ($(BUILD_VARIANT),full)
 - COPTS += -DHAVE_DNSSEC
 + COPTS += $(if 
 $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_dhcpv6),,-DNO_DHCP6) \
 + $(if 
 $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_dnssec),-DHAVE_DNSSEC) \
 + $(if 
 $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_auth),,-DNO_AUTH) \
 + $(if 
 $(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_ipset),,-DNO_IPSET)
   COPTS += $(if $(CONFIG_LIBNETTLE_MINI),-DNO_GMP,)
  else
   COPTS += -DNO_AUTH -DNO_IPSET
 The installation section Package/dnsmasq-full/install should be
 modified, too:
 if DNSSEC is not enabled, there's no need to install trust-anchors.conf

 ---

 The patch leaves the main problem unsolved:
 you still have to enable the whole IPv6 stuff for DNS Auth / DNSSEC /
 IPSET support.

 I personally have no problems with migrating from package variants to
 the configure at build time approach.
 But I don't think they should be mixed together.
 The dnsmasq-full package with only dhcpv6 enabled is exactly the same
 as the dnsmasq-dhcpv6 package variant.
 And as soon as you deselect one of the config options, dnsmasq-full
 actually isn't a full package anymore, right ? ;-)
 IMHO too much inconsistencies.
 If we go this way, I would prefer having just a single (fully
 configurable) dnsmasq package.

Another configurable variant should be better.  I just noticed that if
dnsmasq-full is selected, all of its dependencies like kmod-ipv6 and
libnettle are still selected and compiled in even when IPV6 and dnssec
are de-selected.


 I'll send a patch later this evening.

Thank you for the comments.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] [PATCH] dnsmasq: allow de-selecting features from -full variant.

2014-12-16 Thread Yousong Zhou

Signed-off-by: Yousong Zhou yszhou4t...@gmail.com
---
 package/network/services/dnsmasq/Makefile  |   27 ++--
 .../network/services/dnsmasq/files/dnsmasq.init|5 
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/package/network/services/dnsmasq/Makefile 
b/package/network/services/dnsmasq/Makefile
index a530225..2da593d 100644
--- a/package/network/services/dnsmasq/Makefile
+++ b/package/network/services/dnsmasq/Makefile
@@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk
 
 PKG_NAME:=dnsmasq
 PKG_VERSION:=2.72
-PKG_RELEASE:=1
+PKG_RELEASE:=2
 
 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
 PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq
@@ -72,6 +72,26 @@ define Package/dnsmasq/conffiles
 /etc/dnsmasq.conf
 endef
 
+define Package/dnsmasq/config/Default
+  if PACKAGE_$(1)-$(2)
+  config PACKAGE_dnsmasq_$(2)_dhcpv6
+bool Build with DHCPv6 support.
+default y
+  config PACKAGE_dnsmasq_$(2)_dnssec
+bool Build with DNSSEC support.
+default y
+  config PACKAGE_dnsmasq_$(2)_auth
+bool Build with the facility to act as an authoritative DNS server.
+default y
+  config PACKAGE_dnsmasq_$(2)_ipset
+bool Build with ipset support.
+select PACKAGE_kmod-ipt-ipset
+default y
+  endif
+endef
+
+Package/dnsmasq-full/config=$(call Package/dnsmasq/config/Default,dnsmasq,full)
+
 Package/dnsmasq-dhcpv6/conffiles = $(Package/dnsmasq/conffiles)
 Package/dnsmasq-full/conffiles = $(Package/dnsmasq/conffiles)
 
@@ -85,7 +105,10 @@ ifeq ($(BUILD_VARIANT),nodhcpv6)
 endif
 
 ifeq ($(BUILD_VARIANT),full)
-   COPTS += -DHAVE_DNSSEC
+   COPTS += $(if 
$(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_dhcpv6),,-DNO_DHCP6) \
+   $(if 
$(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_dnssec),-DHAVE_DNSSEC) \
+   $(if 
$(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_auth),,-DNO_AUTH) \
+   $(if 
$(CONFIG_PACKAGE_dnsmasq_$(BUILD_VARIANT)_ipset),,-DNO_IPSET)
COPTS += $(if $(CONFIG_LIBNETTLE_MINI),-DNO_GMP,)
 else
COPTS += -DNO_AUTH -DNO_IPSET
diff --git a/package/network/services/dnsmasq/files/dnsmasq.init 
b/package/network/services/dnsmasq/files/dnsmasq.init
index 942acd7..209952b 100644
--- a/package/network/services/dnsmasq/files/dnsmasq.init
+++ b/package/network/services/dnsmasq/files/dnsmasq.init
@@ -85,6 +85,10 @@ append_address() {
xappend --address=$1
 }
 
+append_ipset() {
+   xappend --ipset=$1
+}
+
 append_interface() {
local ifname=$(uci_get_state network $1 ifname $1)
xappend --interface=$ifname
@@ -135,6 +139,7 @@ dnsmasq() {
append_parm $cfg local --server
config_list_foreach $cfg server append_server
config_list_foreach $cfg address append_address
+   config_list_foreach $cfg ipset append_ipset
config_list_foreach $cfg interface append_interface
config_list_foreach $cfg notinterface append_notinterface
config_list_foreach $cfg addnhosts append_addnhosts
-- 
1.7.10.4
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH] dnsmasq: allow de-selecting features from -full variant.

2014-12-16 Thread Steven Barth

applied, thanks.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel