> 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