On Thu, May 25, 2017 at 01:14:54PM +0200, Mike Belopuhov wrote:
> >
> > Sorry I didn't test it: I had understood that due the problem was due to
> > my (unsupported) use of fq on pppoe0, so I just skipped it (the
> > backtrace of the panic passed by sppp_ functions).
> >
>
> We still need to solve the issue and commit the fix so please run with
> it and report if something goes bad. I've been running with this diff
> for about a week and haven't had any issues.
>
I ran with it now.
> >
> > # pfctl -sq
> > pfctl: DIOCGETQSTATS: Bad file descriptor
> >
> > (and same error with systat queue).
> >
>
> Yes, this is a side effect of using pf kifs in the queueing code.
> This issue has been there since the beginning. Claudio has recently
> pointed out that we should probably resolve it. The problem here is
> that you have specified a nonexistent interface name for your queue.
> For instance you have loaded a pf ruleset with "queue foo on pppoe0"
> before creating pppoe0 itself. This will result in a successful
> application of the ruleset but queues won't be created.
>
> So please make sure that your "ifconfig" output corresponds to the
> pf.conf and you *don't* get a 'DIOCGETQSTATS: Bad file descriptor'.
> This is an indictaion that your queues are not created or some of
> them are not created.
I had a typo in my interface name... once corrected it works better :)
sorry for the noise.
> Claudio has also suggested aborting when a queue is to be created
> on a non-exitent interface
the diff works as expected: I couldn't create anymore a queue for a
non-existing interface.
note it is still possible to make the system enter in inconsistent state
(and getting DIOCGETQSTATS error) by creating the queue, and next
removing the interface (ifconfig pppoe0 destroy).
Thanks.
--
Sebastien Marie
> diff --git sys/net/pf_ioctl.c sys/net/pf_ioctl.c
> index 7cb7b92ed8a..6cf62cd2ec6 100644
> --- sys/net/pf_ioctl.c
> +++ sys/net/pf_ioctl.c
> @@ -578,11 +578,11 @@ pf_ifp2q(struct pf_queue_if *list, struct ifnet *ifp)
> int
> pf_create_queues(void)
> {
> struct pf_queuespec *q;
> struct ifnet *ifp;
> - struct pf_queue_if *list = NULL, *qif;
> + struct pf_queue_if *list = NULL, *qif;
> int error;
>
> /*
> * Find root queues and allocate traffic conditioner
> * private data for these interfaces
> @@ -1115,20 +1115,19 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> bcopy(&q->queue, qs, sizeof(*qs));
> qs->qid = pf_qname2qid(qs->qname, 1);
> if (qs->parent[0] && (qs->parent_qid =
> pf_qname2qid(qs->parent, 0)) == 0) {
> pool_put(&pf_queue_pl, qs);
> - error = ESRCH;
> + error = EINVAL;
> break;
> }
> qs->kif = pfi_kif_get(qs->ifname);
> - if (qs->kif == NULL) {
> + if (qs->kif == NULL || qs->kif->pfik_ifp == NULL) {
> pool_put(&pf_queue_pl, qs);
> - error = ESRCH;
> + error = EINVAL;
> break;
> }
> - /* XXX resolve bw percentage specs */
> pfi_kif_ref(qs->kif, PFI_KIF_REF_RULE);
>
> TAILQ_INSERT_TAIL(pf_queues_inactive, qs, entries);
>
> break;
>
