[Dnsmasq-discuss] [PATCH] Netfilter IPSet Support

[Simon Kelley]

On 16/02/13 03:56, Jason A. Donenfeld wrote:
> Hah, forget that. I just wrote the entire thing, including configuration,
> and it works great. I'll send full patches to the mailing list shortly.
>
>

Fantastic! I'm beating my way though my my inbox and just caught up with 
you. I was intending to put this off to the first thing in 2.67, rather 
than the last thing in 2.66, but having got this far, I'm not so sure.

You even wrote the man-page text!

Queries which arise mainly from a fresh pair of eyes:

1)  Do we need to support HAVE_OLD_IPSET? I can see more reason for 2.4 
kernels that 2.6, but I think most routers are using modern kernels now? 
If we do need to support both, I'd like to do both in one binary, just 
to reduce the potential build confusion. Am I right that the setsockopt 
method only supports IPv4? So we can't support just that.


2) Can we get by without adding a dependency on libmnl? Do we want to?
    even if we don't, we can replace the code which starts

mnl = mnl_socket_open(NETLINK_NETFILTER);

and ends

mnl_socket_close(mnl);

with something which uses the already-existing netlink socket created in 
src/netlink.c, and avoid the overhead of making a new netlink socket on 
each iteration?  Worry about process_reply() being called from the 
TCP-request code-path, from a forked process. I think the netlink socket 
inherited by the forked process will still be OK.

3)
(af == AF_INET ? sizeof(ipaddr->addr.addr4) : sizeof(ipaddr->addr.addr4)),

second "addr4" should be "addr6"?

4) The code should compile if HAVE_IPV6 is not set even if the headers 
don't define AF_INET6

5) Compile-time options need to be added to the prepocessor stuff 
defining the compile_opts string in src/config.c

Cheers,

Simon.