netmap_user.h:

   add separate rx/tx ring indexes
   add ring specifier in nm_open device name

netmap.c, netmap_vale.c
   more consistent errno numbers

netmap_generic.c
   correctly handle failure in registering interfaces.

tools/tools/netmap/
   massive cleanup of the example programs
   (a lot of common code is now in netmap_user.h.)

nm_util.[ch] are going away soon.
pcap.c will also go when i commit the native netmap support for libpcap.

3 files changed, 33 insertions, 24 deletions

diff --git a/sys/dev/netmap/netmap.c b/sys/dev/netmap/netmap.c
index 70f82cee8a0f..fdd368a346fe 100644
--- a/sys/dev/netmap/netmap.c
+++ b/sys/dev/netmap/netmap.c
@@ -1052,7 +1052,7 @@ netmap_get_hw_na(struct ifnet *ifp, struct netmap_adapter **na)
 	 * to use generic adapters, we cannot satisfy the request.
 	 */
 	if (!NETMAP_CAPABLE(ifp) && i == NETMAP_ADMODE_NATIVE)
-		return EINVAL;
+		return EOPNOTSUPP;
 
 	/* Otherwise, create a generic adapter and return it,
 	 * saving the previously used netmap adapter, if any.
@@ -1090,22 +1090,19 @@ netmap_get_hw_na(struct ifnet *ifp, struct netmap_adapter **na)
 /*
  * MUST BE CALLED UNDER NMG_LOCK()
  *
- * get a refcounted reference to an interface.
+ * Get a refcounted reference to a netmap adapter attached
+ * to the interface specified by nmr.
  * This is always called in the execution of an ioctl().
  *
- * Return ENXIO if the interface does not exist, EINVAL if netmap
- * is not supported by the interface.
- * If successful, hold a reference.
- *
- * When the NIC is attached to a bridge, reference is managed
- * at na->na_bdg_refcount using ADD/DROP_BDG_REF() as well as
- * virtual ports.  Hence, on the final DROP_BDG_REF(), the NIC
- * is detached from the bridge, then ifp's refcount is dropped (this
- * is equivalent to that ifp is destroyed in case of virtual ports.
+ * Return ENXIO if the interface specified by the request does
+ * not exist, ENOTSUP if netmap is not supported by the interface,
+ * EBUSY if the interface is already attached to a bridge,
+ * EINVAL if parameters are invalid, ENOMEM if needed resources
+ * could not be allocated.
+ * If successful, hold a reference to the netmap adapter.
  *
- * This function uses if_rele() when we want to prevent the NIC from
- * being detached from the bridge in error handling.  But once refcount
- * is acquired by this function, it must be released using nm_if_rele().
+ * No reference is kept on the real interface, which may then
+ * disappear at any time.
  */
 int
 netmap_get_na(struct nmreq *nmr, struct netmap_adapter **na, int create)
@@ -1135,7 +1132,7 @@ netmap_get_na(struct nmreq *nmr, struct netmap_adapter **na, int create)
 	if (ret != NULL) {
 		/* Users cannot use the NIC attached to a bridge directly */
 		if (NETMAP_OWNED_BY_KERN(ret)) {
-			error = EINVAL;
+			error = EBUSY;
 			goto out;
 		}
 		error = 0;
diff --git a/sys/dev/netmap/netmap_generic.c b/sys/dev/netmap/netmap_generic.c
index 109a734cac9f..e695fcbd29f8 100644
--- a/sys/dev/netmap/netmap_generic.c
+++ b/sys/dev/netmap/netmap_generic.c
@@ -261,7 +261,7 @@ generic_netmap_register(struct netmap_adapter *na, int enable)
 		/* Prepare to intercept incoming traffic. */
 		error = netmap_catch_rx(na, 1);
 		if (error) {
-			D("netdev_rx_handler_register() failed");
+			D("netdev_rx_handler_register() failed (%d)", error);
 			goto register_handler;
 		}
 		ifp->if_capenable |= IFCAP_NETMAP;
@@ -283,7 +283,11 @@ generic_netmap_register(struct netmap_adapter *na, int enable)
 		rate_ctx.refcount++;
 #endif /* RATE */
 
-	} else { /* Disable netmap mode. */
+	} else if (na->tx_rings[0].tx_pool) {
+		/* Disable netmap mode. We enter here only if the previous
+		   generic_netmap_register(na, 1) was successfull.
+		   If it was not, na->tx_rings[0].tx_pool was set to NULL by the
+		   error handling code below. */
 		rtnl_lock();
 
 		ifp->if_capenable &= ~IFCAP_NETMAP;
@@ -322,7 +326,7 @@ generic_netmap_register(struct netmap_adapter *na, int enable)
 #ifdef REG_RESET
 	error = ifp->netdev_ops->ndo_open(ifp);
 	if (error) {
-		goto alloc_tx_pool;
+		goto free_tx_pools;
 	}
 #endif
 
@@ -338,6 +342,11 @@ free_tx_pools:
 			if (na->tx_rings[r].tx_pool[i])
 				m_freem(na->tx_rings[r].tx_pool[i]);
 		free(na->tx_rings[r].tx_pool, M_DEVBUF);
+		na->tx_rings[r].tx_pool = NULL;
+	}
+	netmap_mitigation_cleanup(gna);
+	for (r=0; r<na->num_rx_rings; r++) {
+		mbq_safe_destroy(&na->rx_rings[r].rx_queue);
 	}
 
 	return error;
