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!