git: 09ee0fc023c0 - main - if_clone: rework cloning KPI
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 22 Sep 2022 10:20:40 UTC
The branch main has been updated by melifaro:
URL: https://cgit.FreeBSD.org/src/commit/?id=09ee0fc023c0c7ef90cb277afbb473abe4d95a9a
commit 09ee0fc023c0c7ef90cb277afbb473abe4d95a9a
Author: Alexander V. Chernikov <melifaro@FreeBSD.org>
AuthorDate: 2022-09-22 09:37:37 +0000
Commit: Alexander V. Chernikov <melifaro@FreeBSD.org>
CommitDate: 2022-09-22 10:18:31 +0000
if_clone: rework cloning KPI
The current cloning KPI does not provide a way of creating interfaces
with parameres from within kernel. The reason is that those parameters
are passed as an opaque pointer and it is not possible to specify whether
this pointer references kernel-space or user-space.
Instead of just adding a flag, generalise the KPI to simplify the
extension process. Unify current notion of `SIMPLE` and `ADVANCED` users
by leveraging newly-added IFC_C_AUTOUNIT flag to automatically pick
unit number, which is a primary feature of the "SIMPLE" KPI.
Use extendable structures everywhere instead of passing function
pointers or parameters.
Isolate all parts of the oldKPI under `CLONE_COMPAT_13` so it can be safely
merged back to 13. Old KPI will be removed after the merge.
Differential Revision: https://reviews.freebsd.org/D36632
MFC after: 2 weeks
---
sys/net/if_clone.c | 247 +++++++++++++++++++++++++++++++++++++----------------
sys/net/if_clone.h | 41 ++++++++-
2 files changed, 213 insertions(+), 75 deletions(-)
diff --git a/sys/net/if_clone.c b/sys/net/if_clone.c
index 272a98f285d5..50664c28ff88 100644
--- a/sys/net/if_clone.c
+++ b/sys/net/if_clone.c
@@ -72,12 +72,14 @@ struct if_clone {
LIST_HEAD(, ifnet) ifc_iflist; /* (i) List of cloned interfaces */
struct mtx ifc_mtx; /* Mutex to protect members. */
- enum { SIMPLE, ADVANCED } ifc_type; /* (c) */
+ ifc_match_f *ifc_match; /* (c) Matcher function */
+ ifc_create_f *ifc_create; /* (c) Creates new interface */
+ ifc_destroy_f *ifc_destroy; /* (c) Destroys cloned interface */
+#ifdef CLONE_COMPAT_13
/* (c) Driver specific cloning functions. Called with no locks held. */
union {
struct { /* advanced cloner */
- ifc_match_t *_ifc_match;
ifc_create_t *_ifc_create;
ifc_destroy_t *_ifc_destroy;
} A;
@@ -88,23 +90,31 @@ struct if_clone {
} S;
} U;
-#define ifc_match U.A._ifc_match
-#define ifc_create U.A._ifc_create
-#define ifc_destroy U.A._ifc_destroy
+#define ifca_create U.A._ifc_create
+#define ifca_destroy U.A._ifc_destroy
#define ifcs_create U.S._ifcs_create
#define ifcs_destroy U.S._ifcs_destroy
#define ifcs_minifs U.S._ifcs_minifs
+#endif
LIST_ENTRY(if_clone) ifc_list; /* (e) On list of cloners */
};
+
+
static void if_clone_free(struct if_clone *ifc);
static int if_clone_createif(struct if_clone *ifc, char *name, size_t len,
- caddr_t params);
+ struct ifc_data *ifd, struct ifnet **ifpp);
-static int ifc_simple_match(struct if_clone *, const char *);
-static int ifc_simple_create(struct if_clone *, char *, size_t, caddr_t);
-static int ifc_simple_destroy(struct if_clone *, struct ifnet *);
+static int ifc_simple_match(struct if_clone *ifc, const char *name);
+static int ifc_handle_unit(struct if_clone *ifc, char *name, size_t len, int *punit);
+
+#ifdef CLONE_COMPAT_13
+static int ifc_simple_create_wrapper(struct if_clone *ifc, char *name, size_t maxlen,
+ struct ifc_data *ifc_data, struct ifnet **ifpp);
+static int ifc_advanced_create_wrapper(struct if_clone *ifc, char *name, size_t maxlen,
+ struct ifc_data *ifc_data, struct ifnet **ifpp);
+#endif
static struct mtx if_cloners_mtx;
MTX_SYSINIT(if_cloners_lock, &if_cloners_mtx, "if_cloners lock", MTX_DEF);
@@ -175,26 +185,45 @@ vnet_if_clone_init(void)
* Lookup and create a clone network interface.
*/
int
-if_clone_create(char *name, size_t len, caddr_t params)
+ifc_create_ifp(const char *name, struct ifc_data *ifd,
+ struct ifnet **ifpp)
{
struct if_clone *ifc;
+ char ifname[IFNAMSIZ];
+ struct ifnet *ifp = NULL;
+ int error;
/* Try to find an applicable cloner for this request */
IF_CLONERS_LOCK();
- LIST_FOREACH(ifc, &V_if_cloners, ifc_list)
- if (ifc->ifc_type == SIMPLE) {
- if (ifc_simple_match(ifc, name))
- break;
- } else {
- if (ifc->ifc_match(ifc, name))
- break;
- }
+ LIST_FOREACH(ifc, &V_if_cloners, ifc_list) {
+ if (ifc->ifc_match(ifc, name))
+ break;
+ }
IF_CLONERS_UNLOCK();
if (ifc == NULL)
return (EINVAL);
- return (if_clone_createif(ifc, name, len, params));
+ strlcpy(ifname, name, IFNAMSIZ);
+ error = if_clone_createif(ifc, ifname, IFNAMSIZ, ifd, &ifp);
+ if (ifpp != NULL)
+ *ifpp = ifp;
+
+ return (error);
+}
+
+int
+if_clone_create(char *name, size_t len, caddr_t params)
+{
+ struct ifc_data ifd = { .params = params };
+ struct ifnet *ifp;
+
+ int error = ifc_create_ifp(name, &ifd, &ifp);
+
+ if (error == 0)
+ strlcpy(name, if_name(ifp), len);
+
+ return (error);
}
void
@@ -213,26 +242,27 @@ if_clone_addif(struct if_clone *ifc, struct ifnet *ifp)
* Create a clone network interface.
*/
static int
-if_clone_createif(struct if_clone *ifc, char *name, size_t len, caddr_t params)
+if_clone_createif(struct if_clone *ifc, char *name, size_t len,
+ struct ifc_data *ifd, struct ifnet **ifpp)
{
- int err;
- struct ifnet *ifp;
+ int err, unit = 0;
if (ifunit(name) != NULL)
return (EEXIST);
- if (ifc->ifc_type == SIMPLE)
- err = ifc_simple_create(ifc, name, len, params);
- else
- err = (*ifc->ifc_create)(ifc, name, len, params);
-
- if (!err) {
- ifp = ifunit(name);
- if (ifp == NULL)
- panic("%s: lookup failed for %s", __func__, name);
-
- if_clone_addif(ifc, ifp);
+ if (ifc->ifc_flags & IFC_F_AUTOUNIT) {
+ if ((err = ifc_handle_unit(ifc, name, len, &unit)) != 0)
+ return (err);
+ ifd->unit = unit;
}
+ *ifpp = NULL;
+ err = (*ifc->ifc_create)(ifc, name, len, ifd, ifpp);
+
+ if (err == 0) {
+ MPASS(*ifpp != NULL);
+ if_clone_addif(ifc, *ifpp);
+ } else if (ifc->ifc_flags & IFC_F_AUTOUNIT)
+ ifc_free_unit(ifc, unit);
return (err);
}
@@ -274,15 +304,12 @@ if_clone_destroy(const char *name)
/*
* Destroy a clone network interface.
*/
-int
-if_clone_destroyif(struct if_clone *ifc, struct ifnet *ifp)
+static int
+if_clone_destroyif_flags(struct if_clone *ifc, struct ifnet *ifp, uint32_t flags)
{
int err;
struct ifnet *ifcifp;
- if (ifc->ifc_type == ADVANCED && ifc->ifc_destroy == NULL)
- return(EOPNOTSUPP);
-
/*
* Given that the cloned ifnet might be attached to a different
* vnet from where its cloner was registered, we have to
@@ -302,26 +329,31 @@ if_clone_destroyif(struct if_clone *ifc, struct ifnet *ifp)
CURVNET_RESTORE();
return (ENXIO); /* ifp is not on the list. */
}
- if ((ifc->ifc_flags & IFC_NOGROUP) == 0)
+ if ((ifc->ifc_flags & IFC_F_NOGROUP) == 0)
if_delgroup(ifp, ifc->ifc_name);
- if (ifc->ifc_type == SIMPLE)
- err = ifc_simple_destroy(ifc, ifp);
- else
- err = (*ifc->ifc_destroy)(ifc, ifp);
+ int unit = ifp->if_dunit;
+ err = (*ifc->ifc_destroy)(ifc, ifp, flags);
if (err != 0) {
- if ((ifc->ifc_flags & IFC_NOGROUP) == 0)
+ if ((ifc->ifc_flags & IFC_F_NOGROUP) == 0)
if_addgroup(ifp, ifc->ifc_name);
IF_CLONE_LOCK(ifc);
IFC_IFLIST_INSERT(ifc, ifp);
IF_CLONE_UNLOCK(ifc);
- }
+ } else if (ifc->ifc_flags & IFC_F_AUTOUNIT)
+ ifc_free_unit(ifc, unit);
CURVNET_RESTORE();
return (err);
}
+int
+if_clone_destroyif(struct if_clone *ifc, struct ifnet *ifp)
+{
+ return (if_clone_destroyif_flags(ifc, ifp, 0));
+}
+
static struct if_clone *
if_clone_alloc(const char *name, int maxunit)
{
@@ -359,6 +391,56 @@ if_clone_attach(struct if_clone *ifc)
return (0);
}
+struct if_clone *
+ifc_attach_cloner(const char *name, struct if_clone_addreq *req)
+{
+ if (req->create_f == NULL || req->destroy_f == NULL)
+ return (NULL);
+ if (strnlen(name, IFCLOSIZ) >= (IFCLOSIZ - 1))
+ return (NULL);
+
+ struct if_clone *ifc = if_clone_alloc(name, req->maxunit);
+ ifc->ifc_match = req->match_f != NULL ? req->match_f : ifc_simple_match;
+ ifc->ifc_create = req->create_f;
+ ifc->ifc_destroy = req->destroy_f;
+ ifc->ifc_flags = (req->flags & (IFC_F_AUTOUNIT | IFC_F_NOGROUP));
+
+ if (if_clone_attach(ifc) != 0)
+ return (NULL);
+
+ EVENTHANDLER_INVOKE(if_clone_event, ifc);
+
+ return (ifc);
+}
+
+void
+ifc_detach_cloner(struct if_clone *ifc)
+{
+ if_clone_detach(ifc);
+}
+
+
+#ifdef CLONE_COMPAT_13
+
+static int
+ifc_advanced_create_wrapper(struct if_clone *ifc, char *name, size_t maxlen,
+ struct ifc_data *ifc_data, struct ifnet **ifpp)
+{
+ int error = ifc->ifca_create(ifc, name, maxlen, ifc_data->params);
+
+ if (error == 0)
+ *ifpp = ifunit(name);
+ return (error);
+}
+
+static int
+ifc_advanced_destroy_wrapper(struct if_clone *ifc, struct ifnet *ifp, uint32_t flags)
+{
+ if (ifc->ifca_destroy == NULL)
+ return (ENOTSUP);
+ return (ifc->ifca_destroy(ifc, ifp));
+}
+
struct if_clone *
if_clone_advanced(const char *name, u_int maxunit, ifc_match_t match,
ifc_create_t create, ifc_destroy_t destroy)
@@ -366,10 +448,11 @@ if_clone_advanced(const char *name, u_int maxunit, ifc_match_t match,
struct if_clone *ifc;
ifc = if_clone_alloc(name, maxunit);
- ifc->ifc_type = ADVANCED;
ifc->ifc_match = match;
- ifc->ifc_create = create;
- ifc->ifc_destroy = destroy;
+ ifc->ifc_create = ifc_advanced_create_wrapper;
+ ifc->ifc_destroy = ifc_advanced_destroy_wrapper;
+ ifc->ifca_destroy = destroy;
+ ifc->ifca_create = create;
if (if_clone_attach(ifc) != 0)
return (NULL);
@@ -379,6 +462,29 @@ if_clone_advanced(const char *name, u_int maxunit, ifc_match_t match,
return (ifc);
}
+static int
+ifc_simple_create_wrapper(struct if_clone *ifc, char *name, size_t maxlen,
+ struct ifc_data *ifc_data, struct ifnet **ifpp)
+{
+ int unit = 0;
+
+ ifc_name2unit(name, &unit);
+ int error = ifc->ifcs_create(ifc, unit, ifc_data->params);
+ if (error == 0)
+ *ifpp = ifunit(name);
+ return (error);
+}
+
+static int
+ifc_simple_destroy_wrapper(struct if_clone *ifc, struct ifnet *ifp, uint32_t flags)
+{
+ if (ifp->if_dunit < ifc->ifcs_minifs && (flags & IFC_F_FORCE) == 0)
+ return (EINVAL);
+
+ ifc->ifcs_destroy(ifp);
+ return (0);
+}
+
struct if_clone *
if_clone_simple(const char *name, ifcs_create_t create, ifcs_destroy_t destroy,
u_int minifs)
@@ -387,10 +493,13 @@ if_clone_simple(const char *name, ifcs_create_t create, ifcs_destroy_t destroy,
u_int unit;
ifc = if_clone_alloc(name, 0);
- ifc->ifc_type = SIMPLE;
+ ifc->ifc_match = ifc_simple_match;
+ ifc->ifc_create = ifc_simple_create_wrapper;
+ ifc->ifc_destroy = ifc_simple_destroy_wrapper;
ifc->ifcs_create = create;
ifc->ifcs_destroy = destroy;
ifc->ifcs_minifs = minifs;
+ ifc->ifc_flags = IFC_F_AUTOUNIT;
if (if_clone_attach(ifc) != 0)
return (NULL);
@@ -398,9 +507,11 @@ if_clone_simple(const char *name, ifcs_create_t create, ifcs_destroy_t destroy,
for (unit = 0; unit < minifs; unit++) {
char name[IFNAMSIZ];
int error __unused;
+ struct ifc_data ifd = {};
+ struct ifnet *ifp;
snprintf(name, IFNAMSIZ, "%s%d", ifc->ifc_name, unit);
- error = if_clone_createif(ifc, name, IFNAMSIZ, NULL);
+ error = if_clone_createif(ifc, name, IFNAMSIZ, &ifd, &ifp);
KASSERT(error == 0,
("%s: failed to create required interface %s",
__func__, name));
@@ -410,6 +521,7 @@ if_clone_simple(const char *name, ifcs_create_t create, ifcs_destroy_t destroy,
return (ifc);
}
+#endif
/*
* Unregister a network interface cloner.
@@ -423,13 +535,9 @@ if_clone_detach(struct if_clone *ifc)
V_if_cloners_count--;
IF_CLONERS_UNLOCK();
- /* Allow all simples to be destroyed */
- if (ifc->ifc_type == SIMPLE)
- ifc->ifcs_minifs = 0;
-
/* destroy all interfaces for this cloner */
while (!LIST_EMPTY(&ifc->ifc_iflist))
- if_clone_destroyif(ifc, LIST_FIRST(&ifc->ifc_iflist));
+ if_clone_destroyif_flags(ifc, LIST_FIRST(&ifc->ifc_iflist), IFC_F_FORCE);
IF_CLONE_REMREF(ifc);
}
@@ -660,7 +768,7 @@ ifc_simple_match(struct if_clone *ifc, const char *name)
}
static int
-ifc_simple_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
+ifc_handle_unit(struct if_clone *ifc, char *name, size_t len, int *punit)
{
char *dp;
int wildcard;
@@ -677,12 +785,6 @@ ifc_simple_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
if (err != 0)
return (err);
- err = ifc->ifcs_create(ifc, unit, params);
- if (err != 0) {
- ifc_free_unit(ifc, unit);
- return (err);
- }
-
/* In the wildcard case, we need to update the name. */
if (wildcard) {
for (dp = name; *dp != '\0'; dp++);
@@ -696,25 +798,22 @@ ifc_simple_create(struct if_clone *ifc, char *name, size_t len, caddr_t params)
panic("if_clone_create(): interface name too long");
}
}
+ *punit = unit;
return (0);
}
-static int
-ifc_simple_destroy(struct if_clone *ifc, struct ifnet *ifp)
+int
+ifc_copyin(const struct ifc_data *ifd, void *target, size_t len)
{
- int unit;
-
- unit = ifp->if_dunit;
-
- if (unit < ifc->ifcs_minifs)
+ if (ifd->params == NULL)
return (EINVAL);
- ifc->ifcs_destroy(ifp);
-
- ifc_free_unit(ifc, unit);
-
- return (0);
+ if (ifd->flags & IFC_F_SYSSPACE) {
+ memcpy(target, ifd->params, len);
+ return (0);
+ } else
+ return (copyin(ifd->params, target, len));
}
const char *
diff --git a/sys/net/if_clone.h b/sys/net/if_clone.h
index 8f1c97705943..de20eef993f4 100644
--- a/sys/net/if_clone.h
+++ b/sys/net/if_clone.h
@@ -39,10 +39,48 @@
#include <sys/_eventhandler.h>
-#define IFC_NOGROUP 0x1
+#define CLONE_COMPAT_13
struct if_clone;
+/* Public KPI */
+struct ifc_data {
+ uint32_t flags;
+ uint32_t unit; /* Selected unit when IFC_C_AUTOUNIT set */
+ void *params;
+ struct vnet *vnet;
+};
+
+typedef int ifc_match_f(struct if_clone *ifc, const char *name);
+typedef int ifc_create_f(struct if_clone *ifc, char *name, size_t maxlen,
+ struct ifc_data *ifd, struct ifnet **ifpp);
+typedef int ifc_destroy_f(struct if_clone *ifc, struct ifnet *ifp, uint32_t flags);
+
+struct if_clone_addreq {
+ uint16_t version; /* Always 0 for now */
+ uint16_t spare;
+ uint32_t flags;
+ uint32_t maxunit; /* Maximum allowed unit number */
+ ifc_match_f *match_f;
+ ifc_create_f *create_f;
+ ifc_destroy_f *destroy_f;
+};
+
+#define IFC_F_NOGROUP 0x01 /* Creation flag: don't add unit group */
+#define IFC_F_AUTOUNIT 0x02 /* Creation flag: automatically select unit */
+#define IFC_F_SYSSPACE 0x04 /* Cloner callback: params pointer is in kernel memory */
+#define IFC_F_FORCE 0x08 /* Deletion flag: force interface deletion */
+
+#define IFC_NOGROUP IFC_F_NOGROUP
+
+struct if_clone *ifc_attach_cloner(const char *name, struct if_clone_addreq *req);
+void ifc_detach_cloner(struct if_clone *ifc);
+int ifc_create_ifp(const char *name, struct ifc_data *ifd,
+ struct ifnet **ifpp);
+
+int ifc_copyin(const struct ifc_data *ifd, void *target, size_t len);
+#ifdef CLONE_COMPAT_13
+
/* Methods. */
typedef int ifc_match_t(struct if_clone *, const char *);
typedef int ifc_create_t(struct if_clone *, char *, size_t, caddr_t);
@@ -58,6 +96,7 @@ struct if_clone *
struct if_clone *
if_clone_simple(const char *, ifcs_create_t, ifcs_destroy_t, u_int);
void if_clone_detach(struct if_clone *);
+#endif
/* Unit (de)allocating functions. */
int ifc_name2unit(const char *name, int *unit);