[Dnsmasq-discuss] [PATCH] Netfilter IPSet Support

[Jason A. Donenfeld]

On Sun, Feb 17, 2013 at 6:28 PM, Simon Kelley <simon at thekelleys.org.uk>wrote:
>
> 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.
>

Wonderful!


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

We really do need to support old ipset. Lots of broadcom-based routers run
2.6.22, because that's the kernel version broadcom has released their
drivers for. They're extremely widespread, so supporting it is really a
must.

If we do need to support both, I'd like to do both in one binary, just to
> reduce the potential build confusion.


This shouldn't be _too big_ of a problem. I'm rewriting things now to check
utsname on the first invocation of the function, and then route
accordingly. The only potential problem is that since we can't rely on
the existence of the ipset-netlink headers on older systems, I'll have to
import some of those constant values.

Personally I think it's much much much cleaner to have them separate
compile time options, but I understand the build confusion issue too.


> Am I right that the setsockopt method only supports IPv4? So we can't
> support just that.


Older ipsets only support IPv4. They're still extremely useful, but it's a
limitation.



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

The ipset userland tool uses libmnl, so it's already on the user's system
if they're using ipset. However, this isn't the case if they're using old
ipset, so it might be best to move away from libmnl. I'm investigating that
right now.


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

If we do stick with libmnl, implementing this will be no problem.


>
> 3)
> (af == AF_INET ? sizeof(ipaddr->addr.addr4) : sizeof(ipaddr->addr.addr4)),
>
> second "addr4" should be "addr6"?
>

Good catch.


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

Roger that.


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

I actually added this after the initial email in the git repository --
http://git.zx2c4.com/dnsmasq-ipset/ .


>
> Cheers,
>
> Simon.
>
> ______________________________**_________________
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss at lists.**thekelleys.org.uk<Dnsmasq-discuss at lists.thekelleys.org.uk>
> http://lists.thekelleys.org.**uk/mailman/listinfo/dnsmasq-**discuss<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/20130217/8bbf8dcb/attachment.html>