Re: Adding a bpf peers_present function and "peers added/gone" callback

[Martin Husemann <martin%duskware.de@localhost>]

On Tue, May 25, 2021 at 07:03:02PM +0200, Martin Husemann wrote:
> Hey folks,
> 
> some drivers (i.e. ieee 802.11 ones) offer bpf tap points that require
> quite a bit of work just for the bpf tap and are in a hot network path.

As probably was to be expected, this design did not survive the first
implementation :-)

I learned hat we already have a quite effective expression for "any peer
present on this tap point" - our code makes sure the struct bpf_if* is NULL
if not.

Still I prefer the more explicit version and would like to provide an inline
function to test for that.

Since most of the time the bpf_if* is NULL, the interface to the new event
register/deregister function needed tiny changes - it now passes the address
of the pointer (in bpf terms: the driver pointer, where at attach time the
bpf_d pointer is stored).

The patch below works for me. (Of course the debug changes/printfs
will go away before commit.)

Any comments?

Martin


Index: bpfdesc.h
===================================================================
RCS file: /cvsroot/src/sys/net/bpfdesc.h,v
retrieving revision 1.47
diff -u -p -r1.47 bpfdesc.h
--- bpfdesc.h	11 Jun 2020 13:36:20 -0000	1.47
+++ bpfdesc.h	1 Jun 2021 13:22:59 -0000
@@ -162,6 +162,14 @@ struct bpf_d_ext {
 
 
 /*
+ * Record for each event tracker watching a tap point
+ */
+struct bpf_event_tracker {
+	SLIST_ENTRY(bpf_event_tracker) bet_entries;
+	void (*bet_notify)(struct bpf_if *, struct ifnet *, int, int);
+};
+
+/*
  * Descriptor associated with each attached hardware interface.
  */
 struct bpf_if {
@@ -179,6 +187,7 @@ struct bpf_if {
 	struct pslist_entry bif_iflist_entry;
 	struct pslist_head bif_dlist_head;
 	struct psref_target bif_psref;
+	SLIST_HEAD(, bpf_event_tracker) bif_trackers;
 #endif
 };
 
Index: bpf.h
===================================================================
RCS file: /cvsroot/src/sys/net/bpf.h,v
retrieving revision 1.75
diff -u -p -r1.75 bpf.h
--- bpf.h	11 Jun 2020 13:36:20 -0000	1.75
+++ bpf.h	1 Jun 2021 13:22:59 -0000
@@ -450,6 +450,11 @@ struct bpf_ops {
 
 	void (*bpf_mtap_softint_init)(struct ifnet *);
 	void (*bpf_mtap_softint)(struct ifnet *, struct mbuf *);
+
+	int (*bpf_register_track_event)(struct bpf_if **,
+	    void (*)(struct bpf_if *, struct ifnet *, int, int));
+	int (*bpf_deregister_track_event)(struct bpf_if **,
+	    void (*)(struct bpf_if *, struct ifnet *, int, int));
 };
 
 extern struct bpf_ops *bpf_ops;
@@ -501,6 +506,16 @@ bpf_change_type(struct ifnet *_ifp, u_in
 	bpf_ops->bpf_change_type(_ifp, _dlt, _hdrlen);
 }
 
+static __inline bool
+bpf_peers_present(struct bpf_if *dp)
+{
+	/*
+	 * Our code makes sure the driver visible point is NULL
+	 * whenever there is no listener on this tap.
+	 */
+	return dp != NULL;
+}
+
 static __inline void
 bpf_detach(struct ifnet *_ifp)
 {
@@ -535,6 +550,24 @@ bpf_mtap_softint(struct ifnet *_ifp, str
 		bpf_ops->bpf_mtap_softint(_ifp, _m);
 }
 
+static __inline int
+bpf_register_track_event(struct bpf_if **_dp,
+	    void (*_fun)(struct bpf_if *, struct ifnet *, int, int))
+{
+	if (bpf_ops->bpf_register_track_event == NULL)
+		return ENXIO;
+	return bpf_ops->bpf_register_track_event(_dp, _fun);
+}
+
+static __inline int
+bpf_deregister_track_event(struct bpf_if **_dp,
+	    void (*_fun)(struct bpf_if *, struct ifnet *, int, int))
+{
+	if (bpf_ops->bpf_deregister_track_event == NULL)
+		return ENXIO;
+	return bpf_ops->bpf_deregister_track_event(_dp, _fun);
+}
+
 void	bpf_setops(void);
 
 void	bpf_ops_handover_enter(struct bpf_ops *);
@@ -560,6 +593,12 @@ u_int	bpf_filter(const struct bpf_insn *
 
 u_int	bpf_filter_with_aux_data(const struct bpf_insn *, const u_char *, u_int, u_int, const struct bpf_aux_data *);
 
+/*
+ * events to be tracked by bpf_register_track_event callbacks
+ */
+#define	BPF_TRACK_EVENT_ATTACH	1
+#define	BPF_TRACK_EVENT_DETACH	2
+
 
 __END_DECLS
 
Index: bpf.c
===================================================================
RCS file: /cvsroot/src/sys/net/bpf.c,v
retrieving revision 1.239
diff -u -p -r1.239 bpf.c
--- bpf.c	18 Dec 2020 01:31:49 -0000	1.239
+++ bpf.c	1 Jun 2021 13:22:59 -0000
@@ -461,6 +461,7 @@ bad:
 static void
 bpf_attachd(struct bpf_d *d, struct bpf_if *bp)
 {
+	struct bpf_event_tracker *t;
 
 	KASSERT(mutex_owned(&bpf_mtx));
 	KASSERT(mutex_owned(d->bd_mtx));
@@ -473,6 +474,11 @@ bpf_attachd(struct bpf_d *d, struct bpf_
 	BPFIF_DLIST_WRITER_INSERT_HEAD(bp, d);
 
 	*bp->bif_driverp = bp;
+
+	SLIST_FOREACH(t, &bp->bif_trackers, bet_entries) {
+		t->bet_notify(bp, bp->bif_ifp, bp->bif_dlt,
+		    BPF_TRACK_EVENT_ATTACH);
+	}
 }
 
 /*
@@ -482,6 +488,7 @@ static void
 bpf_detachd(struct bpf_d *d)
 {
 	struct bpf_if *bp;
+	struct bpf_event_tracker *t;
 
 	KASSERT(mutex_owned(&bpf_mtx));
 	KASSERT(mutex_owned(d->bd_mtx));
@@ -522,7 +529,13 @@ bpf_detachd(struct bpf_d *d)
 		 */
 		*d->bd_bif->bif_driverp = NULL;
 	}
+
 	d->bd_bif = NULL;
+
+	SLIST_FOREACH(t, &bp->bif_trackers, bet_entries) {
+		t->bet_notify(bp, bp->bif_ifp, bp->bif_dlt,
+		    BPF_TRACK_EVENT_DETACH);
+	}
 }
 
 static void
@@ -2125,6 +2138,7 @@ _bpfattach(struct ifnet *ifp, u_int dlt,
 	BPF_IFLIST_ENTRY_INIT(bp);
 	PSLIST_INIT(&bp->bif_dlist_head);
 	psref_target_init(&bp->bif_psref, bpf_psref_class);
+	SLIST_INIT(&bp->bif_trackers);
 
 	BPF_IFLIST_WRITER_INSERT_HEAD(bp);
 
@@ -2132,8 +2146,8 @@ _bpfattach(struct ifnet *ifp, u_int dlt,
 
 	bp->bif_hdrlen = hdrlen;
 	mutex_exit(&bpf_mtx);
-#if 0
-	printf("bpf: %s attached\n", ifp->if_xname);
+#if 1
+	printf("bpf: %s attached with dlt %x\n", ifp->if_xname, dlt);
 #endif
 }
 
@@ -2171,6 +2185,10 @@ _bpfdetach(struct ifnet *ifp)
 	int s;
 
 	mutex_enter(&bpf_mtx);
+#if 1
+	printf("bpf: %s detached\n", ifp->if_xname);
+#endif
+
 	/* Nuke the vnodes for any open instances */
   again_d:
 	BPF_DLIST_WRITER_FOREACH(d) {
@@ -2196,6 +2214,14 @@ _bpfdetach(struct ifnet *ifp)
 			pserialize_perform(bpf_psz);
 			psref_target_destroy(&bp->bif_psref, bpf_psref_class);
 
+			while (!SLIST_EMPTY(&bp->bif_trackers)) {
+				struct bpf_event_tracker *t =
+				    SLIST_FIRST(&bp->bif_trackers);
+				SLIST_REMOVE_HEAD(&bp->bif_trackers,
+				    bet_entries);
+				kmem_free(t, sizeof(*t));
+			}
+
 			BPF_IFLIST_ENTRY_DESTROY(bp);
 			if (bp->bif_si != NULL) {
 				/* XXX NOMPSAFE: assumed running on one CPU */
@@ -2523,10 +2549,69 @@ SYSCTL_SETUP(sysctl_net_bpf_setup, "bpf 
 
 }
 
+static int
+_bpf_register_track_event(struct bpf_if **driverp,
+	    void (*_fun)(struct bpf_if *, struct ifnet *, int, int))
+{
+	struct bpf_if *bp;
+	struct bpf_event_tracker *t;
+	int ret = ENOENT;
+
+	t = kmem_zalloc(sizeof(*t), KM_SLEEP);
+	if (!t)
+		return ENOMEM;
+	t->bet_notify = _fun;
+
+	mutex_enter(&bpf_mtx);
+	BPF_IFLIST_WRITER_FOREACH(bp) {
+		if (bp->bif_driverp != driverp)
+			continue;
+		SLIST_INSERT_HEAD(&bp->bif_trackers, t, bet_entries);
+		ret = 0;
+		break;
+	}
+	mutex_exit(&bpf_mtx);
+
+	return ret;
+}
+
+static int
+_bpf_deregister_track_event(struct bpf_if **driverp,
+	    void (*_fun)(struct bpf_if *, struct ifnet *, int, int))
+{
+	struct bpf_if *bp;
+	struct bpf_event_tracker *t = NULL;
+	int ret = ENOENT;
+
+	mutex_enter(&bpf_mtx);
+	BPF_IFLIST_WRITER_FOREACH(bp) {
+		if (bp->bif_driverp != driverp)
+			continue;
+		SLIST_FOREACH(t, &bp->bif_trackers, bet_entries) {
+			if (t->bet_notify == _fun) {
+				ret = 0;
+				break;
+			}
+		}
+		if (ret == 0)
+			break;
+	}
+	if (ret == 0 && t && t->bet_notify == _fun) {
+		SLIST_REMOVE(&bp->bif_trackers, t, bpf_event_tracker,
+		    bet_entries);
+	}
+	mutex_exit(&bpf_mtx);
+	if (ret == 0)
+		kmem_free(t, sizeof(*t));
+	return ret;
+}
+
 struct bpf_ops bpf_ops_kernel = {
 	.bpf_attach =		_bpfattach,
 	.bpf_detach =		_bpfdetach,
 	.bpf_change_type =	_bpf_change_type,
+	.bpf_register_track_event = _bpf_register_track_event,
+	.bpf_deregister_track_event = _bpf_deregister_track_event,
 
 	.bpf_mtap =		_bpf_mtap,
 	.bpf_mtap2 =		_bpf_mtap2,