Fix the problem with the Cisco Aironet 340 PCMCIA card. Most newer drivers

for Windows are deserialized miniports. Such drivers maintain their own
queues and do their own locking. This particular driver is not deserialized
though, and we need special support to handle it correctly.

Typically, in the ndis_rxeof() handler, we pass all incoming packets
directly to (*ifp->if_input)(). This in turn may cause another thread
to run and preempt us, and the packet may actually be processed and
then released before we even exit the ndis_rxeof() routine. The
problem with this is that releasing a packet calls the ndis_return_packet()
function, which hands the packet and its buffers back to the driver.
Calling ndis_return_packet() before ndis_rxeof() returns will screw
up the driver's internal queues since, not being deserialized,
it does no locking.

To avoid this problem, if we detect a serialized driver (by checking
the attribute flags passed to NdisSetAttributesEx(), we use an alternate
ndis_rxeof() handler, ndis_rxeof_serial(), which puts the call to
(*ifp->if_input)() on the NDIS SWI work queue. This guarantees the
packet won't be processed until after ndis_rxeof_serial() returns.

Note that another approach is to always copy the packet data into
another mbuf and just let the driver retain ownership of the ndis_packet
structure (ndis_return_packet() never needs to be called in this
case). I'm not sure which method is faster.

2 files changed, 124 insertions, 0 deletions

diff --git a/sys/compat/ndis/subr_ndis.c b/sys/compat/ndis/subr_ndis.c
index 80f5180..8543c8d 100644
--- a/sys/compat/ndis/subr_ndis.c
+++ b/sys/compat/ndis/subr_ndis.c
@@ -472,6 +472,7 @@ ndis_setattr_ex(adapter_handle, adapter_ctx, hangsecs,
 	block = (ndis_miniport_block *)adapter_handle;
 	block->nmb_miniportadapterctx = adapter_ctx;
 	block->nmb_checkforhangsecs = hangsecs;
+	block->nmb_flags = flags;
 
 	return(NDIS_STATUS_SUCCESS);
 }
diff --git a/sys/dev/if_ndis/if_ndis.c b/sys/dev/if_ndis/if_ndis.c
index e9edb8b..929f793 100644
--- a/sys/dev/if_ndis/if_ndis.c
+++ b/sys/dev/if_ndis/if_ndis.c
@@ -86,10 +86,14 @@ int ndis_suspend		(device_t);
 int ndis_resume			(device_t);
 void ndis_shutdown		(device_t);
 
+static void ndis_serial_input	(void *);
+
 static __stdcall void ndis_txeof	(ndis_handle,
 	ndis_packet *, ndis_status);
 static __stdcall void ndis_rxeof	(ndis_handle,
 	ndis_packet **, uint32_t);
+static __stdcall void ndis_rxeof_serial	(ndis_handle,
+	ndis_packet **, uint32_t);
 static __stdcall void ndis_linksts	(ndis_handle,
 	ndis_status, void *, uint32_t);
 static __stdcall void ndis_linksts_done	(ndis_handle);
@@ -411,6 +415,14 @@ ndis_attach(dev)
 		goto fail;
 	}
 
+	/*
+	 * Check to see if this driver is deserialized or
+	 * not. If not, we need to do use a special serialized
+	 * receive handler.
+	 */
+	if (!(sc->ndis_block.nmb_flags & NDIS_ATTRIBUTE_DESERIALIZE))
+		sc->ndis_block.nmb_pktind_func = ndis_rxeof_serial;
+
 	/* Reset the adapter. */
 	ndis_reset_nic(sc);
 
