Re: Updated: Enabling tap interfaces in pfinet

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