Hello,
On Sun, Sep 09, 2018 at 10:50:08AM +0200, Alexander Bluhm wrote:
> On Sun, Sep 09, 2018 at 08:41:07AM +0200, Alexandr Nedvedicky wrote:
> > void
> > if_clone_attach(struct if_clone *ifc)
> > {
> > - rw_enter_write(&if_cloners_lock);
> > LIST_INSERT_HEAD(&if_cloners, ifc, ifc_list);
> > if_cloners_count++;
> > - rw_exit_write(&if_cloners_lock);
> > -}
>
> Could you put comment here, that the function is only called during
> boot so no lock is needed? I would prefer an assert to enforce
> that, but I don't know any that is suited.
>
> With comment, OK bluhm@
patch below adds assert... I still need to add a comment.
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index aa4c0a83ef7..a4c7efbd5ef 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -174,6 +174,9 @@ struct emul emul_native = {
sigcoderet
};
+#ifdef DIAGNOSTIC
+int pdevinit_done = 0;
+#endif
/*
* System startup; initialize the world, create process 0, mount root
@@ -401,6 +404,9 @@ main(void *framep)
for (pdev = pdevinit; pdev->pdev_attach != NULL; pdev++)
if (pdev->pdev_count > 0)
(*pdev->pdev_attach)(pdev->pdev_count);
+#ifdef DIAGNOSTIC
+ pdevinit_done = 1;
+#endif
#ifdef CRYPTO
crypto_init();
diff --git a/sys/net/if.c b/sys/net/if.c
index 0a6c4157ff9..d0228f73bcb 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -129,6 +129,8 @@
#include <net/pfvar.h>
#endif
+#include <sys/device.h>
+
void if_attachsetup(struct ifnet *);
void if_attachdomain(struct ifnet *);
void if_attach_common(struct ifnet *);
@@ -219,8 +221,6 @@ void if_idxmap_remove(struct ifnet *);
TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
-/* Serialize access to &if_cloners and if_cloners_count */
-struct rwlock if_cloners_lock = RWLOCK_INITIALIZER("ifclonerslk");
LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
int if_cloners_count;
@@ -1252,13 +1252,11 @@ if_clone_lookup(const char *name, int *unitp)
if (cp - name < IFNAMSIZ-1 && *cp == '0' && cp[1] != '\0')
return (NULL); /* unit number 0 padded */
- rw_enter_read(&if_cloners_lock);
LIST_FOREACH(ifc, &if_cloners, ifc_list) {
if (strlen(ifc->ifc_name) == cp - name &&
!strncmp(name, ifc->ifc_name, cp - name))
break;
}
- rw_exit_read(&if_cloners_lock);
if (ifc == NULL)
return (NULL);
@@ -1284,22 +1282,9 @@ if_clone_lookup(const char *name, int *unitp)
void
if_clone_attach(struct if_clone *ifc)
{
- rw_enter_write(&if_cloners_lock);
+ KASSERT(pdevinit_done == 0);
LIST_INSERT_HEAD(&if_cloners, ifc, ifc_list);
if_cloners_count++;
- rw_exit_write(&if_cloners_lock);
-}
-
-/*
- * Unregister a network interface cloner.
- */
-void
-if_clone_detach(struct if_clone *ifc)
-{
- rw_enter_write(&if_cloners_lock);
- LIST_REMOVE(ifc, ifc_list);
- if_cloners_count--;
- rw_exit_write(&if_cloners_lock);
}
/*
@@ -1314,17 +1299,13 @@ if_clone_list(struct if_clonereq *ifcr)
if ((dst = ifcr->ifcr_buffer) == NULL) {
/* Just asking how many there are. */
- rw_enter_read(&if_cloners_lock);
ifcr->ifcr_total = if_cloners_count;
- rw_exit_read(&if_cloners_lock);
return (0);
}
if (ifcr->ifcr_count < 0)
return (EINVAL);
- rw_enter_read(&if_cloners_lock);
-
ifcr->ifcr_total = if_cloners_count;
count = MIN(if_cloners_count, ifcr->ifcr_count);
@@ -1340,7 +1321,6 @@ if_clone_list(struct if_clonereq *ifcr)
dst += IFNAMSIZ;
}
- rw_exit_read(&if_cloners_lock);
return (error);
}
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index e9f69c964cb..e59f3ba9701 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -328,7 +328,6 @@ void ifafree(struct ifaddr *);
int if_isconnected(const struct ifnet *, unsigned int);
void if_clone_attach(struct if_clone *);
-void if_clone_detach(struct if_clone *);
int if_clone_create(const char *, int);
int if_clone_destroy(const char *);
diff --git a/sys/sys/device.h b/sys/sys/device.h
index 00a1f6ad2a6..56ea6fdafa9 100644
--- a/sys/sys/device.h
+++ b/sys/sys/device.h
@@ -164,6 +164,11 @@ struct pdevinit {
};
#ifdef _KERNEL
+
+#ifdef DIAGNOSTIC
+extern int pdevinit_done;
+#endif
+
extern struct devicelist alldevs; /* list of all devices */
extern int autoconf_verbose;
