Hello,
> Hrvoje Popovski confirmed this reduces "ifconfig" pauses. He also
> confirmed states are correctly purged.
>
> So, I'd appreciate reviews and oks.
sorry for keeping you waiting. the change looks good. just a small nit, which I
don't insist on.
> > Index: net/pfvar.h
> > ===================================================================
> > RCS file: /cvs/src/sys/net/pfvar.h,v
> > retrieving revision 1.460
> > diff -u -p -r1.460 pfvar.h
> > --- net/pfvar.h 28 Jun 2017 19:30:24 -0000 1.460
> > +++ net/pfvar.h 18 Jul 2017 13:30:23 -0000
> > @@ -1647,7 +1647,8 @@ extern int
> > pf_tbladdr_setup(struct pf
> > 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
> > *);
> > -extern void pf_purge_thread(void *);
> > +extern void pf_purge_timeout(void *);
> > +extern void pf_purge(void *);
> > extern void pf_purge_expired_src_nodes();
> > extern void pf_purge_expired_states(u_int32_t);
> > extern void pf_purge_expired_rules();
> > @@ -1826,6 +1827,8 @@ const struct pfq_ops
> > *pf_queue_manager(struct pf_queuespec *);
> >
> > extern struct pf_status pf_status;
> > +extern struct task pf_purge_task;
> > +extern struct timeout pf_purge_to;
> > extern struct pool pf_frent_pl, pf_frag_pl;
> >
I think those declarations should go to net/pfvar_priv.h, which has been
introduced sometime ago. The idea is to split current net/pfvar.h to
two header files:
public (net/pfvar.h)
and private to be included by PF source code only (net/pfvar_priv.h)
I've just updated your patch with change I'm suggesting.
thanks and
regards
sasha
and of course O.K. sashan@
--------8<---------------8<---------------8<------------------8<--------
diff -r 8a63415bc696 src/sys/net/pf.c
--- src/sys/net/pf.c Wed Jul 26 08:30:24 2017 +0200
+++ src/sys/net/pf.c Thu Jul 27 09:00:45 2017 +0200
@@ -121,6 +121,10 @@ u_char pf_tcp_secret[16];
int pf_tcp_secret_init;
int pf_tcp_iss_off;
+int pf_npurge;
+struct task pf_purge_task = TASK_INITIALIZER(pf_purge, &pf_npurge);
+struct timeout pf_purge_to = TIMEOUT_INITIALIZER(pf_purge_timeout, NULL);
+
enum pf_test_status {
PF_TEST_FAIL = -1,
PF_TEST_OK,
@@ -1200,36 +1204,44 @@ pf_purge_expired_rules(void)
}
void
-pf_purge_thread(void *v)
+pf_purge_timeout(void *unused)
+{
+ task_add(softnettq, &pf_purge_task);
+}
+
+void
+pf_purge(void *xnloops)
{
- int nloops = 0, s;
-
- for (;;) {
- tsleep(pf_purge_thread, PWAIT, "pftm", 1 * hz);
-
- NET_LOCK(s);
-
- PF_LOCK();
- /* process a fraction of the state table every second */
- pf_purge_expired_states(1 + (pf_status.states
- / pf_default_rule.timeout[PFTM_INTERVAL]));
-
- /* purge other expired types every PFTM_INTERVAL seconds */
- if (++nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
- pf_purge_expired_src_nodes(0);
- pf_purge_expired_rules();
- }
- PF_UNLOCK();
- /*
- * Fragments don't require PF_LOCK(), they use their own lock.
- */
- if (nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
- pf_purge_expired_fragments();
- nloops = 0;
- }
-
- NET_UNLOCK(s);
- }
+ int *nloops = xnloops;
+ int s;
+
+ KERNEL_LOCK();
+ NET_LOCK(s);
+
+ PF_LOCK();
+ /* process a fraction of the state table every second */
+ pf_purge_expired_states(1 + (pf_status.states
+ / pf_default_rule.timeout[PFTM_INTERVAL]));
+
+ /* purge other expired types every PFTM_INTERVAL seconds */
+ if (++(*nloops) >= pf_default_rule.timeout[PFTM_INTERVAL]) {
+ pf_purge_expired_src_nodes(0);
+ pf_purge_expired_rules();
+ }
+ PF_UNLOCK();
+
+ /*
+ * Fragments don't require PF_LOCK(), they use their own lock.
+ */
+ if ((*nloops) >= pf_default_rule.timeout[PFTM_INTERVAL]) {
+ pf_purge_expired_fragments();
+ *nloops = 0;
+ }
+ NET_UNLOCK(s);
+ KERNEL_UNLOCK();
+
+ if (pf_status.running)
+ timeout_add(&pf_purge_to, 1 * hz);
}
int32_t
diff -r 8a63415bc696 src/sys/net/pf_ioctl.c
--- src/sys/net/pf_ioctl.c Wed Jul 26 08:30:24 2017 +0200
+++ src/sys/net/pf_ioctl.c Thu Jul 27 09:00:45 2017 +0200
@@ -231,16 +231,6 @@ pfattach(int num)
/* XXX do our best to avoid a conflict */
pf_status.hostid = arc4random();
-
- /* require process context to purge states, so perform in a thread */
- kthread_create_deferred(pf_thread_create, NULL);
-}
-
-void
-pf_thread_create(void *v)
-{
- if (kthread_create(pf_purge_thread, NULL, NULL, "pfpurge"))
- panic("pfpurge thread");
}
int
@@ -1027,6 +1017,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_status.stateid = time_second;
pf_status.stateid = pf_status.stateid << 32;
}
+ timeout_add(&pf_purge_to, 1 * hz);
pf_create_queues();
DPFPRINTF(LOG_NOTICE, "pf: started");
}
@@ -2292,7 +2283,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
pf_default_rule_new.timeout[i];
if (pf_default_rule.timeout[i] == PFTM_INTERVAL &&
pf_default_rule.timeout[i] < old)
- wakeup(pf_purge_thread);
+ task_add(softnettq, &pf_purge_task);
}
pfi_xcommit();
pf_trans_set_commit();
diff -r 8a63415bc696 src/sys/net/pfvar.h
--- src/sys/net/pfvar.h Wed Jul 26 08:30:24 2017 +0200
+++ src/sys/net/pfvar.h Thu Jul 27 09:00:45 2017 +0200
@@ -1648,7 +1648,6 @@ extern int
pf_tbladdr_setup(struct pf
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 *);
-extern void pf_purge_thread(void *);
extern void pf_purge_expired_src_nodes();
extern void pf_purge_expired_states(u_int32_t);
extern void pf_purge_expired_rules();
diff -r 8a63415bc696 src/sys/net/pfvar_priv.h
--- src/sys/net/pfvar_priv.h Wed Jul 26 08:30:24 2017 +0200
+++ src/sys/net/pfvar_priv.h Thu Jul 27 09:00:45 2017 +0200
@@ -98,6 +98,9 @@ struct pf_pdesc {
} hdr;
};
+extern struct task pf_purge_task;
+extern struct timeout pf_purge_to;
+
#ifdef WITH_PF_LOCK
extern struct rwlock pf_lock;
@@ -128,6 +131,8 @@ extern struct rwlock pf_lock;
#define PF_ASSERT_UNLOCKED() (void)(0)
#endif /* WITH_PF_LOCK */
+extern void pf_purge_timeout(void *);
+extern void pf_purge(void *);
#endif /* _KERNEL */
#endif /* _NET_PFVAR_PRIV_H_ */
--------8<---------------8<---------------8<------------------8<--------
