On Tuesday 27 November 2007, Florian Smeets wrote: > Max Laier wrote: > > On Tuesday 27 November 2007, Florian Smeets wrote: > >> Hi > >> > >> i was able to reproduce a hang on a 7-STABLE (csuped just after > >> Scotts critical section MFC) firewall which runs mpd4 from ports and > >> uses pf for packet filtering. Sometimes when i restart mpd4 the box > >> just hangs. I have a up-script which calls /sbin/pfctl -f > >> /etc/pf.conf. > > > > It's an ALTQ problem, really. I don't know if there is a down-script > > that runs before the interface is destroyed, but if there is - adding > > pfctl -Fq there should work around your problem. I did have some > > patches to have ALTQ working with dynamic interfaces, but I must have > > dropped them. I'll see what I can dig up in the next few days. > > That would be great. I have a second non productive box where i can > reproduce the problem. I'll be glad to test any patches.
Okay ... try this. Not final yet, but should be functional. With this
you should be able to:
1) Safely remove an interface with active queues
2) Re-add the interface and *magically* get the queues back
3) Write queue rules for non-existing interfaces
- Note that we will assume an MTU of 1500 and you have to specify a
fixed bandwidth as we don't know the interface's native speed
- Obviously these queues will be activated as soon as a matching
interface is created.
BUGS: Doesn't print queues on removed interfaces at all. Should be
changed to something like "queue foo on bar0 (N/A) ...", but it seems I
was too strict with the local_flags. The error handling might need some
work in order to avoid panic if something goes wrong while we de-activate
queues.
I'd like to hear back from you in order to see if I at least got the basic
workings right enough so you can survive the mpd interface destroy.
Could you - in addition to you current setup w/ if-up script - also test
the magic part? i.e. load the ruleset before loading mpd. This should
now be possible as long as you don't put "set loginterface" or fixed
interface-to-address src/dst in it \o/
--
/"\ Best regards, | [EMAIL PROTECTED]
\ / Max Laier | ICQ #67774661
X http://pf4freebsd.love2party.net/ | [EMAIL PROTECTED]
/ \ ASCII Ribbon Campaign | Against HTML Mail and News
diff --git a/contrib/pf/pfctl/pfctl_altq.c b/contrib/pf/pfctl/pfctl_altq.c
index 9104f5a..d2a21c8 100644
--- a/contrib/pf/pfctl/pfctl_altq.c
+++ b/contrib/pf/pfctl/pfctl_altq.c
@@ -154,6 +154,10 @@ print_altq(const struct pf_altq *a, unsigned level, struct node_queue_bw *bw,
}
printf("altq on %s ", a->ifname);
+#ifdef __FreeBSD__
+ if (a->local_flags)
+ printf("N/A ");
+#endif
switch (a->scheduler) {
case ALTQT_CBQ:
@@ -1145,7 +1149,11 @@ getifmtu(char *ifname)
sizeof(ifr.ifr_name))
errx(1, "getifmtu: strlcpy");
if (ioctl(s, SIOCGIFMTU, (caddr_t)&ifr) == -1)
+#ifdef __FreeBSD__
+ ifr.ifr_mtu = 1500;
+#else
err(1, "SIOCGIFMTU");
+#endif
if (shutdown(s, SHUT_RDWR) == -1)
err(1, "shutdown");
if (close(s))
diff --git a/contrib/pf/pfctl/pfctl_qstats.c b/contrib/pf/pfctl/pfctl_qstats.c
index 3eb2987..15695cb 100644
--- a/contrib/pf/pfctl/pfctl_qstats.c
+++ b/contrib/pf/pfctl/pfctl_qstats.c
@@ -157,7 +157,11 @@ pfctl_update_qstats(int dev, struct pf_altq_node **root)
warn("DIOCGETALTQ");
return (-1);
}
+#ifdef __FreeBSD__
+ if (pa.altq.qid > 0 && !pa.altq.local_flags) {
+#else
if (pa.altq.qid > 0) {
+#endif
pq.nr = nr;
pq.ticket = pa.ticket;
pq.buf = &qstats.data;
diff --git a/sys/contrib/pf/net/pf_if.c b/sys/contrib/pf/net/pf_if.c
index 74e9dcb..8b09cc3 100644
--- a/sys/contrib/pf/net/pf_if.c
+++ b/sys/contrib/pf/net/pf_if.c
@@ -893,6 +893,9 @@ pfi_attach_ifnet_event(void *arg __unused, struct ifnet *ifp)
{
PF_LOCK();
pfi_attach_ifnet(ifp);
+#ifdef ALTQ
+ pf_altq_ifnet_event(ifp, 0);
+#endif
PF_UNLOCK();
}
@@ -901,6 +904,9 @@ pfi_detach_ifnet_event(void *arg __unused, struct ifnet *ifp)
{
PF_LOCK();
pfi_detach_ifnet(ifp);
+#ifdef ALTQ
+ pf_altq_ifnet_event(ifp, 1);
+#endif
PF_UNLOCK();
}
diff --git a/sys/contrib/pf/net/pf_ioctl.c b/sys/contrib/pf/net/pf_ioctl.c
index 136e710..0681abf 100644
--- a/sys/contrib/pf/net/pf_ioctl.c
+++ b/sys/contrib/pf/net/pf_ioctl.c
@@ -787,7 +787,11 @@ pf_begin_altq(u_int32_t *ticket)
/* Purge the old altq list */
while ((altq = TAILQ_FIRST(pf_altqs_inactive)) != NULL) {
TAILQ_REMOVE(pf_altqs_inactive, altq, entries);
+#ifdef __FreeBSD__
+ if (altq->qname[0] == 0 && altq->local_flags == 0) {
+#else
if (altq->qname[0] == 0) {
+#endif
/* detach and destroy the discipline */
error = altq_remove(altq);
} else
@@ -812,7 +816,11 @@ pf_rollback_altq(u_int32_t ticket)
/* Purge the old altq list */
while ((altq = TAILQ_FIRST(pf_altqs_inactive)) != NULL) {
TAILQ_REMOVE(pf_altqs_inactive, altq, entries);
+#ifdef __FreeBSD__
+ if (altq->qname[0] == 0 && altq->local_flags == 0) {
+#else
if (altq->qname[0] == 0) {
+#endif
/* detach and destroy the discipline */
error = altq_remove(altq);
} else
@@ -842,7 +850,11 @@ pf_commit_altq(u_int32_t ticket)
/* Attach new disciplines */
TAILQ_FOREACH(altq, pf_altqs_active, entries) {
+#ifdef __FreeBSD__
+ if (altq->qname[0] == 0 && altq->local_flags == 0) {
+#else
if (altq->qname[0] == 0) {
+#endif
/* attach the discipline */
error = altq_pfattach(altq);
if (error == 0 && pf_altq_running)
@@ -857,7 +869,11 @@ pf_commit_altq(u_int32_t ticket)
/* Purge the old altq list */
while ((altq = TAILQ_FIRST(pf_altqs_inactive)) != NULL) {
TAILQ_REMOVE(pf_altqs_inactive, altq, entries);
+#ifdef __FreeBSD__
+ if (altq->qname[0] == 0 && altq->local_flags == 0) {
+#else
if (altq->qname[0] == 0) {
+#endif
/* detach and destroy the discipline */
if (pf_altq_running)
error = pf_disable_altq(altq);
@@ -943,6 +959,91 @@ pf_disable_altq(struct pf_altq *altq)
return (error);
}
+
+#ifdef __FreeBSD__
+void
+pf_altq_ifnet_event(struct ifnet *ifp, int remove)
+{
+ struct ifnet *ifp1;
+ struct pf_altq *a1, *a2, *a3;
+ u_int32_t ticket;
+ int error = 0;
+
+ DPFPRINTF(PF_DEBUG_MISC, ("altq: ifnet event %p %d\n", ifp, remove));
+ /* Interrupt userland queue modifications */
+ if (altqs_inactive_open) {
+ DPFPRINTF(PF_DEBUG_MISC,
+ ("altq: detach event preempted userland\n"));
+ pf_rollback_altq(ticket_altqs_inactive);
+ }
+
+ /* Start new altq ruleset */
+ if (pf_begin_altq(&ticket)) {
+ DPFPRINTF(PF_DEBUG_MISC, ("altq: pf_begin_altq failed\n"));
+ PF_UNLOCK();
+ return;
+ }
+ /* Copy the current active set */
+ TAILQ_FOREACH(a1, pf_altqs_active, entries) {
+ a2 = pool_get(&pf_altq_pl, PR_NOWAIT);
+ if (a2 == NULL) {
+ error = ENOMEM;
+ break;
+ }
+ bcopy(a1, a2, sizeof(struct pf_altq));
+
+ DPFPRINTF(PF_DEBUG_MISC, ("altq: copy %s.%s\n", a2->ifname,
+ a2->qname));
+ if (a2->qname[0] != 0) {
+ if ((a2->qid = pf_qname2qid(a2->qname)) == 0) {
+ error = EBUSY;
+ pool_put(&pf_altq_pl, a2);
+ break;
+ }
+ a2->altq_disc = NULL;
+ TAILQ_FOREACH(a3, pf_altqs_inactive, entries) {
+ if (strncmp(a3->ifname, a2->ifname,
+ IFNAMSIZ) == 0 && a3->qname[0] == 0) {
+ a2->altq_disc = a3->altq_disc;
+ break;
+ }
+ }
+ }
+ /* Deactivate the interface in question */
+ a2->local_flags = 0;
+ if ((ifp1 = ifunit(a2->ifname)) == NULL ||
+ (remove && ifp1 == ifp)) {
+ a2->local_flags |= PFALTQ_FLAG_IF_REMOVED;
+ DPFPRINTF(PF_DEBUG_MISC,
+ ("altq: no interface for %s.%s\n", a2->ifname,
+ a2->qname));
+ } else {
+ PF_UNLOCK();
+ error = altq_add(a2);
+ PF_LOCK();
+
+ if (ticket != ticket_altqs_inactive)
+ error = EBUSY;
+
+ if (error) {
+ pool_put(&pf_altq_pl, a2);
+ break;
+ }
+ }
+ DPFPRINTF(PF_DEBUG_MISC, ("altq: adding %s.%s\n", a2->ifname,
+ a2->qname));
+
+ TAILQ_INSERT_TAIL(pf_altqs_inactive, a2, entries);
+ }
+
+ if (error != 0) {
+ DPFPRINTF(PF_DEBUG_MISC, ("altq: copy failed %d\n", error));
+ pf_rollback_altq(ticket);
+ } else {
+ pf_commit_altq(ticket);
+ }
+}
+#endif
#endif /* ALTQ */
int
@@ -2273,7 +2374,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
/* enable all altq interfaces on active list */
TAILQ_FOREACH(altq, pf_altqs_active, entries) {
+#ifdef __FreeBSD__
+ if (altq->qname[0] == 0 &&
+ altq->local_flags == 0) {
+#else
if (altq->qname[0] == 0) {
+#endif
error = pf_enable_altq(altq);
if (error != 0)
break;
@@ -2290,7 +2396,12 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
/* disable all altq interfaces on active list */
TAILQ_FOREACH(altq, pf_altqs_active, entries) {
+#ifdef __FreeBSD__
+ if (altq->qname[0] == 0 &&
+ altq->local_flags == 0) {
+#else
if (altq->qname[0] == 0) {
+#endif
error = pf_disable_altq(altq);
if (error != 0)
break;
@@ -2316,6 +2427,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
break;
}
bcopy(&pa->altq, altq, sizeof(struct pf_altq));
+#ifdef __FreeBSD__
+ altq->local_flags = 0;
+#endif
/*
* if this is for a queue, find the discipline and
@@ -2327,6 +2441,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
pool_put(&pf_altq_pl, altq);
break;
}
+ altq->altq_disc = NULL;
TAILQ_FOREACH(a, pf_altqs_inactive, entries) {
if (strncmp(a->ifname, altq->ifname,
IFNAMSIZ) == 0 && a->qname[0] == 0) {
@@ -2337,11 +2452,17 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
}
#ifdef __FreeBSD__
- PF_UNLOCK();
+ struct ifnet *ifp;
+
+ if ((ifp = ifunit(altq->ifname)) == NULL) {
+ altq->local_flags |= PFALTQ_FLAG_IF_REMOVED;
+ } else {
+ PF_UNLOCK();
#endif
error = altq_add(altq);
#ifdef __FreeBSD__
- PF_LOCK();
+ PF_LOCK();
+ }
#endif
if (error) {
pool_put(&pf_altq_pl, altq);
@@ -2414,6 +2535,10 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
break;
}
#ifdef __FreeBSD__
+ if (altq->local_flags != 0) {
+ error = ENXIO;
+ break;
+ }
PF_UNLOCK();
#endif
error = altq_getqstats(altq, pq->buf, &nbytes);
diff --git a/sys/contrib/pf/net/pfvar.h b/sys/contrib/pf/net/pfvar.h
index fbc87e3..8673594 100644
--- a/sys/contrib/pf/net/pfvar.h
+++ b/sys/contrib/pf/net/pfvar.h
@@ -1247,6 +1247,11 @@ struct pf_altq {
u_int32_t parent_qid; /* parent queue id */
u_int32_t bandwidth; /* queue bandwidth */
u_int8_t priority; /* priority */
+#ifdef __FreeBSD__
+ u_int8_t local_flags; /* dynamic interface */
+#define PFALTQ_FLAG_IF_REMOVED 0x01
+#define PFALTQ_FLAG_BW_UNKNOWN 0x02
+#endif
u_int16_t qlimit; /* queue size limit */
u_int16_t flags; /* misc flags */
union {
@@ -1574,6 +1579,9 @@ extern void pf_tbladdr_remove(struct pf_addr_wrap *);
extern void pf_tbladdr_copyout(struct pf_addr_wrap *);
extern void pf_calc_skip_steps(struct pf_rulequeue *);
#ifdef __FreeBSD__
+#ifdef ALTQ
+extern void pf_altq_ifnet_event(struct ifnet *, int);
+#endif
extern uma_zone_t pf_src_tree_pl, pf_rule_pl;
extern uma_zone_t pf_state_pl, pf_altq_pl, pf_pooladdr_pl;
extern uma_zone_t pfr_ktable_pl, pfr_kentry_pl, pfr_kentry_pl2;
signature.asc
Description: This is a digitally signed message part.