@@ -835,6 +847,15 @@ ndis_rxeof(adapter, packets, pktcnt)
 		} else {
 			if (p->np_oob.npo_status == NDIS_STATUS_RESOURCES) {
 				m = m_dup(m0, M_DONTWAIT);
+				/*
+				 * NOTE: we want to destroy the mbuf here, but
+				 * we don't actually want to return it to the
+				 * driver via the return packet handler. By
+				 * bumping np_refcnt, we can prevent the
+				 * ndis_return_packet() routine from actually
+				 * doing anything.
+				 */
+				p->np_refcnt++;
 				m_freem(m0);
 				if (m == NULL)
 					ifp->if_ierrors++;
@@ -872,6 +893,108 @@ ndis_rxeof(adapter, packets, pktcnt)
 	return;
 }
 
+static void
+ndis_serial_input(arg)
+	void			*arg;
+{
+	struct mbuf		*m;
+	struct ifnet		*ifp;
+
+	m = arg;
+	ifp = m->m_pkthdr.rcvif;
+	(*ifp->if_input)(ifp, m);
+
+	return;
+}
+
+/*
+ * Special receive handler for serialized miniports. To really serialize
+ * things, we have to make sure not to try and return packets to the driver
+ * until after this routine returns. The best way to do that is put the
+ * call to (*ifp->if_input)() on the ndis swi work queue. In theory,
+ * we could also copy the packet. I'm not sure which is faster.
+ */
+
+__stdcall static void
+ndis_rxeof_serial(adapter, packets, pktcnt)
+	ndis_handle		adapter;
+	ndis_packet		**packets;
+	uint32_t		pktcnt;
+{
+	struct ndis_softc	*sc;
+	ndis_miniport_block	*block;
+	ndis_packet		*p;
+	uint32_t		s;
+	ndis_tcpip_csum		*csum;
+	struct ifnet		*ifp;
+	struct mbuf		*m0, *m;
+	int			i;
+
+	block = (ndis_miniport_block *)adapter;
+	sc = (struct ndis_softc *)(block->nmb_ifp);
+	ifp = block->nmb_ifp;
+
+	for (i = 0; i < pktcnt; i++) {
+		p = packets[i];
+		/* Stash the softc here so ptom can use it. */
+		p->np_softc = sc;
+		if (ndis_ptom(&m0, p)) {
+			device_printf (sc->ndis_dev, "ptom failed\n");
+			if (p->np_oob.npo_status == NDIS_STATUS_SUCCESS)
+				ndis_return_packet(sc, p);
+		} else {
+			if (p->np_oob.npo_status == NDIS_STATUS_RESOURCES) {
+				m = m_dup(m0, M_DONTWAIT);
+				/*
+				 * NOTE: we want to destroy the mbuf here, but
+				 * we don't actually want to return it to the
+				 * driver via the return packet handler. By
+				 * bumping np_refcnt, we can prevent the
+				 * ndis_return_packet() routine from actually
+				 * doing anything.
+				 */
+				p->np_refcnt++;
+				m_freem(m0);
+				if (m == NULL)
+					ifp->if_ierrors++;
+				else
+					m0 = m;
+			} else
+				p->np_oob.npo_status = NDIS_STATUS_PENDING;
+			m0->m_pkthdr.rcvif = ifp;
+
+			/* Deal with checksum offload. */
+
+			if (ifp->if_capenable & IFCAP_RXCSUM &&
+			    p->np_ext.npe_info[ndis_tcpipcsum_info] != NULL) {
+				s = (uintptr_t)
+			 	    p->np_ext.npe_info[ndis_tcpipcsum_info];
+				csum = (ndis_tcpip_csum *)&s;
+				if (csum->u.ntc_rxflags &
+				    NDIS_RXCSUM_IP_PASSED)
+					m0->m_pkthdr.csum_flags |=
+					    CSUM_IP_CHECKED|CSUM_IP_VALID;
+				if (csum->u.ntc_rxflags &
+				    (NDIS_RXCSUM_TCP_PASSED |
+				    NDIS_RXCSUM_UDP_PASSED)) {
+					m0->m_pkthdr.csum_flags |=
+					    CSUM_DATA_VALID|CSUM_PSEUDO_HDR;
+					m0->m_pkthdr.csum_data = 0xFFFF;
+				}
+			}
+
+			if (ndis_sched(ndis_serial_input, m0, NDIS_SWI)) {
+				p->np_refcnt++;
+				m_freem(m0);
+				ifp->if_ierrors++;
+				p->np_oob.npo_status = NDIS_STATUS_SUCCESS;
+			} else
+				ifp->if_ipackets++;
+		}
+	}
+
+	return;
+}
 /*
  * A frame was downloaded to the chip. It's safe for us to clean up
  * the list buffers.