On Sun, Nov 08, 2015 at 02:29:11PM +0100, Alexander Bluhm wrote:
> On Wed, Oct 28, 2015 at 06:24:04PM +0100, Alexandr Nedvedicky wrote:
> > Index: usr.sbin/bgpd/pftable.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/pftable.c,v
> > retrieving revision 1.8
> > diff -u -p -r1.8 pftable.c
> > --- usr.sbin/bgpd/pftable.c 21 Jan 2015 21:50:32 -0000 1.8
> > +++ usr.sbin/bgpd/pftable.c 27 Oct 2015 23:54:49 -0000
> > @@ -57,6 +57,8 @@ pftable_change(struct pf_table *pft)
> > {
> > struct pfioc_table tio;
> > int ret;
> > + int i;
> > + struct pfr_addr *addr;
> >
> > if (pft->naddrs == 0 || pft->what == 0)
> > return (0);
> > @@ -67,11 +69,15 @@ pftable_change(struct pf_table *pft)
> > bzero(&tio, sizeof(tio));
> > strlcpy(tio.pfrio_table.pfrt_name, pft->name,
> > sizeof(tio.pfrio_table.pfrt_name));
> > - tio.pfrio_buffer = pft->worklist;
> > tio.pfrio_esize = sizeof(*pft->worklist);
> > - tio.pfrio_size = pft->naddrs;
> > + tio.pfrio_size = 1;
> >
> > ret = ioctl(devpf, pft->what, &tio);
>
> This ioctl() uses an pfrio_buffer with 0.
>
> > + addr = pft->worklist;
> > + for (i = 0; (i < pft->naddrs) && (ret == 0); i++) {
> > + tio.pfrio_buffer = addr++;
> > + ret = ioctl(devpf, pft->what, &tio);
> > + }
> >
> > /* bad prefixes shouldn't cause us to die */
> > if (ret == -1) {
>
> Perhaps we should not abort the loop on the first failure. Can we
> try to add all addresses and log a warning for each one that fails.
>
> The caller expects that pftable_change() is atomic. I am unsure
> what we should do in case of partial failure. Now the caller ignores
> the partiall success.
>
yes, that's true. I think SIOCADDADDRS will have to come back. The idea is to
use pfr_add_addr() as a backend for SIOCADDADDRS ioctl. The for() loop will be
in kernel. As I've said in other email, I'm working on that prototype
currently.
regards
sasha
