Re: Updated: Enabling tap interfaces in pfinet

--YNtnX2em3eM7NacX
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Fair enough. I've attached the merged patch here, and then took the
opportunity to fix up the tap mac address logic as well.

There's still a problem in using a direct mapping of the tap name to
mac address since then a virtual cable between two hosts will require
there being differently named taps. Perhaps it really needs an
additional pfnet argument for that.

Ralph.

On Wed, Jul 29, 2026 at 05:53:33PM +0200, Samuel Thibault wrote:
> Hello,
> 
> Ralph Ronnquist, le mer. 29 juil. 2026 01:07:31 +1000, a ecrit:
> > This is a follow-on patch for the tap implementation, with a bit of
> > cleanup after feedback from Alperen Erkan <[email protected]>
> 
> Please merge the two, so I just review the merged version instead of two
> separate patches.
> 
> Samuel
> 
> > From 2c68b2e7ea443c167f3d9280132fa53b40fe7d83 Mon Sep 17 00:00:00 2001
> > From: Ralph Ronnquist <[email protected]>
> > Date: Tue, 28 Jul 2026 17:50:09 +1000
> > Subject: [PATCH 2/2] Code cleanup for Ethernet level 2 implementation
> > 
> > ---
> >  pfinet/linux-src/include/linux/if_ether.h | 44 +++++++++++++++
> >  pfinet/linux-src/net/core/dev.c           |  3 +-
> >  pfinet/tunnel.c                           | 65 +++--------------------
> >  3 files changed, 54 insertions(+), 58 deletions(-)
> > 
> > diff --git a/pfinet/linux-src/include/linux/if_ether.h b/pfinet/linux-src/include/linux/if_ether.h
> > index 99bb97fa..efb566e0 100644
> > --- a/pfinet/linux-src/include/linux/if_ether.h
> > +++ b/pfinet/linux-src/include/linux/if_ether.h
> > @@ -87,6 +87,50 @@ struct ethhdr
> >  	unsigned short	h_proto;		/* packet type ID field	*/
> >  };
> >  
> > +/* Alternative Ethernet header with VLAN tagging. The header without
> > +   VLAN tagging (the original header) is found in <net/ethernet.h>.
> > +   This header modelling accounts for packets optionally having VLAN
> > +   tagging inserted before the ether_type field.
> > +*/
> > +
> > +struct ethhdr_vlan
> > +{
> > +    uint8_t  h_dest[ETH_ALEN];        /* destination eth addr */
> > +    uint8_t  h_source[ETH_ALEN];      /* source ether addr    */
> > +    uint16_t h_vlan_tag;              /* htons( 0x8100 ) */
> > +    uint16_t h_vlan_id;               /* VLAN ID */
> > +    uint16_t h_proto;                 /* packet type ID field */
> > +};
> > +
> > +/* Generic header that incorporates both without and with VLAN alternatives */
> > +union ethhdr_either {
> > +    struct ethhdr without_vlan;
> > +    struct ethhdr_vlan with_vlan;
> > +};
> > +
> > +/* Repeating from net/ethernet.h */
> > +#define ETHERTYPE_VLAN 0x8100
> > +
> > +/* Detecting presence of VLAN tag */
> > +static inline int has_vlan_tag(unsigned char *frame) {
> > +    uint16_t v = ntohs( ((struct ethhdr_vlan*)frame)->h_vlan_tag );
> > +    return ( v == ETHERTYPE_VLAN );
> > +}
> > +
> > +/* Obtain ether_type handling optional VLAN tag. */
> > +static inline uint16_t get_protocol(unsigned char *frame)
> > +{
> > +    return ntohs( has_vlan_tag(frame)?
> > +		  (((struct ethhdr_vlan*)frame)->h_proto) :
> > +		  (((struct ethhdr*)frame)->h_proto) );
> > +}
> > +
> > +/* Obtain payload base address handling optional VLAN tag */
> > +static inline unsigned char *get_ether_payload_p(unsigned char *frame) {
> > +    return frame + ( has_vlan_tag(frame)?
> > +		     sizeof(struct ethhdr_vlan) : sizeof(struct ethhdr) );
> > +}
> > +
> >  /*
> >   *	We Have changed the ethernet statistics collection data. This
> >   *	is just for partial compatibility for now.
> > diff --git a/pfinet/linux-src/net/core/dev.c b/pfinet/linux-src/net/core/dev.c
> > index 2f480865..93e5aab1 100644
> > --- a/pfinet/linux-src/net/core/dev.c
> > +++ b/pfinet/linux-src/net/core/dev.c
> > @@ -923,7 +923,8 @@ void net_bh(void)
> >  		 */
> >  
> >  		/* XXX until we figure out every place to modify.. */
> > -		if ( skb->nh.raw < skb->data || skb->nh.raw > skb->data + 20 ) 
> > +		if ( skb->nh.raw < skb->data ||
> > +		     skb->nh.raw > skb->data + sizeof(struct ethhdr_vlan)) 
> >  		    skb->h.raw = skb->nh.raw = skb->data;
> >  
> >  		if (skb->mac.raw < skb->head || skb->mac.raw > skb->data) {
> > diff --git a/pfinet/tunnel.c b/pfinet/tunnel.c
> > index b22cd2a2..b7e4ab53 100644
> > --- a/pfinet/tunnel.c
> > +++ b/pfinet/tunnel.c
> > @@ -58,8 +58,6 @@ struct tunnel_device
> >    struct net_device_stats stats;
> >  };
> >  
> > -static int istap = 0;
> > -
> >  /* Linked list of all tunnel devices.  */
> >  struct tunnel_device *tunnel_dev;
> >  
> > @@ -186,8 +184,6 @@ setup_tunnel_device (char *name, struct device **device)
> >    else
> >      base_name = name;
> >    
> > -  istap = ( strncmp( base_name, "tap", 3 ) == 0 );
> > -
> >    dev->name = strdup (base_name);
> >  
> >    dev->get_stats = tunnel_get_stats;
> > @@ -198,7 +194,7 @@ setup_tunnel_device (char *name, struct device **device)
> >    dev->hard_start_xmit = tunnel_xmit;
> >    dev->set_multicast_list = tunnel_set_multi;
> >  
> > -  if ( istap ) {
> > +  if ( strncmp( base_name, "tap", 3 ) == 0 ) {
> >        /* The setup for a tap is most similar the ethernet.c */
> >        dev->hard_header = eth_header;
> >        dev->rebuild_header = eth_rebuild_header;
> > @@ -214,12 +210,8 @@ setup_tunnel_device (char *name, struct device **device)
> >        dev->change_flags = tunnel_change_flags;
> >  
> >        // We *know* that ETH_ALEN == 6
> > -      char *nametail = name + strlen(name) - 4;
> > -      assert_backtrace ( nametail >= name );
> > -      memcpy( dev->dev_addr, (char[ETH_ALEN]) {
> > -	      0x06, 'H', 'u', 'r', 'd', 0xff & ( nametail[3] - '0' )
> > -	  }, ETH_ALEN );
> > -
> > +      memcpy( dev->dev_addr, "\006Tap\000\000", 6 );
> > +      sscanf( name, "tap%u", (unsigned int*)&dev->dev_addr[4] );
> >    } else {
> >        /* These are the ones set by drivers/net/ppp_generic.c::ppp_net_init.  */
> >        dev->hard_header = 0;
> > @@ -396,48 +388,6 @@ trivfs_S_io_read (struct trivfs_protid *cred,
> >    return 0;
> >  }
> >  
> > -/* Alternative Ethernet header with VLAN tagging. The header without
> > -   VLAN tagging (the original header) is found in <net/ethernet.h>.
> > -   This header modelling accounts for packets optionally having VLAN
> > -   tagging inserted before the ether_type field.
> > -*/
> > -
> > -struct ethhdr_vlan {
> > -    uint8_t  h_dest[ETH_ALEN];        /* destination eth addr */
> > -    uint8_t  h_source[ETH_ALEN];      /* source ether addr    */
> > -    uint16_t h_vlan_tag;              /* htons( 0x8100 ) */
> > -    uint16_t h_vlan_id;               /* VLAN ID */
> > -    uint16_t h_proto;                 /* packet type ID field */
> > -};
> > -
> > -union ethhdr_either {
> > -    struct ethhdr without_vlan;
> > -    struct ethhdr_vlan with_vlan;
> > -};
> > -
> > -/* Repeating from net/ethernet.h */
> > -#define ETHERTYPE_VLAN 0x8100
> > -
> > -/* Detecting presence of VLAN tag */
> > -static inline int has_vlan_tag(unsigned char *frame) {
> > -    uint16_t v = ntohs( ((struct ethhdr_vlan*)frame)->h_vlan_tag );
> > -    return ( v == ETHERTYPE_VLAN );
> > -}
> > -
> > -/* Obtain ether_type handling optional VLAN tag. */
> > -static inline uint16_t get_protocol(unsigned char *frame)
> > -{
> > -    return ntohs( has_vlan_tag(frame)?
> > -		  (((struct ethhdr_vlan*)frame)->h_proto) :
> > -		  (((struct ethhdr*)frame)->h_proto) );
> > -}
> > -
> > -/* Obtain payload base address handling optional VLAN tag */
> > -static inline unsigned char *get_ether_payload_p(unsigned char *frame) {
> > -    return frame + ( has_vlan_tag(frame)?
> > -		     sizeof(struct ethhdr_vlan) : sizeof(struct ethhdr*) );
> > -}
> > -
> >  /* Write data to an IO object.  If offset is -1, write at the object
> >     maintained file pointer.  If the object is not seekable, offset is
> >     ignored.  The amount successfully written is returned in amount.  A
> > @@ -464,14 +414,15 @@ trivfs_S_io_write (struct trivfs_protid *cred,
> >    if (! (cred->po->openmodes & O_WRITE))
> >      return EBADF;
> >  
> > -  if ( istap && ( datalen < 18 ) ) // Too small Ethernet packet
> > -    return EBADF;
> > -
> >    if (cred->pi.class != tunnel_class)
> >      return EOPNOTSUPP;
> >  
> >    tdev = (struct tunnel_device *) cred->po->cntl->hook;
> >  
> > +  if ((tdev->dev.hard_header != 0) && (datalen < 18)) // small packet
> > +    return EBADF;
> > +
> > +
> >    pthread_mutex_lock (&tdev->lock);
> >  
> >    pthread_mutex_lock (&net_bh_lock);
> > @@ -484,7 +435,7 @@ trivfs_S_io_write (struct trivfs_protid *cred,
> >  
> >    /* Drop it on the queue. */
> >    skb->mac.raw = skb->data;
> > -  if ( istap ) {
> > +  if ( tdev->dev.hard_header != 0 ) {
> >        skb->protocol = get_protocol(skb->data);
> >        skb->h.raw = skb->nh.raw = get_ether_payload_p(skb->data);
> >    } else {
> > -- 
> > 2.53.0
> > 
> 
> 
> -- 
> Samuel
>   bien sûr que ça convient mieux à tout le monde
>   enfin, dans la mesure où tout le monde c'est comme moi
>  -+- le consensus, c'est facile -+-

--YNtnX2em3eM7NacX
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="0001-Implementation-of-tap-device-for-Ethernet-level-2-in.patch"

From 9bed4aa661cd3e52500718022f581c4c04c2416a Mon Sep 17 00:00:00 2001
From: Ralph Ronnquist <[email protected]>
Date: Thu, 30 Jul 2026 09:01:21 +1000
Subject: [PATCH] Implementation of tap device for Ethernet level 2 in pfinet

---
 pfinet/linux-src/include/linux/if_ether.h | 44 +++++++++++++
 pfinet/linux-src/net/core/dev.c           |  4 +-
 pfinet/main.c                             |  2 +
 pfinet/tunnel.c                           | 77 ++++++++++++++++++-----
 4 files changed, 110 insertions(+), 17 deletions(-)

diff --git a/pfinet/linux-src/include/linux/if_ether.h b/pfinet/linux-src/include/linux/if_ether.h
index 99bb97fa2..efb566e05 100644
--- a/pfinet/linux-src/include/linux/if_ether.h
+++ b/pfinet/linux-src/include/linux/if_ether.h
@@ -87,6 +87,50 @@ struct ethhdr
 	unsigned short	h_proto;		/* packet type ID field	*/
 };
 
+/* Alternative Ethernet header with VLAN tagging. The header without
+   VLAN tagging (the original header) is found in <net/ethernet.h>.
+   This header modelling accounts for packets optionally having VLAN
+   tagging inserted before the ether_type field.
+*/
+
+struct ethhdr_vlan
+{
+    uint8_t  h_dest[ETH_ALEN];        /* destination eth addr */
+    uint8_t  h_source[ETH_ALEN];      /* source ether addr    */
+    uint16_t h_vlan_tag;              /* htons( 0x8100 ) */
+    uint16_t h_vlan_id;               /* VLAN ID */
+    uint16_t h_proto;                 /* packet type ID field */
+};
+
+/* Generic header that incorporates both without and with VLAN alternatives */
+union ethhdr_either {
+    struct ethhdr without_vlan;
+    struct ethhdr_vlan with_vlan;
+};
+
+/* Repeating from net/ethernet.h */
+#define ETHERTYPE_VLAN 0x8100
+
+/* Detecting presence of VLAN tag */
+static inline int has_vlan_tag(unsigned char *frame) {
+    uint16_t v = ntohs( ((struct ethhdr_vlan*)frame)->h_vlan_tag );
+    return ( v == ETHERTYPE_VLAN );
+}
+
+/* Obtain ether_type handling optional VLAN tag. */
+static inline uint16_t get_protocol(unsigned char *frame)
+{
+    return ntohs( has_vlan_tag(frame)?
+		  (((struct ethhdr_vlan*)frame)->h_proto) :
+		  (((struct ethhdr*)frame)->h_proto) );
+}
+
+/* Obtain payload base address handling optional VLAN tag */
+static inline unsigned char *get_ether_payload_p(unsigned char *frame) {
+    return frame + ( has_vlan_tag(frame)?
+		     sizeof(struct ethhdr_vlan) : sizeof(struct ethhdr) );
+}
+
 /*
  *	We Have changed the ethernet statistics collection data. This
  *	is just for partial compatibility for now.
diff --git a/pfinet/linux-src/net/core/dev.c b/pfinet/linux-src/net/core/dev.c
index b47c50270..93e5aab1f 100644
--- a/pfinet/linux-src/net/core/dev.c
+++ b/pfinet/linux-src/net/core/dev.c
@@ -923,7 +923,9 @@ void net_bh(void)
 		 */
 
 		/* XXX until we figure out every place to modify.. */
-		skb->h.raw = skb->nh.raw = skb->data;
+		if ( skb->nh.raw < skb->data ||
+		     skb->nh.raw > skb->data + sizeof(struct ethhdr_vlan)) 
+		    skb->h.raw = skb->nh.raw = skb->data;
 
 		if (skb->mac.raw < skb->head || skb->mac.raw > skb->data) {
 			printk(KERN_CRIT "%s: wrong mac.raw ptr, proto=%04x\n", skb->dev->name, skb->protocol);
diff --git a/pfinet/main.c b/pfinet/main.c
index cd50950b7..233735a3b 100644
--- a/pfinet/main.c
+++ b/pfinet/main.c
@@ -244,6 +244,8 @@ find_device (char *name, struct device **device)
 
   if (strncmp(base_name, "tun", 3) == 0)
     setup_tunnel_device (name, device);
+  else if (strncmp(base_name, "tap", 3) == 0)
+    setup_tunnel_device (name, device);
   else if (strncmp(base_name, "dummy", 5) == 0)
     setup_dummy_device (name, device);
   else
diff --git a/pfinet/tunnel.c b/pfinet/tunnel.c
index 4301d054f..6532cac5c 100644
--- a/pfinet/tunnel.c
+++ b/pfinet/tunnel.c
@@ -58,7 +58,6 @@ struct tunnel_device
   struct net_device_stats stats;
 };
 
-
 /* Linked list of all tunnel devices.  */
 struct tunnel_device *tunnel_dev;
 
@@ -146,6 +145,14 @@ tunnel_xmit (struct sk_buff *skb, struct device *dev)
   return 0;
 }
 
+/* Obtained from ethernet.c and edited */
+static int
+tunnel_change_flags (struct device *dev, short flags)
+{
+    /* unsupported, but ignore */
+  return 0;
+}
+
 void
 setup_tunnel_device (char *name, struct device **device)
 {
@@ -168,16 +175,17 @@ setup_tunnel_device (char *name, struct device **device)
   tunnel_dev = tdev;
 
   *device = dev = &tdev->dev;
+  dev->priv = tdev;
+  skb_queue_head_init(&tdev->xq); // Initialise tdev->xq
 
   base_name = strrchr (name, '/');
   if (base_name)
     base_name++;
   else
     base_name = name;
-
+  
   dev->name = strdup (base_name);
 
-  dev->priv = tdev;
   dev->get_stats = tunnel_get_stats;
 
   /* Functions.  These ones are the true "hardware layer" in Linux.  */
@@ -186,21 +194,44 @@ setup_tunnel_device (char *name, struct device **device)
   dev->hard_start_xmit = tunnel_xmit;
   dev->set_multicast_list = tunnel_set_multi;
 
-  /* These are the ones set by drivers/net/ppp_generic.c::ppp_net_init.  */
-  dev->hard_header = 0;
-  dev->hard_header_len = 0;
-  dev->mtu = PPP_MTU;
-  dev->addr_len = 0;
-  dev->tx_queue_len = 3;
-  dev->type = ARPHRD_PPP;
-  dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
+  if ( strncmp( base_name, "tap", 3 ) == 0 ) {
+      /* The setup for a tap is most similar the ethernet.c */
+      dev->hard_header = eth_header;
+      dev->rebuild_header = eth_rebuild_header;
+      dev->hard_header_cache = eth_header_cache;
+      dev->header_cache_update = eth_header_cache_update;
+      dev->hard_header_parse = eth_header_parse;
+      dev->hard_header_len = ETH_HLEN;
+      dev->mtu = PPP_MTU;
+      dev->addr_len = ETH_ALEN;
+      dev->tx_queue_len = 3;
+      dev->type = ARPHRD_ETHER;
+      dev->flags = IFF_BROADCAST | IFF_MULTICAST | IFF_ALLMULTI;
+      dev->change_flags = tunnel_change_flags;
+
+      // We *know* that ETH_ALEN == 6
+      unsigned char num[ 4 ]; // 
+      sscanf( name, "tap%u", (unsigned int*)&num );
+      memcpy( dev->dev_addr, (char[6]) {
+	      0x06, 'T', num[3], num[2], num[1], num[0] }, 6 );
+  } else {
+      /* These are the ones set by drivers/net/ppp_generic.c::ppp_net_init.  */
+      dev->hard_header = 0;
+      dev->hard_header_len = 0;
+      dev->mtu = PPP_MTU;
+      dev->addr_len = 0;
+      dev->tx_queue_len = 3;
+      dev->type = ARPHRD_ETHER;
+      dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
+      dev->change_flags = tunnel_change_flags;
+  }
 
   dev_init_buffers (dev);
 
   if (base_name != name)
     tdev->devname = strdup (name);
   else
-    /* Setting up the translator at /dev/tunX.  */
+    /* Setting up the translator at /dev/tunX or /dev/tapX  */
     asprintf (&tdev->devname, "/dev/%s", tdev->dev.name);
   tdev->underlying = file_name_lookup (tdev->devname, O_CREAT|O_NOTRANS, 0664);
 
@@ -299,7 +330,8 @@ trivfs_S_io_read (struct trivfs_protid *cred,
   /* Deny access if they have bad credentials. */
   if (! cred)
     return EOPNOTSUPP;
-  else if (! (cred->po->openmodes & O_READ))
+
+  if (! (cred->po->openmodes & O_READ))
     return EBADF;
 
   if (cred->pi.class != tunnel_class)
@@ -380,7 +412,8 @@ trivfs_S_io_write (struct trivfs_protid *cred,
   /* Deny access if they have bad credentials. */
   if (! cred)
     return EOPNOTSUPP;
-  else if (! (cred->po->openmodes & O_WRITE))
+
+  if (! (cred->po->openmodes & O_WRITE))
     return EBADF;
 
   if (cred->pi.class != tunnel_class)
@@ -388,6 +421,10 @@ trivfs_S_io_write (struct trivfs_protid *cred,
 
   tdev = (struct tunnel_device *) cred->po->cntl->hook;
 
+  if ((tdev->dev.hard_header != 0) && (datalen < 18)) // small packet
+    return EBADF;
+
+
   pthread_mutex_lock (&tdev->lock);
 
   pthread_mutex_lock (&net_bh_lock);
@@ -400,7 +437,14 @@ trivfs_S_io_write (struct trivfs_protid *cred,
 
   /* Drop it on the queue. */
   skb->mac.raw = skb->data;
-  skb->protocol = htons (ETH_P_IP);
+  if ( tdev->dev.hard_header != 0 ) {
+      skb->protocol = get_protocol(skb->data);
+      skb->h.raw = skb->nh.raw = get_ether_payload_p(skb->data);
+  } else {
+      skb->protocol = htons(ETH_P_IP);
+      skb->h.raw = skb->nh.raw = skb->data;
+  }
+
   netif_rx (skb);
   pthread_mutex_unlock (&net_bh_lock);
 
@@ -424,7 +468,8 @@ trivfs_S_io_readable (struct trivfs_protid *cred,
   /* Deny access if they have bad credentials. */
   if (! cred)
     return EOPNOTSUPP;
-  else if (! (cred->po->openmodes & O_READ))
+
+  if (! (cred->po->openmodes & O_READ))
     return EBADF;
 
   if (cred->pi.class != tunnel_class)
-- 
2.47.3


--YNtnX2em3eM7NacX--