diff --git a/sys/dev/netmap/netmap_vale.c b/sys/dev/netmap/netmap_vale.c
index cec2ac15a7e9..13a725378c28 100644
--- a/sys/dev/netmap/netmap_vale.c
+++ b/sys/dev/netmap/netmap_vale.c
@@ -515,7 +515,7 @@ netmap_get_bdg_na(struct nmreq *nmr, struct netmap_adapter **na, int create)
 	b = nm_find_bridge(name, create);
 	if (b == NULL) {
 		D("no bridges available for '%s'", name);
-		return (ENXIO);
+		return (create ? ENOMEM : ENXIO);
 	}
 
 	/* Now we are sure that name starts with the bridge's name,
@@ -547,7 +547,7 @@ netmap_get_bdg_na(struct nmreq *nmr, struct netmap_adapter **na, int create)
 	needed = 2; /* in some cases we only need 1 */
 	if (b->bdg_active_ports + needed >= NM_BDG_MAXPORTS) {
 		D("bridge full %d, cannot create new port", b->bdg_active_ports);
-		return EINVAL;
+		return ENOMEM;
 	}
 	/* record the next two ports available, but do not allocate yet */
 	cand = b->bdg_port_index[b->bdg_active_ports];
@@ -594,7 +594,7 @@ netmap_get_bdg_na(struct nmreq *nmr, struct netmap_adapter **na, int create)
 		if (NETMAP_OWNED_BY_ANY(ret)) {
 			D("NIC %s busy, cannot attach to bridge",
 				NM_IFPNAME(ifp));
-			error = EINVAL;
+			error = EBUSY;
 			goto out;
 		}
 		/* create a fake interface */
@@ -658,11 +658,13 @@ nm_bdg_attach(struct nmreq *nmr)
 	npriv = malloc(sizeof(*npriv), M_DEVBUF, M_NOWAIT|M_ZERO);
 	if (npriv == NULL)
 		return ENOMEM;
+
 	NMG_LOCK();
-	/* XXX probably netmap_get_bdg_na() */
+
 	error = netmap_get_bdg_na(nmr, &na, 1 /* create if not exists */);
 	if (error) /* no device, or another bridge or user owns the device */
 		goto unlock_exit;
+
 	if (na == NULL) { /* VALE prefix missing */
 		error = EINVAL;
 		goto unlock_exit;
@@ -707,6 +709,7 @@ nm_bdg_detach(struct nmreq *nmr)
 	if (error) { /* no device, or another bridge or user owns the device */
 		goto unlock_exit;
 	}
+
 	if (na == NULL) { /* VALE prefix missing */
 		error = EINVAL;
 		goto unlock_exit;
@@ -1945,7 +1948,7 @@ netmap_bwrap_notify(struct netmap_adapter *na, u_int ring_n, enum txrx tx, int f
 	int error = 0;
 
 	if (tx == NR_TX)
-	        return ENXIO;
+	        return EINVAL;
 
 	kring = &na->rx_rings[ring_n];
 	hw_kring = &hwna->tx_rings[ring_n];
@@ -1999,7 +2002,7 @@ netmap_bwrap_host_notify(struct netmap_adapter *na, u_int ring_n, enum txrx tx,
 	struct netmap_bwrap_adapter *bna = na->na_private;
 	struct netmap_adapter *port_na = &bna->up.up;
 	if (tx == NR_TX || ring_n != 0)
-		return ENXIO;
+		return EINVAL;
 	return netmap_bwrap_notify(port_na, port_na->num_rx_rings, NR_RX, flags);
 }