On Wed, Oct 28, 2015 at 06:21:10PM +0100, Alexandr Nedvedicky wrote:
> Index: sbin/pfctl/pfctl_radix.c
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/pfctl_radix.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 pfctl_radix.c
> --- sbin/pfctl/pfctl_radix.c 21 Jan 2015 21:50:33 -0000 1.32
> +++ sbin/pfctl/pfctl_radix.c 27 Oct 2015 22:56:54 -0000
> @@ -207,6 +207,7 @@ pfr_del_addrs(struct pfr_table *tbl, str
> int *ndel, int flags)
> {
> struct pfioc_table io;
> + int i, rv, del = 0;
>
> if (tbl == NULL || size < 0 || (size && addr == NULL)) {
> errno = EINVAL;
> @@ -215,14 +216,18 @@ pfr_del_addrs(struct pfr_table *tbl, str
> bzero(&io, sizeof io);
> io.pfrio_flags = flags;
> io.pfrio_table = *tbl;
> - io.pfrio_buffer = addr;
> io.pfrio_esize = sizeof(*addr);
> - io.pfrio_size = size;
> - if (ioctl(dev, DIOCRDELADDRS, &io))
> - return (-1);
> - if (ndel != NULL)
> - *ndel = io.pfrio_ndel;
> - return (0);
> + io.pfrio_size = 1;
> + for (i = 0; (i < size) && (rv == 0); i++) {
rv may be unitialized
> + io.pfrio_buffer = addr++;
> + rv = ioctl(dev, DIOCRDELADDR, &io);
> + del++;
> + }
> +
> + if ((rv == 0) && (ndel != NULL))
> + *ndel = del;
> +
> + return (rv);
> }
>
> int
> Index: sys/net/pf_ioctl.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> retrieving revision 1.291
> diff -u -p -r1.291 pf_ioctl.c
> --- sys/net/pf_ioctl.c 13 Oct 2015 19:32:31 -0000 1.291
> +++ sys/net/pf_ioctl.c 27 Oct 2015 22:57:20 -0000
> @@ -835,7 +835,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> case DIOCRCLRTSTATS:
> case DIOCRCLRADDRS:
> case DIOCRADDADDRS:
> - case DIOCRDELADDRS:
> + case DIOCRDELADDR:
> case DIOCRSETADDRS:
> case DIOCRGETASTATS:
> case DIOCRCLRASTATS:
> @@ -888,7 +888,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> case DIOCRCLRTSTATS:
> case DIOCRCLRADDRS:
> case DIOCRADDADDRS:
> - case DIOCRDELADDRS:
> + case DIOCRDELADDR:
> case DIOCRSETADDRS:
> case DIOCRSETTFLAGS:
> if (((struct pfioc_table *)addr)->pfrio_flags &
> @@ -1829,16 +1829,15 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> break;
> }
>
> - case DIOCRDELADDRS: {
> + case DIOCRDELADDR: {
> struct pfioc_table *io = (struct pfioc_table *)addr;
>
> if (io->pfrio_esize != sizeof(struct pfr_addr)) {
> error = ENODEV;
> break;
> }
> - error = pfr_del_addrs(&io->pfrio_table, io->pfrio_buffer,
> - io->pfrio_size, &io->pfrio_ndel, io->pfrio_flags |
> - PFR_FLAG_USERIOCTL);
> + error = pfr_del_addr(&io->pfrio_table, io->pfrio_buffer,
> + io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
Don't pass io->pfrio_size.
> break;
> }
>
> Index: sys/net/pf_table.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf_table.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 pf_table.c
> --- sys/net/pf_table.c 7 Oct 2015 11:57:44 -0000 1.115
> +++ sys/net/pf_table.c 27 Oct 2015 22:57:24 -0000
> @@ -152,6 +152,8 @@ void pfr_destroy_kentries(struct
> pfr_
> void pfr_destroy_kentry(struct pfr_kentry *);
> void pfr_insert_kentries(struct pfr_ktable *,
> struct pfr_kentryworkq *, time_t);
> +void pfr_remove_kentry(struct pfr_ktable *,
> + struct pfr_kentry *);
> void pfr_remove_kentries(struct pfr_ktable *,
> struct pfr_kentryworkq *);
> void pfr_clstats_kentries(struct pfr_kentryworkq *, time_t,
> @@ -343,14 +345,12 @@ _bad:
> }
>
> int
> -pfr_del_addrs(struct pfr_table *tbl, struct pfr_addr *addr, int size,
> - int *ndel, int flags)
> +pfr_del_addr(struct pfr_table *tbl, struct pfr_addr *addr, int size, int
> flags)
Don't pass size.
> {
> struct pfr_ktable *kt;
> - struct pfr_kentryworkq workq;
> struct pfr_kentry *p;
> struct pfr_addr ad;
> - int i, rv, xdel = 0, log = 1;
> + int rv;
>
> ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY | PFR_FLAG_FEEDBACK);
> if (pfr_validate_table(tbl, 0, flags & PFR_FLAG_USERIOCTL))
> @@ -360,70 +360,29 @@ pfr_del_addrs(struct pfr_table *tbl, str
> return (ESRCH);
> if (kt->pfrkt_flags & PFR_TFLAG_CONST)
> return (EPERM);
> - /*
> - * there are two algorithms to choose from here.
> - * with:
> - * n: number of addresses to delete
> - * N: number of addresses in the table
> - *
> - * one is O(N) and is better for large 'n'
> - * one is O(n*LOG(N)) and is better for small 'n'
> - *
> - * following code try to decide which one is best.
> - */
> - for (i = kt->pfrkt_cnt; i > 0; i >>= 1)
> - log++;
> - if (size > kt->pfrkt_cnt/log) {
> - /* full table scan */
> - pfr_mark_addrs(kt);
> - } else {
> - /* iterate over addresses to delete */
> - for (i = 0; i < size; i++) {
> - YIELD(i, flags & PFR_FLAG_USERIOCTL);
> - if (COPYIN(addr+i, &ad, sizeof(ad), flags))
> - return (EFAULT);
> - if (pfr_validate_addr(&ad))
> - return (EINVAL);
> - p = pfr_lookup_addr(kt, &ad, 1);
> - if (p != NULL)
> - p->pfrke_flags &= ~PFRKE_FLAG_MARK;
> - }
> - }
> - SLIST_INIT(&workq);
> - for (i = 0; i < size; i++) {
> - YIELD(i, flags & PFR_FLAG_USERIOCTL);
> - if (COPYIN(addr+i, &ad, sizeof(ad), flags))
> +
> + if (COPYIN(addr, &ad, sizeof(ad), flags))
> + senderr(EFAULT);
> + if (pfr_validate_addr(&ad))
> + senderr(EINVAL);
> + p = pfr_lookup_addr(kt, &ad, 1);
> + if (flags & PFR_FLAG_FEEDBACK) {
> + if (p == NULL)
> + ad.pfra_fback = PFR_FB_NONE;
> + else if ((p->pfrke_flags & PFRKE_FLAG_NOT) !=
> + ad.pfra_not)
> + ad.pfra_fback = PFR_FB_CONFLICT;
> + else
> + ad.pfra_fback = PFR_FB_DELETED;
> +
> + if (COPYOUT(&ad, addr, sizeof(ad), flags))
> senderr(EFAULT);
> - if (pfr_validate_addr(&ad))
> - senderr(EINVAL);
> - p = pfr_lookup_addr(kt, &ad, 1);
> - if (flags & PFR_FLAG_FEEDBACK) {
> - if (p == NULL)
> - ad.pfra_fback = PFR_FB_NONE;
> - else if ((p->pfrke_flags & PFRKE_FLAG_NOT) !=
> - ad.pfra_not)
> - ad.pfra_fback = PFR_FB_CONFLICT;
> - else if (p->pfrke_flags & PFRKE_FLAG_MARK)
> - ad.pfra_fback = PFR_FB_DUPLICATE;
> - else
> - ad.pfra_fback = PFR_FB_DELETED;
> - }
> - if (p != NULL &&
> - (p->pfrke_flags & PFRKE_FLAG_NOT) == ad.pfra_not &&
> - !(p->pfrke_flags & PFRKE_FLAG_MARK)) {
> - p->pfrke_flags |= PFRKE_FLAG_MARK;
> - SLIST_INSERT_HEAD(&workq, p, pfrke_workq);
> - xdel++;
> - }
> - if (flags & PFR_FLAG_FEEDBACK)
> - if (COPYOUT(&ad, addr+i, sizeof(ad), flags))
> - senderr(EFAULT);
> }
> - if (!(flags & PFR_FLAG_DUMMY)) {
> - pfr_remove_kentries(kt, &workq);
> +
> + if (!(flags & PFR_FLAG_DUMMY) && (p != NULL)) {
> + pfr_remove_kentry(kt, p);
> }
> - if (ndel != NULL)
> - *ndel = xdel;
> +
> return (0);
> _bad:
> if (flags & PFR_FLAG_FEEDBACK)
Don't pass size in pfr_reset_feedback(addr, size, flags), it is 1.
> @@ -947,6 +906,25 @@ pfr_insert_kentry(struct pfr_ktable *kt,
> pfr_ktable_winfo_update(kt, p);
>
> return (0);
> +}
> +
> +void
> +pfr_remove_kentry(struct pfr_ktable *kt, struct pfr_kentry *p)
> +{
> + struct pfr_kentryworkq addrq;
> +
> + pfr_unroute_kentry(kt, p);
> + if (p->pfrke_type == PFRKE_COST)
> + kt->pfrkt_refcntcost--;
> + kt->pfrkt_cnt--;
> +
Also copy the comment. /* update maxweight and gcd for load balancing */
> + if (kt->pfrkt_refcntcost > 0) {
> + kt->pfrkt_gcdweight = 0;
> + kt->pfrkt_maxweight = 1;
> + pfr_enqueue_addrs(kt, &addrq, NULL, 0);
> + SLIST_FOREACH(p, &addrq, pfrke_workq)
> + pfr_ktable_winfo_update(kt, p);
> + }
> }
>
> void
> Index: sys/net/pfvar.h
> ===================================================================
> RCS file: /cvs/src/sys/net/pfvar.h,v
> retrieving revision 1.421
> diff -u -p -r1.421 pfvar.h
> --- sys/net/pfvar.h 13 Oct 2015 19:32:32 -0000 1.421
> +++ sys/net/pfvar.h 27 Oct 2015 22:57:25 -0000
> @@ -1614,7 +1614,7 @@ struct pfioc_iface {
> #define DIOCRCLRTSTATS _IOWR('D', 65, struct pfioc_table)
> #define DIOCRCLRADDRS _IOWR('D', 66, struct pfioc_table)
> #define DIOCRADDADDRS _IOWR('D', 67, struct pfioc_table)
> -#define DIOCRDELADDRS _IOWR('D', 68, struct pfioc_table)
> +#define DIOCRDELADDR _IOWR('D', 68, struct pfioc_table)
> #define DIOCRSETADDRS _IOWR('D', 69, struct pfioc_table)
> #define DIOCRGETADDRS _IOWR('D', 70, struct pfioc_table)
> #define DIOCRGETASTATS _IOWR('D', 71, struct pfioc_table)
> @@ -1791,8 +1791,7 @@ int pfr_clr_addrs(struct pfr_table *, in
> int pfr_insert_kentry(struct pfr_ktable *, struct pfr_addr *, time_t);
> int pfr_add_addrs(struct pfr_table *, struct pfr_addr *, int, int *,
> int);
> -int pfr_del_addrs(struct pfr_table *, struct pfr_addr *, int, int *,
> - int);
> +int pfr_del_addr(struct pfr_table *, struct pfr_addr *, int, int);
> int pfr_set_addrs(struct pfr_table *, struct pfr_addr *, int, int *,
> int *, int *, int *, int, u_int32_t);
> int pfr_get_addrs(struct pfr_table *, struct pfr_addr *, int *, int);
> Index: sbin/pfctl/pfctl_radix.c
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/pfctl_radix.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 pfctl_radix.c
> --- sbin/pfctl/pfctl_radix.c 21 Jan 2015 21:50:33 -0000 1.32
> +++ sbin/pfctl/pfctl_radix.c 27 Oct 2015 22:59:35 -0000
This diff is a duplicate.
> @@ -207,6 +207,7 @@ pfr_del_addrs(struct pfr_table *tbl, str
> int *ndel, int flags)
> {
> struct pfioc_table io;
> + int i, rv, del = 0;
>
> if (tbl == NULL || size < 0 || (size && addr == NULL)) {
> errno = EINVAL;
> @@ -215,14 +216,18 @@ pfr_del_addrs(struct pfr_table *tbl, str
> bzero(&io, sizeof io);
> io.pfrio_flags = flags;
> io.pfrio_table = *tbl;
> - io.pfrio_buffer = addr;
> io.pfrio_esize = sizeof(*addr);
> - io.pfrio_size = size;
> - if (ioctl(dev, DIOCRDELADDRS, &io))
> - return (-1);
> - if (ndel != NULL)
> - *ndel = io.pfrio_ndel;
> - return (0);
> + io.pfrio_size = 1;
> + for (i = 0; (i < size) && (rv == 0); i++) {
> + io.pfrio_buffer = addr++;
> + rv = ioctl(dev, DIOCRDELADDR, &io);
> + del++;
> + }
> +
> + if ((rv == 0) && (ndel != NULL))
> + *ndel = del;
> +
> + return (rv);
> }
>
> int
bluhm
