Subject: Re: Re: kernel failure with pf and altq
To: Pavel Cahyna <pavel@netbsd.org>
From: matthew sporleder <msporleder@gmail.com>
List: current-users
Date: 09/25/2006 18:13:34
On 9/25/06, Pavel Cahyna <pavel@netbsd.org> wrote:
> On Sat, Sep 09, 2006 at 08:27:25AM -0400, matthew sporleder wrote:
> > It looks like having pf and altq in your kernel don't work.  (I
> > thought our altq was decoupled)
>
> This is known and documented, see
>
> http://www.netbsd.org/Documentation/network/pf.html#differences
>
> I have a patch to correct this, could you please test it?
> (to be applied under sys/dist/pf/net.)
>
> Pavel
>
> Index: pf.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dist/pf/net/pf.c,v
> retrieving revision 1.23
> diff -u -r1.23 pf.c
> --- pf.c        14 May 2006 03:40:02 -0000      1.23
> +++ pf.c        25 Sep 2006 15:52:53 -0000
> @@ -1515,7 +1515,7 @@
>                 }
>                 m_tag_prepend(m, mtag);
>         }
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>         if (r != NULL && r->qid) {
>                 struct m_tag    *mtag;
>                 struct altq_tag *atag;
> @@ -1530,7 +1530,7 @@
>                         m_tag_prepend(m, mtag);
>                 }
>         }
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>         m->m_data += max_linkhdr;
>         m->m_pkthdr.len = m->m_len = len;
>         m->m_pkthdr.rcvif = NULL;
> @@ -1666,7 +1666,7 @@
>         }
>         m_tag_prepend(m0, mtag);
>
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>         if (r->qid) {
>                 struct altq_tag *atag;
>
> @@ -1680,7 +1680,7 @@
>                         m_tag_prepend(m0, mtag);
>                 }
>         }
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>
>         switch (af) {
>  #ifdef INET
> @@ -6056,7 +6056,7 @@
>         if (s && s->tag)
>                 pf_tag_packet(m, pf_get_tag(m), s->tag);
>
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>         if (action == PF_PASS && r->qid) {
>                 struct m_tag    *mtag;
>                 struct altq_tag *atag;
> @@ -6074,7 +6074,7 @@
>                         m_tag_prepend(m, mtag);
>                 }
>         }
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>
>         /*
>          * connections redirected to loopback should not match sockets
> @@ -6397,7 +6397,7 @@
>         if (s && s->tag)
>                 pf_tag_packet(m, pf_get_tag(m), s->tag);
>
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>         if (action == PF_PASS && r->qid) {
>                 struct m_tag    *mtag;
>                 struct altq_tag *atag;
> @@ -6415,7 +6415,7 @@
>                         m_tag_prepend(m, mtag);
>                 }
>         }
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>
>         if (dir == PF_IN && action == PF_PASS && (pd.proto == IPPROTO_TCP ||
>             pd.proto == IPPROTO_UDP) && s != NULL && s->nat_rule.ptr != NULL &&
> Index: pf_ioctl.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dist/pf/net/pf_ioctl.c,v
> retrieving revision 1.23
> diff -u -r1.23 pf_ioctl.c
> --- pf_ioctl.c  8 Sep 2006 20:58:57 -0000       1.23
> +++ pf_ioctl.c  25 Sep 2006 15:52:53 -0000
> @@ -38,7 +38,6 @@
>
>  #ifdef _KERNEL_OPT
>  #include "opt_inet.h"
> -#include "opt_altq.h"
>  #include "opt_pfil_hooks.h"
>  #endif
>
> @@ -95,7 +94,7 @@
>  #include <netinet/in_pcb.h>
>  #endif /* INET6 */
>
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>  #include <altq/altq.h>
>  #endif
>
> @@ -118,13 +117,13 @@
>  void                    pf_mv_pool(struct pf_palist *, struct pf_palist *);
>  void                    pf_empty_pool(struct pf_palist *);
>  int                     pfioctl(dev_t, u_long, caddr_t, int, struct lwp *);
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>  int                     pf_begin_altq(u_int32_t *);
>  int                     pf_rollback_altq(u_int32_t);
>  int                     pf_commit_altq(u_int32_t);
>  int                     pf_enable_altq(struct pf_altq *);
>  int                     pf_disable_altq(struct pf_altq *);
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>  int                     pf_begin_rules(u_int32_t *, int, const char *);
>  int                     pf_rollback_rules(u_int32_t, int, char *);
>  int                     pf_commit_rules(u_int32_t, int, char *);
> @@ -148,7 +147,7 @@
>  #endif
>
>  struct pf_rule          pf_default_rule;
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>  static int              pf_altq_running;
>  #endif
>
> @@ -267,7 +266,7 @@
>         for (i = 0; i < PF_RULESET_MAX; i++)
>                 if (pf_begin_rules(&ticket, i, &r) == 0)
>                         pf_commit_rules(ticket, i, &r);
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>         if (pf_begin_altq(&ticket) == 0)
>                 pf_commit_altq(ticket);
>  #endif
> @@ -721,7 +720,7 @@
>                 return;
>         pf_tag_unref(rule->tag);
>         pf_tag_unref(rule->match_tag);
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>         if (rule->pqid != rule->qid)
>                 pf_qid_unref(rule->pqid);
>         pf_qid_unref(rule->qid);
> @@ -886,7 +885,7 @@
>  #endif
>  }
>
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>  u_int32_t
>  pf_qname2qid(char *qname)
>  {
> @@ -1058,7 +1057,7 @@
>
>         return (error);
>  }
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>
>  int
>  pf_begin_rules(u_int32_t *ticket, int rs_num, const char *anchor)
> @@ -1343,7 +1342,7 @@
>                         }
>                 }
>
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>                 /* set queue IDs */
>                 if (rule->qname[0] != 0) {
>                         if ((rule->qid = pf_qname2qid(rule->qname)) == 0)
> @@ -1564,7 +1563,7 @@
>                         } else
>                                 newrule->kif = NULL;
>
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>                         /* set queue IDs */
>                         if (newrule->qname[0] != 0) {
>                                 if ((newrule->qid =
> @@ -1577,7 +1576,7 @@
>                                 } else
>                                         newrule->pqid = newrule->qid;
>                         }
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>                         if (newrule->tagname[0])
>                                 if ((newrule->tag =
>                                     pf_tagname2tag(newrule->tagname)) == 0)
> @@ -2027,7 +2026,7 @@
>                 break;
>         }
>
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>         case DIOCSTARTALTQ: {
>                 struct pf_altq          *altq;
>
> @@ -2174,7 +2173,7 @@
>                 }
>                 break;
>         }
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>
>         case DIOCBEGINADDRS: {
>                 struct pfioc_pooladdr   *pp = (struct pfioc_pooladdr *)addr;
> @@ -2667,7 +2666,7 @@
>                                 goto fail;
>                         }
>                         switch (ioe.rs_num) {
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>                         case PF_RULESET_ALTQ:
>                                 if (ioe.anchor[0]) {
>                                         error = EINVAL;
> @@ -2676,7 +2675,7 @@
>                                 if ((error = pf_begin_altq(&ioe.ticket)))
>                                         goto fail;
>                                 break;
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>                         case PF_RULESET_TABLE:
>                                 bzero(&table, sizeof(table));
>                                 strlcpy(table.pfrt_anchor, ioe.anchor,
> @@ -2716,7 +2715,7 @@
>                                 goto fail;
>                         }
>                         switch (ioe.rs_num) {
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>                         case PF_RULESET_ALTQ:
>                                 if (ioe.anchor[0]) {
>                                         error = EINVAL;
> @@ -2725,7 +2724,7 @@
>                                 if ((error = pf_rollback_altq(ioe.ticket)))
>                                         goto fail; /* really bad */
>                                 break;
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>                         case PF_RULESET_TABLE:
>                                 bzero(&table, sizeof(table));
>                                 strlcpy(table.pfrt_anchor, ioe.anchor,
> @@ -2763,7 +2762,7 @@
>                                 goto fail;
>                         }
>                         switch (ioe.rs_num) {
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>                         case PF_RULESET_ALTQ:
>                                 if (ioe.anchor[0]) {
>                                         error = EINVAL;
> @@ -2775,7 +2774,7 @@
>                                         goto fail;
>                                 }
>                                 break;
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>                         case PF_RULESET_TABLE:
>                                 rs = pf_find_ruleset(ioe.anchor);
>                                 if (rs == NULL || !rs->topen || ioe.ticket !=
> @@ -2808,12 +2807,12 @@
>                                 goto fail;
>                         }
>                         switch (ioe.rs_num) {
> -#ifdef ALTQ
> +#ifdef ALTQ_NEW
>                         case PF_RULESET_ALTQ:
>                                 if ((error = pf_commit_altq(ioe.ticket)))
>                                         goto fail; /* really bad */
>                                 break;
> -#endif /* ALTQ */
> +#endif /* ALTQ_NEW */
>                         case PF_RULESET_TABLE:
>                                 bzero(&table, sizeof(table));
>                                 strlcpy(table.pfrt_anchor, ioe.anchor,
>

I just built the kernel successfully, but I won't have a chance to try
booting it for a while.

Thanks,
_Matt