From e9f6263cd3d507155394d3e9101cc9cf18818aeb Mon Sep 17 00:00:00 2001 From: melifaro Date: Wed, 11 Jun 2014 11:27:44 +0000 Subject: [PATCH] Improve logic besides net.bpf.optimize_writers. Direct bpf(4) consumers should now work fine with this tunable turned on. In fact, the only case when optimized_writers can change program behavior is direct bpf(4) consumer setting its read filter to catch-all one. MFC after: 2 weeks Sponsored by: Yandex LLC --- sys/net/bpf.c | 75 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/sys/net/bpf.c b/sys/net/bpf.c index d9686e8a4b42..91a453a6d97b 100644 --- a/sys/net/bpf.c +++ b/sys/net/bpf.c @@ -642,6 +642,67 @@ bpf_attachd(struct bpf_d *d, struct bpf_if *bp) EVENTHANDLER_INVOKE(bpf_track, bp->bif_ifp, bp->bif_dlt, 1); } +/* + * Check if we need to upgrade our descriptor @d from write-only mode. + */ +static int +bpf_check_upgrade(u_long cmd, struct bpf_d *d, struct bpf_insn *fcode, int flen) +{ + int is_snap, need_upgrade; + + /* + * Check if we've already upgraded or new filter is empty. + */ + if (d->bd_writer == 0 || fcode == NULL) + return (0); + + need_upgrade = 0; + + /* + * Check if cmd looks like snaplen setting from + * pcap_bpf.c:pcap_open_live(). + * Note we're not checking .k value here: + * while pcap_open_live() definitely sets to to non-zero value, + * we'd prefer to treat k=0 (deny ALL) case the same way: e.g. + * do not consider upgrading immediately + */ + if (cmd == BIOCSETF && flen == 1 && fcode[0].code == (BPF_RET | BPF_K)) + is_snap = 1; + else + is_snap = 0; + + if (is_snap == 0) { + /* + * We're setting first filter and it doesn't look like + * setting snaplen. We're probably using bpf directly. + * Upgrade immediately. + */ + need_upgrade = 1; + } else { + /* + * Do not require upgrade by first BIOCSETF + * (used to set snaplen) by pcap_open_live(). + */ + + if (--d->bd_writer == 0) { + /* + * First snaplen filter has already + * been set. This is probably catch-all + * filter + */ + need_upgrade = 1; + } + } + + CTR5(KTR_NET, + "%s: filter function set by pid %d, " + "bd_writer counter %d, snap %d upgrade %d", + __func__, d->bd_pid, d->bd_writer, + is_snap, need_upgrade); + + return (need_upgrade); +} + /* * Add d to the list of active bp filters. * Reuqires bpf_attachd() to be called before @@ -1802,17 +1863,7 @@ bpf_setf(struct bpf_d *d, struct bpf_program *fp, u_long cmd) if (cmd == BIOCSETF) reset_d(d); - if (fcode != NULL) { - /* - * Do not require upgrade by first BIOCSETF - * (used to set snaplen) by pcap_open_live(). - */ - if (d->bd_writer != 0 && --d->bd_writer == 0) - need_upgrade = 1; - CTR4(KTR_NET, "%s: filter function set by pid %d, " - "bd_writer counter %d, need_upgrade %d", - __func__, d->bd_pid, d->bd_writer, need_upgrade); - } + need_upgrade = bpf_check_upgrade(cmd, d, fcode, flen); } BPFD_UNLOCK(d); if (d->bd_bif != NULL) @@ -1825,7 +1876,7 @@ bpf_setf(struct bpf_d *d, struct bpf_program *fp, u_long cmd) #endif /* Move d to active readers list. */ - if (need_upgrade) + if (need_upgrade != 0) bpf_upgraded(d); BPF_UNLOCK();