> On 09.11.2015, at 22:21, Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com> 
> wrote:
> 
> On Sun, Nov 08, 2015 at 01:18:22PM +0100, Alexander Bluhm wrote:
>> On Sun, Nov 08, 2015 at 02:37:58AM +0100, Alexander Bluhm wrote:
>>>> +  for (i = 0; (i < size) && (rv == 0); i++) {
>>> 
>>> rv is unitialized in the first interation
>>> 
>>>> +          io.pfrio_buffer = addr++;
>>>> +          rv = ioctl(dev, DIOCRADDADDR, &io);
>>> 
>>> I would suggest to return (-1) if ioctl fails...
>>> 
>>>> +          add++;
>>>> +  }
>> 
>> To keep the illusion of an atomic operation, we could remove the
>> addresses we just added before the one add failed.
>> 
> 
> actually pfctl_radix.c is just tip of the iceberg,  there are other tools than
> pfctl, which manipulate with PF-tables:
>       authpf
>       bgpd
>       pfutils
> 
> The more I'm thinking about s/SIOCADDADDRS/SIOCADDADDR the less I like it.  I
> feel good about s/pfr_add_addrs/pfr_add_addr. The pfr_add_addr() should be a
> back end for SIOCADDADDRS ioctl operation, which I think should go back.  The
> ioctl in kernel will iterate over the array of addresses coming from userland.
> It seems to me as more convenient approach. I'm working on prototype, I
> hope I'll send updated patches soon.
> 

I'm wondering - how does it affect tools that load several thousands of IPs 
into a table?
Like spamd, bgpd (for spam lists etc.), or pfctl for IP black lists (as 
distributed by ET).

There are valid use cases with HUGE tables, but I have to admit that I didn't 
test your
diff yet. Just a concern that loading IPs one after another might take forever.

Reyk

Reply via email to