Re: Updated: Enabling tap interfaces in pfinet

Bradley Morgan <[email protected]> Thu, 30 Jul 2026 00:28:13 +0100
Newsgroups gmane.os.hurd.bugs
Message-ID <[email protected]>
On 30 July 2026 00:16:41 BST, Ralph Ronnquist <[email protected]> wrote:
>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.
>

I'm still concerned as to why people in hurd sends their patches as
attachments...



>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 -+-
>

Thanks!