Re: svn commit: r353057 - head/sys/net

2019-10-06 Thread Allan Jude
On 2019-10-04 08:57, Kyle Evans wrote:
> On Thu, Oct 3, 2019 at 12:54 PM Kyle Evans  wrote:
>>
>> Author: kevans
>> Date: Thu Oct  3 17:54:00 2019
>> New Revision: 353057
>> URL: https://svnweb.freebsd.org/changeset/base/353057
>>
>> Log:
>>   if_tuntap: create /dev aliases when a tuntap device gets renamed
>>
>>   Currently, if you do:
>>
>>   $ ifconfig tun0 create
>>   $ ifconfig tun0 name wg0
>>   $ ls -l /dev | egrep 'wg|tun'
>>
>>   You will see tun0, but no wg0. In fact, it's slightly more annoying to make
>>   the association between the new name and the old name in order to open the
>>   device (if it hadn't been opened during the rename).
>>
>>   Register an eventhandler for ifnet_arrival_events and catch interface
>>   renames. We can determine if the ifnet is a tun easily enough from the
>>   if_dname, which matches the cevsw.d_name from the associated tuntap_driver.
>>
>>   Some locking dance is required because renames don't require the device to
>>   be opened, so it could go away in the middle of handling the ioctl, but as
>>   soon as we've verified this isn't the case we can attempt to busy the tun
>>   and either bail out if the tun device is dying, or we can proceed with the
>>   rename.
>>
>>   We only create these aliases on a best-effort basis. Renaming a tun device
>>   to "usbctl", which doesn't exist as an ifnet but does as a /dev, is clearly
>>   not that disastrous, but we can't and won't create a /dev for that.
>>
> 
> It's been brought to my attention that I actually had a PR that I took
> six months ago that this should've belonged to.
> 
> PR: 219746
> 

Thanks for this, I was having similar problems with this trying to use
wireguard inside a VNET jail, so it was even harder to find and destroy
the correct interface.

-- 
Allan Jude



signature.asc
Description: OpenPGP digital signature


Re: svn commit: r353057 - head/sys/net

2019-10-04 Thread Kyle Evans
On Thu, Oct 3, 2019 at 12:54 PM Kyle Evans  wrote:
>
> Author: kevans
> Date: Thu Oct  3 17:54:00 2019
> New Revision: 353057
> URL: https://svnweb.freebsd.org/changeset/base/353057
>
> Log:
>   if_tuntap: create /dev aliases when a tuntap device gets renamed
>
>   Currently, if you do:
>
>   $ ifconfig tun0 create
>   $ ifconfig tun0 name wg0
>   $ ls -l /dev | egrep 'wg|tun'
>
>   You will see tun0, but no wg0. In fact, it's slightly more annoying to make
>   the association between the new name and the old name in order to open the
>   device (if it hadn't been opened during the rename).
>
>   Register an eventhandler for ifnet_arrival_events and catch interface
>   renames. We can determine if the ifnet is a tun easily enough from the
>   if_dname, which matches the cevsw.d_name from the associated tuntap_driver.
>
>   Some locking dance is required because renames don't require the device to
>   be opened, so it could go away in the middle of handling the ioctl, but as
>   soon as we've verified this isn't the case we can attempt to busy the tun
>   and either bail out if the tun device is dying, or we can proceed with the
>   rename.
>
>   We only create these aliases on a best-effort basis. Renaming a tun device
>   to "usbctl", which doesn't exist as an ifnet but does as a /dev, is clearly
>   not that disastrous, but we can't and won't create a /dev for that.
>

It's been brought to my attention that I actually had a PR that I took
six months ago that this should've belonged to.

PR: 219746
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


svn commit: r353057 - head/sys/net

2019-10-03 Thread Kyle Evans
Author: kevans
Date: Thu Oct  3 17:54:00 2019
New Revision: 353057
URL: https://svnweb.freebsd.org/changeset/base/353057

Log:
  if_tuntap: create /dev aliases when a tuntap device gets renamed
  
  Currently, if you do:
  
  $ ifconfig tun0 create
  $ ifconfig tun0 name wg0
  $ ls -l /dev | egrep 'wg|tun'
  
  You will see tun0, but no wg0. In fact, it's slightly more annoying to make
  the association between the new name and the old name in order to open the
  device (if it hadn't been opened during the rename).
  
  Register an eventhandler for ifnet_arrival_events and catch interface
  renames. We can determine if the ifnet is a tun easily enough from the
  if_dname, which matches the cevsw.d_name from the associated tuntap_driver.
  
  Some locking dance is required because renames don't require the device to
  be opened, so it could go away in the middle of handling the ioctl, but as
  soon as we've verified this isn't the case we can attempt to busy the tun
  and either bail out if the tun device is dying, or we can proceed with the
  rename.
  
  We only create these aliases on a best-effort basis. Renaming a tun device
  to "usbctl", which doesn't exist as an ifnet but does as a /dev, is clearly
  not that disastrous, but we can't and won't create a /dev for that.

Modified:
  head/sys/net/if_tuntap.c

Modified: head/sys/net/if_tuntap.c
==
--- head/sys/net/if_tuntap.cThu Oct  3 17:46:27 2019(r353056)
+++ head/sys/net/if_tuntap.cThu Oct  3 17:54:00 2019(r353057)
@@ -106,6 +106,7 @@ struct tuntap_driver;
  */
 struct tuntap_softc {
TAILQ_ENTRY(tuntap_softc)tun_list;
+   struct cdev *tun_alias;
struct cdev *tun_dev;
u_short  tun_flags; /* misc flags */
 #defineTUN_OPEN0x0001
@@ -149,7 +150,8 @@ struct tuntap_softc {
  * which are static after setup.
  */
 static struct mtx tunmtx;
-static eventhandler_tag tag;
+static eventhandler_tag arrival_tag;
+static eventhandler_tag clone_tag;
 static const char tunname[] = "tun";
 static const char tapname[] = "tap";
 static const char vmnetname[] = "vmnet";
@@ -193,6 +195,7 @@ static int  tuntap_name2info(const char *name, int *uni
 static voidtunclone(void *arg, struct ucred *cred, char *name,
int namelen, struct cdev **dev);
 static voidtuncreate(struct cdev *dev, struct tuntap_driver *);
+static voidtunrename(void *arg, struct ifnet *ifp);
 static int tunifioctl(struct ifnet *, u_long, caddr_t);
 static voidtuninit(struct ifnet *);
 static voidtunifinit(void *xtp);
@@ -604,6 +607,7 @@ tun_destroy(struct tuntap_softc *tp)
 
CURVNET_SET(TUN2IFP(tp)->if_vnet);
 
+   /* destroy_dev will take care of any alias. */
destroy_dev(tp->tun_dev);
seldrain(>tun_rsel);
knlist_clear(>tun_rsel.si_note, 0);
@@ -682,7 +686,8 @@ tun_uninit(const void *unused __unused)
struct tuntap_softc *tp;
int i;
 
-   EVENTHANDLER_DEREGISTER(dev_clone, tag);
+   EVENTHANDLER_DEREGISTER(ifnet_arrival_event, arrival_tag);
+   EVENTHANDLER_DEREGISTER(dev_clone, clone_tag);
drain_dev_clone_events();
 
mtx_lock();
@@ -702,6 +707,24 @@ tun_uninit(const void *unused __unused)
 }
 SYSUNINIT(tun_uninit, SI_SUB_PROTO_IF, SI_ORDER_ANY, tun_uninit, NULL);
 
+static struct tuntap_driver *
+tuntap_driver_from_ifnet(const struct ifnet *ifp)
+{
+   struct tuntap_driver *drv;
+   int i;
+
+   if (ifp == NULL)
+   return (NULL);
+
+   for (i = 0; i < nitems(tuntap_drivers); ++i) {
+   drv = _drivers[i];
+   if (strcmp(ifp->if_dname, drv->cdevsw.d_name) == 0)
+   return (drv);
+   }
+
+   return (NULL);
+}
+
 static int
 tuntapmodevent(module_t mod, int type, void *data)
 {
@@ -716,9 +739,13 @@ tuntapmodevent(module_t mod, int type, void *data)
clone_setup(>clones);
drv->unrhdr = new_unrhdr(0, IF_MAXUNIT, );
}
-   tag = EVENTHANDLER_REGISTER(dev_clone, tunclone, 0, 1000);
-   if (tag == NULL)
+   arrival_tag = EVENTHANDLER_REGISTER(ifnet_arrival_event,
+  tunrename, 0, 1000);
+   if (arrival_tag == NULL)
return (ENOMEM);
+   clone_tag = EVENTHANDLER_REGISTER(dev_clone, tunclone, 0, 1000);
+   if (clone_tag == NULL)
+   return (ENOMEM);
break;
case MOD_UNLOAD:
/* See tun_uninit, so it's done after the vnet_sysuninit() */
@@ -900,6 +927,57 @@ tuncreate(struct cdev *dev, struct tuntap_driver *drv)
 
TUNDEBUG(ifp, "interface %s is created, minor = %#x\n",
ifp->if_xname, dev2unit(dev));
+}
+
+static void
+tunrename(void *arg __unused, struct