[undi] Fill in ProtType correctly in PXENV_UNDI_ISR

Determine the network-layer packet type and fill it in for UNDI
clients.  This is required by some NBPs such as emBoot's winBoot/i.

This change requires refactoring the link-layer portions of the
gPXE netdevice API, so that it becomes possible to strip the
link-layer header without passing the packet up the network stack.

src/drivers/net/ipoib.c

@@ -153,18 +153,17 @@ static struct ipoib_mac ipoib_broadcast = {
 };
 
 /**
- * Transmit IPoIB packet
+ * Add IPoIB link-layer header
  *
  * @v iobuf		I/O buffer
  * @v netdev		Network device
  * @v net_protocol	Network-layer protocol
  * @v ll_dest		Link-layer destination address
- *
- * Prepends the IPoIB link-layer header and transmits the packet.
  */
-static int ipoib_tx ( struct io_buffer *iobuf, struct net_device *netdev,
-		      struct net_protocol *net_protocol,
-		      const void *ll_dest ) {
+static int ipoib_push ( struct io_buffer *iobuf,
+			struct net_device *netdev __unused,
+			struct net_protocol *net_protocol,
+			const void *ll_dest ) {
 	struct ipoib_hdr *ipoib_hdr =
 		iob_push ( iobuf, sizeof ( *ipoib_hdr ) );
 
@@ -174,36 +173,38 @@ static int ipoib_tx ( struct io_buffer *iobuf, struct net_device *netdev,
 	ipoib_hdr->real.proto = net_protocol->net_proto;
 	ipoib_hdr->real.reserved = 0;
 
-	/* Hand off to network device */
-	return netdev_tx ( netdev, iobuf );
+	return 0;
 }
 
 /**
- * Process received IPoIB packet
- *
- * @v iobuf	I/O buffer
- * @v netdev	Network device
+ * Remove IPoIB link-layer header
  *
- * Strips off the IPoIB link-layer header and passes up to the
- * network-layer protocol.
+ * @v iobuf		I/O buffer
+ * @v netdev		Network device
+ * @v net_proto		Network-layer protocol, in network-byte order
+ * @v ll_source		Source link-layer address
+ * @ret rc		Return status code
  */
-static int ipoib_rx ( struct io_buffer *iobuf, struct net_device *netdev ) {
+static int ipoib_pull ( struct io_buffer *iobuf,
+			struct net_device *netdev __unused,
+			uint16_t *net_proto, const void **ll_source ) {
 	struct ipoib_hdr *ipoib_hdr = iobuf->data;
 
 	/* Sanity check */
 	if ( iob_len ( iobuf ) < sizeof ( *ipoib_hdr ) ) {
 		DBG ( "IPoIB packet too short for link-layer header\n" );
 		DBG_HD ( iobuf->data, iob_len ( iobuf ) );
-		free_iob ( iobuf );
 		return -EINVAL;
 	}
 
 	/* Strip off IPoIB header */
 	iob_pull ( iobuf, sizeof ( *ipoib_hdr ) );
 
-	/* Hand off to network-layer protocol */
-	return net_rx ( iobuf, netdev, ipoib_hdr->real.proto,
-			&ipoib_hdr->pseudo.peer );
+	/* Fill in required fields */
+	*net_proto = ipoib_hdr->real.proto;
+	*ll_source = &ipoib_hdr->pseudo.peer;
+
+	return 0;
 }
 
 /**
@@ -231,8 +232,8 @@ struct ll_protocol ipoib_protocol __ll_protocol = {
 	.ll_addr_len	= IPOIB_ALEN,
 	.ll_header_len	= IPOIB_HLEN,
 	.ll_broadcast	= ( uint8_t * ) &ipoib_broadcast,
-	.tx		= ipoib_tx,
-	.rx		= ipoib_rx,
+	.push		= ipoib_push,
+	.pull		= ipoib_pull,
 	.ntoa		= ipoib_ntoa,
 };
 

src/include/gpxe/netdevice.h

@@ -76,7 +76,7 @@ struct ll_protocol {
 	/** Protocol name */
 	const char *name;
 	/**
-	 * Transmit network-layer packet via network device
+	 * Add link-layer header
 	 *
 	 * @v iobuf		I/O buffer
 	 * @v netdev		Network device
@@ -85,24 +85,28 @@ struct ll_protocol {
 	 * @ret rc		Return status code
 	 *
 	 * This method should prepend in the link-layer header
-	 * (e.g. the Ethernet DIX header) and transmit the packet.
-	 * This method takes ownership of the I/O buffer.
+	 * (e.g. the Ethernet DIX header).
 	 */
-	int ( * tx ) ( struct io_buffer *iobuf, struct net_device *netdev,
-		       struct net_protocol *net_protocol,
-		       const void *ll_dest );
+	int ( * push ) ( struct io_buffer *iobuf, struct net_device *netdev,
+			 struct net_protocol *net_protocol,
+			 const void *ll_dest );
 	/**
-	 * Handle received packet
+	 * Remove link-layer header
 	 *
 	 * @v iobuf	I/O buffer
 	 * @v netdev	Network device
+	 * @v net_proto	Network-layer protocol, in network-byte order
+	 * @v ll_source	Source link-layer address
+	 * @ret rc	Return status code
 	 *
 	 * This method should strip off the link-layer header
-	 * (e.g. the Ethernet DIX header) and pass the packet to
-	 * net_rx().  This method takes ownership of the packet
-	 * buffer.
+	 * (e.g. the Ethernet DIX header) and return the protocol and
+	 * source link-layer address.  The method must not alter the
+	 * packet content, and may return the link-layer address as a
+	 * pointer to data within the packet.
 	 */
-	int ( * rx ) ( struct io_buffer *iobuf, struct net_device *netdev );
+	int ( * pull ) ( struct io_buffer *iobuf, struct net_device *netdev,
+			 uint16_t *net_proto, const void **ll_source );
 	/**
 	 * Transcribe link-layer address
 	 *

src/interface/pxe/pxe_undi.c

@@ -251,11 +251,9 @@ PXENV_EXIT_t pxenv_undi_transmit ( struct s_PXENV_UNDI_TRANSMIT
 				 datablk->TDDataLen );
 	}
 
-	/* Transmit packet */
-	if ( net_protocol == NULL ) {
-		/* Link-layer header already present */
-		rc = netdev_tx ( pxe_netdev, iobuf );
-	} else {
+	/* Add link-layer header, if required to do so */
+	if ( net_protocol != NULL ) {
+
 		/* Calculate destination address */
 		if ( undi_transmit->XmitFlag == XMT_DESTADDR ) {
 			copy_from_real ( destaddr,
@@ -267,14 +265,28 @@ PXENV_EXIT_t pxenv_undi_transmit ( struct s_PXENV_UNDI_TRANSMIT
 			DBG ( " BCAST" );
 			ll_dest = pxe_netdev->ll_protocol->ll_broadcast;
 		}
-		rc = net_tx ( iobuf, pxe_netdev, net_protocol, ll_dest );
+
+		/* Add link-layer header */
+		if ( ( rc = pxe_netdev->ll_protocol->push ( iobuf, pxe_netdev,
+							    net_protocol,
+							    ll_dest )) != 0 ){
+			free_iob ( iobuf );
+			undi_transmit->Status = PXENV_STATUS ( rc );
+			return PXENV_EXIT_FAILURE;
+		}
+	}
+
+	/* Transmit packet */
+	if ( ( rc = netdev_tx ( pxe_netdev, iobuf ) ) != 0 ) {
+		undi_transmit->Status = PXENV_STATUS ( rc );
+		return PXENV_EXIT_FAILURE;
 	}
 
 	/* Flag transmission as in-progress */
 	undi_tx_count++;
 
-	undi_transmit->Status = PXENV_STATUS ( rc );
-	return ( ( rc == 0 ) ? PXENV_EXIT_SUCCESS : PXENV_EXIT_FAILURE );
+	undi_transmit->Status = PXENV_STATUS_SUCCESS;
+	return PXENV_EXIT_SUCCESS;
 }
 
 /* PXENV_UNDI_SET_MCAST_ADDRESS
@@ -532,6 +544,13 @@ PXENV_EXIT_t pxenv_undi_get_state ( struct s_PXENV_UNDI_GET_STATE
 PXENV_EXIT_t pxenv_undi_isr ( struct s_PXENV_UNDI_ISR *undi_isr ) {
 	struct io_buffer *iobuf;
 	size_t len;
+	struct ll_protocol *ll_protocol;
+	const void *ll_source;
+	uint16_t net_proto;
+	size_t ll_hlen;
+	struct net_protocol *net_protocol;
+	unsigned int prottype;
+	int rc;
 
 	DBG ( "PXENV_UNDI_ISR" );
 
@@ -604,16 +623,46 @@ PXENV_EXIT_t pxenv_undi_isr ( struct s_PXENV_UNDI_ISR *undi_isr ) {
 		}
 		memcpy ( basemem_packet, iobuf->data, len );
 
+		/* Strip link-layer header */
+		ll_protocol = pxe_netdev->ll_protocol;
+		if ( ( rc = ll_protocol->pull ( iobuf, pxe_netdev,
+						&net_proto,
+						&ll_source ) ) != 0 ) {
+			/* Assume unknown net_proto and no ll_source */
+			net_proto = 0;
+			ll_source = NULL;
+		}
+		ll_hlen = ( len - iob_len ( iobuf ) );
+
+		/* Determine network-layer protocol */
+		switch ( net_proto ) {
+		case htons ( ETH_P_IP ):
+			net_protocol = &ipv4_protocol;
+			prottype = P_IP;
+			break;
+		case htons ( ETH_P_ARP ):
+			net_protocol = &arp_protocol;
+			prottype = P_ARP;
+			break;
+		case htons ( ETH_P_RARP ):
+			net_protocol = &rarp_protocol;
+			prottype = P_RARP;
+			break;
+		default:
+			net_protocol = NULL;
+			prottype = P_UNKNOWN;
+			break;
+		}
+		DBG ( " %s", ( net_protocol ? net_protocol->name : "RAW" ) );
+
 		/* Fill in UNDI_ISR structure */
 		undi_isr->FuncFlag = PXENV_UNDI_ISR_OUT_RECEIVE;
 		undi_isr->BufferLength = len;
 		undi_isr->FrameLength = len;
-		undi_isr->FrameHeaderLength =
-			pxe_netdev->ll_protocol->ll_header_len;
+		undi_isr->FrameHeaderLength = ll_hlen;
 		undi_isr->Frame.segment = rm_ds;
 		undi_isr->Frame.offset = __from_data16 ( basemem_packet );
-		/* Probably ought to fill in packet type */
-		undi_isr->ProtType = P_UNKNOWN;
+		undi_isr->ProtType = prottype;
 		undi_isr->PktType = XMT_DESTADDR;
 
 		/* Free packet */

src/net/ethernet.c

@@ -38,17 +38,16 @@
 static uint8_t eth_broadcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
 
 /**
- * Transmit Ethernet packet
+ * Add Ethernet link-layer header
  *
  * @v iobuf		I/O buffer
  * @v netdev		Network device
  * @v net_protocol	Network-layer protocol
  * @v ll_dest		Link-layer destination address
- *
- * Prepends the Ethernet link-layer header and transmits the packet.
  */
-static int eth_tx ( struct io_buffer *iobuf, struct net_device *netdev,
-		    struct net_protocol *net_protocol, const void *ll_dest ) {
+static int eth_push ( struct io_buffer *iobuf, struct net_device *netdev,
+		      struct net_protocol *net_protocol,
+		      const void *ll_dest ) {
 	struct ethhdr *ethhdr = iob_push ( iobuf, sizeof ( *ethhdr ) );
 
 	/* Build Ethernet header */
@@ -56,35 +55,38 @@ static int eth_tx ( struct io_buffer *iobuf, struct net_device *netdev,
 	memcpy ( ethhdr->h_source, netdev->ll_addr, ETH_ALEN );
 	ethhdr->h_protocol = net_protocol->net_proto;
 
-	/* Hand off to network device */
-	return netdev_tx ( netdev, iobuf );
+	return 0;
 }
 
 /**
- * Process received Ethernet packet
- *
- * @v iobuf	I/O buffer
- * @v netdev	Network device
+ * Remove Ethernet link-layer header
  *
- * Strips off the Ethernet link-layer header and passes up to the
- * network-layer protocol.
+ * @v iobuf		I/O buffer
+ * @v netdev		Network device
+ * @v net_proto		Network-layer protocol, in network-byte order
+ * @v ll_source		Source link-layer address
+ * @ret rc		Return status code
  */
-static int eth_rx ( struct io_buffer *iobuf, struct net_device *netdev ) {
+static int eth_pull ( struct io_buffer *iobuf,
+		      struct net_device *netdev __unused,
+		      uint16_t *net_proto, const void **ll_source ) {
 	struct ethhdr *ethhdr = iobuf->data;
 
 	/* Sanity check */
 	if ( iob_len ( iobuf ) < sizeof ( *ethhdr ) ) {
 		DBG ( "Ethernet packet too short (%zd bytes)\n",
 		      iob_len ( iobuf ) );
-		free_iob ( iobuf );
 		return -EINVAL;
 	}
 
 	/* Strip off Ethernet header */
 	iob_pull ( iobuf, sizeof ( *ethhdr ) );
 
-	/* Hand off to network-layer protocol */
-	return net_rx ( iobuf, netdev, ethhdr->h_protocol, ethhdr->h_source );
+	/* Fill in required fields */
+	*net_proto = ethhdr->h_protocol;
+	*ll_source = ethhdr->h_source;
+
+	return 0;
 }
 
 /**
@@ -110,7 +112,7 @@ struct ll_protocol ethernet_protocol __ll_protocol = {
 	.ll_addr_len	= ETH_ALEN,
 	.ll_header_len	= ETH_HLEN,
 	.ll_broadcast	= eth_broadcast,
-	.tx		= eth_tx,
-	.rx		= eth_rx,
+	.push		= eth_push,
+	.pull		= eth_pull,
 	.ntoa		= eth_ntoa,
 };

src/net/netdevice.c

@@ -439,6 +439,7 @@ struct net_device * find_netdev_by_location ( unsigned int bus_type,
  */
 int net_tx ( struct io_buffer *iobuf, struct net_device *netdev,
 	     struct net_protocol *net_protocol, const void *ll_dest ) {
+	int rc;
 
 	/* Force a poll on the netdevice to (potentially) clear any
 	 * backed-up TX completions.  This is needed on some network
@@ -447,7 +448,15 @@ int net_tx ( struct io_buffer *iobuf, struct net_device *netdev,
 	 */
 	netdev_poll ( netdev );
 
-	return netdev->ll_protocol->tx ( iobuf, netdev, net_protocol, ll_dest );
+	/* Add link-layer header */
+	if ( ( rc = netdev->ll_protocol->push ( iobuf, netdev, net_protocol,
+						ll_dest ) ) != 0 ) {
+		free_iob ( iobuf );
+		return rc;
+	}
+
+	/* Transmit packet */
+	return netdev_tx ( netdev, iobuf );
 }
 
 /**
@@ -485,6 +494,10 @@ int net_rx ( struct io_buffer *iobuf, struct net_device *netdev,
 static void net_step ( struct process *process __unused ) {
 	struct net_device *netdev;
 	struct io_buffer *iobuf;
+	struct ll_protocol *ll_protocol;
+	uint16_t net_proto;
+	const void *ll_source;
+	int rc;
 
 	/* Poll and process each network device */
 	list_for_each_entry ( netdev, &net_devices, list ) {
@@ -499,10 +512,21 @@ static void net_step ( struct process *process __unused ) {
 		 * NIC faster than they arrive.
 		 */
 		if ( ( iobuf = netdev_rx_dequeue ( netdev ) ) ) {
+
 			DBGC ( netdev, "NETDEV %p processing %p (%p+%zx)\n",
 			       netdev, iobuf, iobuf->data,
 			       iob_len ( iobuf ) );
-			netdev->ll_protocol->rx ( iobuf, netdev );
+
+			/* Remove link-layer header */
+			ll_protocol = netdev->ll_protocol;
+			if ( ( rc = ll_protocol->pull ( iobuf, netdev,
+							&net_proto,
+							&ll_source ) ) != 0 ) {
+				free_iob ( iobuf );
+				continue;
+			}
+
+			net_rx ( iobuf, netdev, net_proto, ll_source );
 		}
 	}
 }