Page MenuHomeFreeBSD

tun(4)/tap(4): allow devices to be configured as transient
Needs RevisionPublic

Authored by kevans on Mar 3 2024, 8:20 PM.
Tags
None
Referenced Files
F82235609: D44200.diff
Fri, Apr 26, 7:52 PM
Unknown Object (File)
Fri, Apr 26, 3:54 AM
Unknown Object (File)
Sun, Apr 14, 5:15 AM
Unknown Object (File)
Sat, Apr 6, 8:58 AM
Unknown Object (File)
Mar 17 2024, 10:14 PM
Unknown Object (File)
Mar 17 2024, 10:12 PM
Unknown Object (File)
Mar 17 2024, 10:08 PM
Unknown Object (File)
Mar 16 2024, 7:27 AM

Details

Reviewers
kp
glebius
melifaro
Group Reviewers
network
Summary

Transient tunnel devices are removed immediately after last close, so
that an application that's created the tunnel could eliminate the need
to manually destroy the tunnel whose lifetime it's already managing.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 56420
Build 53308: arc lint + arc unit

Event Timeline

zlei added inline comments.
sys/net/if_tuntap.c
1234

What about making ifc_find_cloner_in_vnet() public and using it here ?
So we do not need complex logic in if_clone_destroyif_flags() .

glebius requested changes to this revision.Mar 11 2024, 5:50 PM
glebius added inline comments.
sys/net/if_tuntap.c
1234

We have the knowledge about the cloner inside the tuntap driver. You don't need to hack if_clone.c to achieve what you want. Let me come with a prototype quickly...

This revision now requires changes to proceed.Mar 11 2024, 5:50 PM
sys/net/if_tuntap.c
1234

Right, I had a version that looked up the cloner, but it was a little gross because you also need to switch back to the home vnet and grab the cloner from that particular vnet, effectively duplicating what's already done in if_clone.c.

Damn, this again comes to the stupid idea of interfaces traveling between vnets. Grrrr. Still, I believe it should be done in the tuntap, not in if_clone.c. The existing SLIST of cloners can be simplified a lot, let me try.

Damn, this again comes to the stupid idea of interfaces traveling between vnets. Grrrr. Still, I believe it should be done in the tuntap, not in if_clone.c. The existing SLIST of cloners can be simplified a lot, let me try.

I don't really feel that strongly about it either way, it just seemed less error prone since what if_clone finds should always match which list it's actually on, in case someone gets a wild tendency and decides that an interface moving between vnets should be reflected in the cloners instead

So, with D44307 in, you can add this:

static struct if_clone *
tuntap_cloner_from_flags(int tun_flags)
{
 
        for (u_int i = 0; i < NDRV; i++)
                if ((tun_flags & TUN_DRIVER_IDENT_MASK) ==
                    tuntap_drivers[i].ident_flags)
                        return (V_tuntap_driver_cloners[i]);
 
        return (NULL);
}

And then call

if_clone_destroyif(tuntap_cloner_from_flags(flags), ifp);

This of course won't cover the problematic traveling case. To cover that, you'd need to:

CURVNET_SET_QUIET(ifp->if_home_vnet);
cloner = tuntap_cloner_from_flags(flags)
CURVNET_RESTORE();
if_clone_destroyif(cloner, ifp);

So, with D44307 in, you can add this:

static struct if_clone *
tuntap_cloner_from_flags(int tun_flags)
{
 
        for (u_int i = 0; i < NDRV; i++)
                if ((tun_flags & TUN_DRIVER_IDENT_MASK) ==
                    tuntap_drivers[i].ident_flags)
                        return (V_tuntap_driver_cloners[i]);
 
        return (NULL);
}

And then call

if_clone_destroyif(tuntap_cloner_from_flags(flags), ifp);

This of course won't cover the problematic traveling case. To cover that, you'd need to:

CURVNET_SET_QUIET(ifp->if_home_vnet);
cloner = tuntap_cloner_from_flags(flags)
CURVNET_RESTORE();
if_clone_destroyif(cloner, ifp);

Alright, I'll drop the other review and update this one with the latter variant -- though I likely won't have any time to return to this for a couple weeks. I don't think this is a particularly urgent feature anyways, I just like the idea of allowing a program to construct a tunnel and not worry about the lifetime (and it's reportedly the default Linux behavior, so I could see how some tunnelling applications might benefit from a way to match it).