* [PATCH] xtables-addons 3.18 condition - Improved network namespace support
@ 2021-08-20 18:24 Grzegorz Kuczyński
2021-08-22 11:19 ` Jan Engelhardt
0 siblings, 1 reply; 5+ messages in thread
From: Grzegorz Kuczyński @ 2021-08-20 18:24 UTC (permalink / raw)
To: netfilter-devel
Hello
A few years ago I add network namespace to extension condition.
I review this changes again and make changes again.
This is better version.
diff --git a/extensions/xt_condition.c b/extensions/xt_condition.c
index 8227c5d..c5b0df3 100644
--- a/extensions/xt_condition.c
+++ b/extensions/xt_condition.c
@@ -65,12 +65,11 @@ static DEFINE_MUTEX(proc_lock);
struct condition_net {
struct list_head conditions_list;
struct proc_dir_entry *proc_net_condition;
- bool after_clear;
};
static int condition_net_id;
-static inline struct condition_net *condition_pernet(struct net *net)
+static inline struct condition_net *get_condition_pernet(struct net *net)
{
return net_generic(net, condition_net_id);
}
@@ -132,7 +131,7 @@ static int condition_mt_check(const struct
xt_mtchk_param *par)
{
struct xt_condition_mtinfo *info = par->matchinfo;
struct condition_variable *var;
- struct condition_net *condition_net = condition_pernet(par->net);
+ struct condition_net *condition_net =
get_condition_pernet(par->net);
/* Forbid certain names */
if (*info->name == '\0' || *info->name == '.' ||
@@ -190,13 +189,10 @@ static void condition_mt_destroy(const struct
xt_mtdtor_param *par)
{
const struct xt_condition_mtinfo *info = par->matchinfo;
struct condition_variable *var = info->condvar;
- struct condition_net *cnet = condition_pernet(par->net);
-
- if (cnet->after_clear)
- return;
-
+ struct condition_net *cnet = get_condition_pernet(par->net);
+
mutex_lock(&proc_lock);
- if (--var->refcount == 0) {
+ if (--var->refcount == 0 &&
!list_empty_careful(&cnet->conditions_list)) {
list_del(&var->list);
remove_proc_entry(var->name, cnet->proc_net_condition);
mutex_unlock(&proc_lock);
@@ -233,18 +229,17 @@ static const char *const dir_name = "nf_condition";
static int __net_init condition_net_init(struct net *net)
{
- struct condition_net *condition_net = condition_pernet(net);
+ struct condition_net *condition_net = get_condition_pernet(net);
INIT_LIST_HEAD(&condition_net->conditions_list);
condition_net->proc_net_condition = proc_mkdir(dir_name,
net->proc_net);
if (condition_net->proc_net_condition == NULL)
return -EACCES;
- condition_net->after_clear = 0;
return 0;
}
static void __net_exit condition_net_exit(struct net *net)
{
- struct condition_net *condition_net = condition_pernet(net);
+ struct condition_net *condition_net = get_condition_pernet(net);
struct list_head *pos, *q;
struct condition_variable *var = NULL;
@@ -256,7 +251,6 @@ static void __net_exit condition_net_exit(struct net
*net)
kfree(var);
}
mutex_unlock(&proc_lock);
- condition_net->after_clear = true;
}
static struct pernet_operations condition_net_ops = {
--
Grzegorz Kuczyński
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xtables-addons 3.18 condition - Improved network namespace support
2021-08-20 18:24 [PATCH] xtables-addons 3.18 condition - Improved network namespace support Grzegorz Kuczyński
@ 2021-08-22 11:19 ` Jan Engelhardt
2021-08-22 11:50 ` Jeremy Sowden
0 siblings, 1 reply; 5+ messages in thread
From: Jan Engelhardt @ 2021-08-22 11:19 UTC (permalink / raw)
To: Grzegorz Kuczyński; +Cc: netfilter-devel
On Friday 2021-08-20 20:24, Grzegorz Kuczyński wrote:
>Hello
>A few years ago I add network namespace to extension condition.
>I review this changes again and make changes again.
>This is better version.
It does not apply. Your mail software mangled the patch.
I would also wish for a more precise description what causes
these lines to need removal.
>- if (cnet->after_clear)
>- return;
>- if (--var->refcount == 0) {
>+ if (--var->refcount == 0 &&
>!list_empty_careful(&cnet->conditions_list)) {
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xtables-addons 3.18 condition - Improved network namespace support
2021-08-22 11:19 ` Jan Engelhardt
@ 2021-08-22 11:50 ` Jeremy Sowden
0 siblings, 0 replies; 5+ messages in thread
From: Jeremy Sowden @ 2021-08-22 11:50 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: Grzegorz Kuczyński, netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 3431 bytes --]
On 2021-08-22, at 13:19:46 +0200, Jan Engelhardt wrote:
> On Friday 2021-08-20 20:24, Grzegorz Kuczyński wrote:
> > A few years ago I add network namespace to extension condition.
> > I review this changes again and make changes again.
> > This is better version.
>
> It does not apply. Your mail software mangled the patch.
>
> I would also wish for a more precise description what causes
> these lines to need removal.
>
> >- if (cnet->after_clear)
> >- return;
>
> >- if (--var->refcount == 0) {
> >+ if (--var->refcount == 0 && !list_empty_careful(&cnet->conditions_list)) {
I've been looking at this, and I don't think it works.
The idea is to do away with a separate flag whose purpose is to indicate
that condition_mt_destroy is being called during the tear-down of a
name-space.
Currently we have:
static void __net_exit condition_net_exit(struct net *net)
{
struct condition_net *condition_net = condition_pernet(net);
struct list_head *pos, *q;
struct condition_variable *var = NULL;
remove_proc_subtree(dir_name, net->proc_net);
mutex_lock(&proc_lock);
list_for_each_safe(pos, q, &condition_net->conditions_list) {
var = list_entry(pos, struct condition_variable, list);
list_del(pos);
kfree(var);
}
mutex_unlock(&proc_lock);
condition_net->after_clear = true;
}
and:
static void condition_mt_destroy(const struct xt_mtdtor_param *par)
{
const struct xt_condition_mtinfo *info = par->matchinfo;
struct condition_variable *var = info->condvar;
struct condition_net *cnet = condition_pernet(par->net);
if (cnet->after_clear)
return;
mutex_lock(&proc_lock);
if (--var->refcount == 0) {
list_del(&var->list);
remove_proc_entry(var->name, cnet->proc_net_condition);
mutex_unlock(&proc_lock);
kfree(var);
return;
}
mutex_unlock(&proc_lock);
}
Since pernet_operations destructors are called in the reverse of the
order in which they are registered, the destructor for xt_condition will
be called before the destructor for the table which deletes all the
rules. The `after_clear` flag serves to indicate during the call of
condition_mt_destroy that condition_net_exit has already been called and
freed all the variables. Replacing it with a check whether list of
variables is empty could work, but not after we've checked the ref-count
of a variable that we have already freed. Based on what I've seen in
other modules, I'd be more inclined to do something like this:
static void __net_exit condition_net_exit(struct net *net)
{
struct condition_net *condition_net = condition_pernet(net);
remove_proc_subtree(dir_name, net->proc_net);
condition_net->proc_net_condition = NULL;
}
and:
static void condition_mt_destroy(const struct xt_mtdtor_param *par)
{
const struct xt_condition_mtinfo *info = par->matchinfo;
struct condition_variable *var = info->condvar;
struct condition_net *cnet = condition_pernet(par->net);
mutex_lock(&proc_lock);
if (--var->refcount == 0) {
list_del(&var->list);
if (condition_net->proc_net_condition)
remove_proc_entry(var->name, cnet->proc_net_condition);
kfree(var);
}
mutex_unlock(&proc_lock);
}
J.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xtables-addons 3.18 condition - Improved network namespace support
2021-08-22 16:14 Grzegorz Kuczyński
@ 2021-08-22 16:34 ` Jeremy Sowden
0 siblings, 0 replies; 5+ messages in thread
From: Jeremy Sowden @ 2021-08-22 16:34 UTC (permalink / raw)
To: Grzegorz Kuczyński; +Cc: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 1845 bytes --]
On 2021-08-22, at 18:14:13 +0200, Grzegorz Kuczyński wrote:
> My intention was removed special var 'after_clear'. The main idea was
> use a list as a flag.
>
> But Jeremy has right, I used list_empty_careful() in wrong place.
> I can change this for that:
>
> static void condition_mt_destroy(const struct xt_mtdtor_param *par)
> {
> const struct xt_condition_mtinfo *info = par->matchinfo;
> struct condition_variable *var = info->condvar;
> struct condition_net *cnet = get_condition_pernet(par->net);
>
> mutex_lock(&proc_lock);
> // is called after net namespace deleted?
> if (list_empty_careful(&cnet->conditions_list)) {
> mutex_unlock(&proc_lock);
> return;
> }
> if (--var->refcount == 0) {
> list_del(&var->list);
> remove_proc_entry(var->name, cnet->proc_net_condition);
> mutex_unlock(&proc_lock);
> kfree(var);
> return;
> }
> mutex_unlock(&proc_lock);
> }
>
> But now I realized that one var bool is less expensive than call a
> function. So my improved is turned out the fail :) ... sorry for
> taking the time.
I think there are some improvements that can be made. Will send
patches.
J.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xtables-addons 3.18 condition - Improved network namespace support
@ 2021-08-22 16:14 Grzegorz Kuczyński
2021-08-22 16:34 ` Jeremy Sowden
0 siblings, 1 reply; 5+ messages in thread
From: Grzegorz Kuczyński @ 2021-08-22 16:14 UTC (permalink / raw)
To: netfilter-devel
My intention was removed special var 'after_clear'. The main idea was use a list as a flag.
But Jeremy has right, I used list_empty_careful() in wrong place.
I can change this for that:
static void condition_mt_destroy(const struct xt_mtdtor_param *par)
{
const struct xt_condition_mtinfo *info = par->matchinfo;
struct condition_variable *var = info->condvar;
struct condition_net *cnet = get_condition_pernet(par->net);
mutex_lock(&proc_lock);
// is called after net namespace deleted?
if (list_empty_careful(&cnet->conditions_list)) {
mutex_unlock(&proc_lock);
return;
}
if (--var->refcount == 0) {
list_del(&var->list);
remove_proc_entry(var->name, cnet->proc_net_condition);
mutex_unlock(&proc_lock);
kfree(var);
return;
}
mutex_unlock(&proc_lock);
}
But now I realized that one var bool is less expensive than call a function.
So my improved is turned out the fail :) ... sorry for taking the time.
--
Grzegorz Kuczyński
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-22 16:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 18:24 [PATCH] xtables-addons 3.18 condition - Improved network namespace support Grzegorz Kuczyński
2021-08-22 11:19 ` Jan Engelhardt
2021-08-22 11:50 ` Jeremy Sowden
2021-08-22 16:14 Grzegorz Kuczyński
2021-08-22 16:34 ` Jeremy Sowden
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.