Re: Document ifc_list immutability
ok
> On 8 Nov 2022, at 21:38, Klemens Nanni wrote:
>
> On Tue, Nov 08, 2022 at 09:34:36PM +0300, Vitaliy Makkoveev wrote:
>>> On 8 Nov 2022, at 21:26, Klemens Nanni wrote:
>>>
>>> On Tue, Nov 08, 2022 at 09:18:47PM +0300, Vitaliy Makkoveev wrote:
> On 8 Nov 2022, at 21:08, Klemens Nanni wrote:
>
> Now properly. How about a single comment block at the top instead of
> repeating it for every struct?
>
>
You forgot to mark [I] `if_cloners’ within net/if.c.
>>>
>>> Like this? First locking commit in if.c.
>>>
>>>
>>
>> Please, do this in consistency with other places:
>>
>> LIST_HEAD(, ipsec_ids) ipsp_ids_gc_list =
>>LIST_HEAD_INITIALIZER(ipsp_ids_gc_list);/* [F] */
>
> OK?
>
> diff --git a/sys/net/if.c b/sys/net/if.c
> index 58a972b802c..a7b0b9bb8a3 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -228,8 +228,9 @@ void if_idxmap_remove(struct ifnet *);
>
> TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
>
> -LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
> -int if_cloners_count;
> +LIST_HEAD(, if_clone) if_cloners =
> + LIST_HEAD_INITIALIZER(if_cloners); /* [I] list of clonable interfaces */
> +int if_cloners_count;/* [I] number of clonable interfaces */
>
> struct rwlock if_cloners_lock = RWLOCK_INITIALIZER("clonelk");
>
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 28514a0bfcd..6272be882f4 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -73,6 +73,18 @@
> * interfaces. These routines live in the files if.c and route.c
> */
>
> +/*
> + * Locks used to protect struct members in this file:
> + * I immutable after creation
> + * d protection left do the driver
> + * c only used in ioctl or routing socket contexts (kernel lock)
> + * K kernel lock
> + * N net lock
> + *
> + * For SRP related structures that allow lock-free reads, the write lock
> + * is indicated below.
> + */
> +
> struct rtentry;
> struct ifnet;
> struct task;
> @@ -82,7 +94,7 @@ struct cpumem;
> * Structure describing a `cloning' interface.
> */
> struct if_clone {
> - LIST_ENTRY(if_clone) ifc_list; /* on list of cloners */
> + LIST_ENTRY(if_clone) ifc_list; /* [I] on list of cloners */
> const char *ifc_name; /* name of device, e.g. `gif' */
> size_t ifc_namelen; /* length of name */
>
> @@ -99,17 +111,6 @@ struct if_clone {
> .ifc_destroy= destroy,
> \
> }
>
> -/*
> - * Locks used to protect struct members in this file:
> - * I immutable after creation
> - * d protection left do the driver
> - * c only used in ioctl or routing socket contexts (kernel lock)
> - * K kernel lock
> - * N net lock
> - *
> - * For SRP related structures that allow lock-free reads, the write lock
> - * is indicated below.
> - */
> /*
> * Structure defining a queue for a network interface.
> *
Re: Document ifc_list immutability
On Tue, Nov 08, 2022 at 09:34:36PM +0300, Vitaliy Makkoveev wrote:
> > On 8 Nov 2022, at 21:26, Klemens Nanni wrote:
> >
> > On Tue, Nov 08, 2022 at 09:18:47PM +0300, Vitaliy Makkoveev wrote:
> >>> On 8 Nov 2022, at 21:08, Klemens Nanni wrote:
> >>>
> >>> Now properly. How about a single comment block at the top instead of
> >>> repeating it for every struct?
> >>>
> >>>
> >>
> >> You forgot to mark [I] `if_cloners’ within net/if.c.
> >
> > Like this? First locking commit in if.c.
> >
> >
>
> Please, do this in consistency with other places:
>
> LIST_HEAD(, ipsec_ids) ipsp_ids_gc_list =
> LIST_HEAD_INITIALIZER(ipsp_ids_gc_list);/* [F] */
OK?
diff --git a/sys/net/if.c b/sys/net/if.c
index 58a972b802c..a7b0b9bb8a3 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -228,8 +228,9 @@ voidif_idxmap_remove(struct ifnet *);
TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
-LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
-int if_cloners_count;
+LIST_HEAD(, if_clone) if_cloners =
+ LIST_HEAD_INITIALIZER(if_cloners); /* [I] list of clonable interfaces */
+int if_cloners_count; /* [I] number of clonable interfaces */
struct rwlock if_cloners_lock = RWLOCK_INITIALIZER("clonelk");
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 28514a0bfcd..6272be882f4 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -73,6 +73,18 @@
* interfaces. These routines live in the files if.c and route.c
*/
+/*
+ * Locks used to protect struct members in this file:
+ * I immutable after creation
+ * d protection left do the driver
+ * c only used in ioctl or routing socket contexts (kernel lock)
+ * K kernel lock
+ * N net lock
+ *
+ * For SRP related structures that allow lock-free reads, the write lock
+ * is indicated below.
+ */
+
struct rtentry;
struct ifnet;
struct task;
@@ -82,7 +94,7 @@ struct cpumem;
* Structure describing a `cloning' interface.
*/
struct if_clone {
- LIST_ENTRY(if_clone) ifc_list; /* on list of cloners */
+ LIST_ENTRY(if_clone) ifc_list; /* [I] on list of cloners */
const char *ifc_name; /* name of device, e.g. `gif' */
size_t ifc_namelen; /* length of name */
@@ -99,17 +111,6 @@ struct if_clone {
.ifc_destroy = destroy, \
}
-/*
- * Locks used to protect struct members in this file:
- * I immutable after creation
- * d protection left do the driver
- * c only used in ioctl or routing socket contexts (kernel lock)
- * K kernel lock
- * N net lock
- *
- * For SRP related structures that allow lock-free reads, the write lock
- * is indicated below.
- */
/*
* Structure defining a queue for a network interface.
*
Re: Document ifc_list immutability
> On 8 Nov 2022, at 21:26, Klemens Nanni wrote:
>
> On Tue, Nov 08, 2022 at 09:18:47PM +0300, Vitaliy Makkoveev wrote:
>>> On 8 Nov 2022, at 21:08, Klemens Nanni wrote:
>>>
>>> Now properly. How about a single comment block at the top instead of
>>> repeating it for every struct?
>>>
>>>
>>
>> You forgot to mark [I] `if_cloners’ within net/if.c.
>
> Like this? First locking commit in if.c.
>
>
Please, do this in consistency with other places:
LIST_HEAD(, ipsec_ids) ipsp_ids_gc_list =
LIST_HEAD_INITIALIZER(ipsp_ids_gc_list);/* [F] */
> diff --git a/sys/net/if.c b/sys/net/if.c
> index 58a972b802c..b7fc5258bd4 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -228,6 +228,7 @@ void if_idxmap_remove(struct ifnet *);
>
> TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
>
> +/* [I] list of clonable interfaces */
> LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
> int if_cloners_count;
>
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 28514a0bfcd..6272be882f4 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -73,6 +73,18 @@
> * interfaces. These routines live in the files if.c and route.c
> */
>
> +/*
> + * Locks used to protect struct members in this file:
> + * I immutable after creation
> + * d protection left do the driver
> + * c only used in ioctl or routing socket contexts (kernel lock)
> + * K kernel lock
> + * N net lock
> + *
> + * For SRP related structures that allow lock-free reads, the write lock
> + * is indicated below.
> + */
> +
> struct rtentry;
> struct ifnet;
> struct task;
> @@ -82,7 +94,7 @@ struct cpumem;
> * Structure describing a `cloning' interface.
> */
> struct if_clone {
> - LIST_ENTRY(if_clone) ifc_list; /* on list of cloners */
> + LIST_ENTRY(if_clone) ifc_list; /* [I] on list of cloners */
> const char *ifc_name; /* name of device, e.g. `gif' */
> size_t ifc_namelen; /* length of name */
>
> @@ -99,17 +111,6 @@ struct if_clone {
> .ifc_destroy= destroy,
> \
> }
>
> -/*
> - * Locks used to protect struct members in this file:
> - * I immutable after creation
> - * d protection left do the driver
> - * c only used in ioctl or routing socket contexts (kernel lock)
> - * K kernel lock
> - * N net lock
> - *
> - * For SRP related structures that allow lock-free reads, the write lock
> - * is indicated below.
> - */
> /*
> * Structure defining a queue for a network interface.
> *
>
Re: Document ifc_list immutability
On Tue, Nov 08, 2022 at 09:18:47PM +0300, Vitaliy Makkoveev wrote:
> > On 8 Nov 2022, at 21:08, Klemens Nanni wrote:
> >
> > Now properly. How about a single comment block at the top instead of
> > repeating it for every struct?
> >
> >
>
> You forgot to mark [I] `if_cloners’ within net/if.c.
Like this? First locking commit in if.c.
diff --git a/sys/net/if.c b/sys/net/if.c
index 58a972b802c..b7fc5258bd4 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -228,6 +228,7 @@ voidif_idxmap_remove(struct ifnet *);
TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
+/* [I] list of clonable interfaces */
LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
int if_cloners_count;
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 28514a0bfcd..6272be882f4 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -73,6 +73,18 @@
* interfaces. These routines live in the files if.c and route.c
*/
+/*
+ * Locks used to protect struct members in this file:
+ * I immutable after creation
+ * d protection left do the driver
+ * c only used in ioctl or routing socket contexts (kernel lock)
+ * K kernel lock
+ * N net lock
+ *
+ * For SRP related structures that allow lock-free reads, the write lock
+ * is indicated below.
+ */
+
struct rtentry;
struct ifnet;
struct task;
@@ -82,7 +94,7 @@ struct cpumem;
* Structure describing a `cloning' interface.
*/
struct if_clone {
- LIST_ENTRY(if_clone) ifc_list; /* on list of cloners */
+ LIST_ENTRY(if_clone) ifc_list; /* [I] on list of cloners */
const char *ifc_name; /* name of device, e.g. `gif' */
size_t ifc_namelen; /* length of name */
@@ -99,17 +111,6 @@ struct if_clone {
.ifc_destroy = destroy, \
}
-/*
- * Locks used to protect struct members in this file:
- * I immutable after creation
- * d protection left do the driver
- * c only used in ioctl or routing socket contexts (kernel lock)
- * K kernel lock
- * N net lock
- *
- * For SRP related structures that allow lock-free reads, the write lock
- * is indicated below.
- */
/*
* Structure defining a queue for a network interface.
*
Re: Document ifc_list immutability
> On 8 Nov 2022, at 21:08, Klemens Nanni wrote:
>
> Now properly. How about a single comment block at the top instead of
> repeating it for every struct?
>
>
You forgot to mark [I] `if_cloners’ within net/if.c.
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 28514a0bfcd..6272be882f4 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -73,6 +73,18 @@
> * interfaces. These routines live in the files if.c and route.c
> */
>
> +/*
> + * Locks used to protect struct members in this file:
> + * I immutable after creation
> + * d protection left do the driver
> + * c only used in ioctl or routing socket contexts (kernel lock)
> + * K kernel lock
> + * N net lock
> + *
> + * For SRP related structures that allow lock-free reads, the write lock
> + * is indicated below.
> + */
> +
> struct rtentry;
> struct ifnet;
> struct task;
> @@ -82,7 +94,7 @@ struct cpumem;
> * Structure describing a `cloning' interface.
> */
> struct if_clone {
> - LIST_ENTRY(if_clone) ifc_list; /* on list of cloners */
> + LIST_ENTRY(if_clone) ifc_list; /* [I] on list of cloners */
> const char *ifc_name; /* name of device, e.g. `gif' */
> size_t ifc_namelen; /* length of name */
>
> @@ -99,17 +111,6 @@ struct if_clone {
> .ifc_destroy= destroy,
> \
> }
>
> -/*
> - * Locks used to protect struct members in this file:
> - * I immutable after creation
> - * d protection left do the driver
> - * c only used in ioctl or routing socket contexts (kernel lock)
> - * K kernel lock
> - * N net lock
> - *
> - * For SRP related structures that allow lock-free reads, the write lock
> - * is indicated below.
> - */
> /*
> * Structure defining a queue for a network interface.
> *
>
Document ifc_list immutability
Now properly. How about a single comment block at the top instead of
repeating it for every struct?
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 28514a0bfcd..6272be882f4 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -73,6 +73,18 @@
* interfaces. These routines live in the files if.c and route.c
*/
+/*
+ * Locks used to protect struct members in this file:
+ * I immutable after creation
+ * d protection left do the driver
+ * c only used in ioctl or routing socket contexts (kernel lock)
+ * K kernel lock
+ * N net lock
+ *
+ * For SRP related structures that allow lock-free reads, the write lock
+ * is indicated below.
+ */
+
struct rtentry;
struct ifnet;
struct task;
@@ -82,7 +94,7 @@ struct cpumem;
* Structure describing a `cloning' interface.
*/
struct if_clone {
- LIST_ENTRY(if_clone) ifc_list; /* on list of cloners */
+ LIST_ENTRY(if_clone) ifc_list; /* [I] on list of cloners */
const char *ifc_name; /* name of device, e.g. `gif' */
size_t ifc_namelen; /* length of name */
@@ -99,17 +111,6 @@ struct if_clone {
.ifc_destroy = destroy, \
}
-/*
- * Locks used to protect struct members in this file:
- * I immutable after creation
- * d protection left do the driver
- * c only used in ioctl or routing socket contexts (kernel lock)
- * K kernel lock
- * N net lock
- *
- * For SRP related structures that allow lock-free reads, the write lock
- * is indicated below.
- */
/*
* Structure defining a queue for a network interface.
*
