On Wed, Apr 11, 2012 at 12:20:30PM +0200, Henning Brauer wrote:
> * Henning Brauer <[email protected]> [2012-04-11 11:26]:
> > * Siju George <[email protected]> [2012-04-10 08:16]:
> > > On Tue, Apr 10, 2012 at 11:40 AM, Andres Perera <[email protected]> wrote:
> > > > altering the max might have consequences i don't know about:
> > > I will stick with 15 :-)
> >
> > actually, bumping it should be absolutely safe.
> >
> > pretty dumb limit actually, we should just dynamically allocate the
> > pflogifs array.
>
> please try this & report back
>
> Index: if_pflog.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_pflog.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 if_pflog.c
> --- if_pflog.c 3 Feb 2012 01:57:50 -0000 1.49
> +++ if_pflog.c 11 Apr 2012 10:19:56 -0000
> @@ -80,6 +80,7 @@
> #endif
>
> void pflogattach(int);
> +int pflogifs_resize(size_t);
> int pflogoutput(struct ifnet *, struct mbuf *, struct sockaddr *,
> struct rtentry *);
> int pflogioctl(struct ifnet *, u_long, caddr_t);
> @@ -91,16 +92,14 @@ LIST_HEAD(, pflog_softc) pflogif_list;
> struct if_clone pflog_cloner =
> IF_CLONE_INITIALIZER("pflog", pflog_clone_create, pflog_clone_destroy);
>
> -struct ifnet *pflogifs[PFLOGIFS_MAX]; /* for fast access */
> -struct mbuf *pflog_mhdr = NULL, *pflog_mptr = NULL;
> +int npflogifs = 0;
> +struct ifnet **pflogifs = NULL; /* for fast access */
> +struct mbuf *pflog_mhdr = NULL, *pflog_mptr = NULL;
>
> void
> pflogattach(int npflog)
> {
> - int i;
> LIST_INIT(&pflogif_list);
> - for (i = 0; i < PFLOGIFS_MAX; i++)
> - pflogifs[i] = NULL;
> if (pflog_mhdr == NULL)
> if ((pflog_mhdr = m_get(M_DONTWAIT, MT_HEADER)) == NULL)
> panic("pflogattach: no mbuf");
> @@ -111,14 +110,41 @@ pflogattach(int npflog)
> }
>
> int
> +pflogifs_resize(size_t n)
> +{
> + struct ifnet **p;
> + int i;
> +
> + if (n > SIZE_MAX / sizeof(struct ifnet))
> + return (EINVAL);
> + if (n == 0)
> + p = NULL;
> + else
> + if ((p = malloc(n * sizeof(struct ifnet), M_DEVBUF,
> + M_NOWAIT|M_ZERO)) == NULL)
> + return (ENOMEM);
don't you need two different index vars for this next
section?
> + for (i = 0; i < n; i++)
> + if (i < npflogifs)
> + p[i] = pflogifs[i];
> + else
> + p[i] = NULL;
something like the following with caveats that a) it is
5am-ish for me and b) i did not try compiling it:
for (i = 0, j = 0; i < n; i++, j++) {
for (; j < npflogifs && NULL == pflogifs[j]; j++)
;
if (j == npflogifs)
break;
p[i] = pflogifs[j];
}
for (; i < n; i++)
p[i] = NULL;
> +
> + if(pflogifs)
^^ nit
> + free(pflogifs, M_DEVBUF);
> + pflogifs = p;
> + npflogifs = n;
> + return (0);
> +}
> +
> +int
> pflog_clone_create(struct if_clone *ifc, int unit)
> {
> struct ifnet *ifp;
> struct pflog_softc *pflogif;
> int s;
>
> - if (unit >= PFLOGIFS_MAX)
> - return (EINVAL);
> + if (unit + 1 > npflogifs && pflogifs_resize(unit + 1) != 0)
> + return (ENOMEM);
>
> if ((pflogif = malloc(sizeof(*pflogif),
> M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
> @@ -154,11 +180,16 @@ int
> pflog_clone_destroy(struct ifnet *ifp)
> {
> struct pflog_softc *pflogif = ifp->if_softc;
> - int s;
> + int s, i;
>
> s = splnet();
> pflogifs[pflogif->sc_unit] = NULL;
> LIST_REMOVE(pflogif, sc_list);
> +
> + for (i = npflogifs; i > 0 && pflogifs[i - 1] != NULL; i--)
> + ; /* nothing */
> + if (i < npflogifs)
> + pflogifs_resize(i); /* error harmless here */
So, if the last pflogifs entry is NULL don't resize
down? Not really questioning the logic, but want to
make sure I understand that's what is meant, cause
there is an easier check for that than the for()-loop.
Caveats: a) 5am-ish, b) not familiar with code.
--patrick
> splx(s);
>
> if_detach(ifp);
> @@ -225,7 +256,8 @@ pflog_packet(struct pf_pdesc *pd, u_int8
> if (rm == NULL || pd == NULL || pd->kif == NULL || pd->m == NULL)
> return (-1);
>
> - if ((ifn = pflogifs[rm->logif]) == NULL || !ifn->if_bpf)
> + if (rm->logif >= npflogifs || (ifn = pflogifs[rm->logif]) == NULL ||
> + !ifn->if_bpf)
> return (0);
>
> bzero(&hdr, sizeof(hdr));
> Index: pf_ioctl.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> retrieving revision 1.250
> diff -u -p -r1.250 pf_ioctl.c
> --- pf_ioctl.c 3 Apr 2012 15:09:03 -0000 1.250
> +++ pf_ioctl.c 11 Apr 2012 10:19:57 -0000
> @@ -2595,8 +2595,6 @@ pf_rule_copyin(struct pf_rule *from, str
> #if NPFLOG > 0
> if (!to->log)
> to->logif = 0;
> - if (to->logif >= PFLOGIFS_MAX)
> - return (EINVAL);
> #endif
> to->quick = from->quick;
> to->ifnot = from->ifnot;
>
> --
> Henning Brauer, [email protected], [email protected]
> BS Web Services, http://bsws.de, Full-Service ISP
> Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully
> Managed
> Henning Brauer Consulting, http://henningbrauer.com/
