* [RFC 0/2] netfilter: xtables: CONDITION target implementation
@ 2010-07-19 14:15 Luciano Coelho
2010-07-19 14:15 ` [RFC 1/2] netfilter: xt_condition: export list management code Luciano Coelho
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Luciano Coelho @ 2010-07-19 14:15 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, kaber, jengelh, sameo
From: Luciano Coelho <coelho@testbed.(none)>
Hi all,
As discussed earlier, I've been looking for a way to enable and disable the
condition match automatically, in the netfilter tables themselves (ie. without
the need to use procfs).
This is my initial implementation. Please let me know how it looks. The first
patch is based on the xt_condition patch that Jan sent to the list (but which
has not been finalized for inclusion yet). Once the condition match gets
applied, I'll forward port my patch and submit it again.
Cheers,
Luca.
Luciano Coelho (2):
netfilter: xt_condition: export list management code
netfilter: xtables: implement CONDITION target
include/linux/netfilter/Kbuild | 1 +
include/linux/netfilter/xt_CONDITION.h | 39 +++++++++++
include/linux/netfilter/xt_condition.h | 17 +++++-
net/netfilter/Kconfig | 12 ++++
net/netfilter/Makefile | 1 +
net/netfilter/xt_CONDITION.c | 112 ++++++++++++++++++++++++++++++++
net/netfilter/xt_condition.c | 82 ++++++++++++++----------
7 files changed, 229 insertions(+), 35 deletions(-)
create mode 100644 include/linux/netfilter/xt_CONDITION.h
create mode 100644 net/netfilter/xt_CONDITION.c
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 1/2] netfilter: xt_condition: export list management code
2010-07-19 14:15 [RFC 0/2] netfilter: xtables: CONDITION target implementation Luciano Coelho
@ 2010-07-19 14:15 ` Luciano Coelho
2010-07-19 16:13 ` Jan Engelhardt
2010-07-19 14:15 ` [RFC 2/2] netfilter: xtables: implement CONDITION target Luciano Coelho
2010-07-19 14:27 ` [RFC 0/2] netfilter: xtables: CONDITION target implementation Changli Gao
2 siblings, 1 reply; 8+ messages in thread
From: Luciano Coelho @ 2010-07-19 14:15 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, kaber, jengelh, sameo
From: Luciano Coelho <coelho@testbed>
This patch isolates and exports the condition list management code, in
preparation for the CONDITION target to use it. No functional change,
just reorganization of the code.
Signed-off-by: Luciano Coelho <luciano.coelho@nokia.com>
---
include/linux/netfilter/xt_condition.h | 17 ++++++-
net/netfilter/xt_condition.c | 82 ++++++++++++++++++-------------
2 files changed, 64 insertions(+), 35 deletions(-)
diff --git a/include/linux/netfilter/xt_condition.h b/include/linux/netfilter/xt_condition.h
index 4faf3ca..eebf41a 100644
--- a/include/linux/netfilter/xt_condition.h
+++ b/include/linux/netfilter/xt_condition.h
@@ -3,12 +3,27 @@
#include <linux/types.h>
+#define XT_CONDITION_MAX_NAME_SIZE 30
+
struct xt_condition_mtinfo {
- char name[31];
+ char name[XT_CONDITION_MAX_NAME_SIZE + 1];
__u8 invert;
/* Used internally by the kernel */
void *condvar __attribute__((aligned(8)));
};
+#ifdef __KERNEL__
+struct condition_variable {
+ struct list_head list;
+ struct proc_dir_entry *status_proc;
+ unsigned int refcount;
+ bool enabled;
+};
+
+struct condition_variable *xt_condition_insert(const char *name);
+void xt_condition_put(struct condition_variable *var);
+void xt_condition_set(struct condition_variable *var, bool enabled);
+#endif /* __KERNEL__ */
+
#endif /* _XT_CONDITION_H */
diff --git a/net/netfilter/xt_condition.c b/net/netfilter/xt_condition.c
index a7ccea3..dec97fe 100644
--- a/net/netfilter/xt_condition.c
+++ b/net/netfilter/xt_condition.c
@@ -43,13 +43,6 @@ MODULE_PARM_DESC(condition_gid_perms, "default group owner of /proc/net/nf_condi
MODULE_ALIAS("ipt_condition");
MODULE_ALIAS("ip6t_condition");
-struct condition_variable {
- struct list_head list;
- struct proc_dir_entry *status_proc;
- unsigned int refcount;
- bool enabled;
-};
-
/* proc_lock is a user context only semaphore used for write access */
/* to the conditions' list. */
static DEFINE_MUTEX(proc_lock);
@@ -100,47 +93,34 @@ condition_mt(const struct sk_buff *skb, struct xt_action_param *par)
return var->enabled ^ info->invert;
}
-static int condition_mt_check(const struct xt_mtchk_param *par)
+struct condition_variable *xt_condition_insert(const char *name)
{
- struct xt_condition_mtinfo *info = par->matchinfo;
struct condition_variable *var;
- /* Forbid certain names */
- if (*info->name == '\0' || *info->name == '.' ||
- info->name[sizeof(info->name)-1] != '\0' ||
- memchr(info->name, '/', sizeof(info->name)) != NULL) {
- pr_info("name not allowed or too long: \"%.*s\"\n",
- (unsigned int)sizeof(info->name), info->name);
- return -EINVAL;
- }
/*
* Let's acquire the lock, check for the condition and add it
* or increase the reference counter.
*/
mutex_lock(&proc_lock);
list_for_each_entry(var, &conditions_list, list) {
- if (strcmp(info->name, var->status_proc->name) == 0) {
+ if (strcmp(name, var->status_proc->name) == 0) {
++var->refcount;
- mutex_unlock(&proc_lock);
- info->condvar = var;
- return 0;
+ goto out;
}
}
/* At this point, we need to allocate a new condition variable. */
var = kmalloc(sizeof(struct condition_variable), GFP_KERNEL);
- if (var == NULL) {
- mutex_unlock(&proc_lock);
- return -ENOMEM;
- }
+ if (var == NULL)
+ goto out;
/* Create the condition variable's proc file entry. */
- var->status_proc = create_proc_entry(info->name, condition_list_perms,
+ var->status_proc = create_proc_entry(name, condition_list_perms,
proc_net_condition);
if (var->status_proc == NULL) {
kfree(var);
- mutex_unlock(&proc_lock);
- return -ENOMEM;
+ var = NULL;
+ goto out;
}
var->refcount = 1;
@@ -151,16 +131,14 @@ static int condition_mt_check(const struct xt_mtchk_param *par)
var->status_proc->uid = condition_uid_perms;
var->status_proc->gid = condition_gid_perms;
list_add(&var->list, &conditions_list);
+out:
mutex_unlock(&proc_lock);
- info->condvar = var;
- return 0;
+ return var;
}
+EXPORT_SYMBOL_GPL(xt_condition_insert);
-static void condition_mt_destroy(const struct xt_mtdtor_param *par)
+void xt_condition_put(struct condition_variable *var)
{
- const struct xt_condition_mtinfo *info = par->matchinfo;
- struct condition_variable *var = info->condvar;
-
mutex_lock(&proc_lock);
if (--var->refcount == 0) {
list_del(&var->list);
@@ -171,6 +149,42 @@ static void condition_mt_destroy(const struct xt_mtdtor_param *par)
}
mutex_unlock(&proc_lock);
}
+EXPORT_SYMBOL_GPL(xt_condition_put);
+
+void xt_condition_set(struct condition_variable *var, bool enabled)
+{
+ var->enabled = enabled;
+}
+EXPORT_SYMBOL_GPL(xt_condition_set);
+
+static int condition_mt_check(const struct xt_mtchk_param *par)
+{
+ struct xt_condition_mtinfo *info = par->matchinfo;
+ struct condition_variable *var;
+
+ /* Forbid certain names */
+ if (*info->name == '\0' || *info->name == '.' ||
+ info->name[sizeof(info->name)-1] != '\0' ||
+ memchr(info->name, '/', sizeof(info->name)) != NULL) {
+ pr_info("name not allowed or too long: \"%.*s\"\n",
+ (unsigned int)sizeof(info->name), info->name);
+ return -EINVAL;
+ }
+
+ var = xt_condition_insert(info->name);
+ if (var == NULL)
+ return -ENOMEM;
+
+ info->condvar = var;
+ return 0;
+}
+
+static void condition_mt_destroy(const struct xt_mtdtor_param *par)
+{
+ const struct xt_condition_mtinfo *info = par->matchinfo;
+
+ xt_condition_put(info->condvar);
+}
static struct xt_match condition_mt_reg __read_mostly = {
.name = "condition",
--
1.7.0.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC 2/2] netfilter: xtables: implement CONDITION target
2010-07-19 14:15 [RFC 0/2] netfilter: xtables: CONDITION target implementation Luciano Coelho
2010-07-19 14:15 ` [RFC 1/2] netfilter: xt_condition: export list management code Luciano Coelho
@ 2010-07-19 14:15 ` Luciano Coelho
2010-07-19 14:27 ` [RFC 0/2] netfilter: xtables: CONDITION target implementation Changli Gao
2 siblings, 0 replies; 8+ messages in thread
From: Luciano Coelho @ 2010-07-19 14:15 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, kaber, jengelh, sameo
From: Luciano Coelho <coelho@testbed>
This patch implements a condition target, which let's the user set
netfilter rules that enable/disable the conditions used by the
condition match. Originally, the condition match only allowed the
variable to be changed via procfs. This new target makes it easy to
enable or disable the condition depending on the rules set.
Signed-off-by: Luciano Coelho <luciano.coelho@nokia.com>
---
include/linux/netfilter/Kbuild | 1 +
include/linux/netfilter/xt_CONDITION.h | 39 +++++++++++
net/netfilter/Kconfig | 12 ++++
net/netfilter/Makefile | 1 +
net/netfilter/xt_CONDITION.c | 112 ++++++++++++++++++++++++++++++++
5 files changed, 165 insertions(+), 0 deletions(-)
create mode 100644 include/linux/netfilter/xt_CONDITION.h
create mode 100644 net/netfilter/xt_CONDITION.c
diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
index c57e099..72eff3a 100644
--- a/include/linux/netfilter/Kbuild
+++ b/include/linux/netfilter/Kbuild
@@ -4,6 +4,7 @@ header-y += nfnetlink_conntrack.h
header-y += nfnetlink_log.h
header-y += nfnetlink_queue.h
header-y += xt_CLASSIFY.h
+header-y += xt_CONDITION.h
header-y += xt_CONNMARK.h
header-y += xt_CONNSECMARK.h
header-y += xt_CT.h
diff --git a/include/linux/netfilter/xt_CONDITION.h b/include/linux/netfilter/xt_CONDITION.h
new file mode 100644
index 0000000..cbffe3f
--- /dev/null
+++ b/include/linux/netfilter/xt_CONDITION.h
@@ -0,0 +1,39 @@
+/*
+ * linux/include/linux/netfilter/xt_CONDITION.h
+ *
+ * Header file for Xtables timer target module.
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contact: Luciano Coelho <luciano.coelho@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#ifndef _XT_CONDITION_TG_H
+#define _XT_CONDITION_TG_H
+
+#include <linux/types.h>
+#include <linux/netfilter/xt_condition.h>
+
+struct condition_tg_info {
+ char name[XT_CONDITION_MAX_NAME_SIZE + 1];
+ __u8 enabled;
+
+ /* Used internally by the kernel */
+ void *condvar __attribute__((aligned(8)));
+};
+
+#endif
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index e54e216..1877c6a 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -310,6 +310,18 @@ config NETFILTER_XT_MARK
"Use netfilter MARK value as routing key") and can also be used by
other subsystems to change their behavior.
+config NETFILTER_XT_TARGET_CONDITION
+ tristate "'CONDITION' target support"
+ depends on NETFILTER_ADVANCED
+ select NETFILTER_XT_MATCH_CONDITION
+ help
+
+ Allows changing the condition match value in procfs from the
+ netfilter tables, without requiring userspace to change the
+ condition value.
+
+ To compile it as a module, choose M here. If unsure, say N.
+
config NETFILTER_XT_CONNMARK
tristate 'ctmark target and match support'
depends on NF_CONNTRACK
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 474dd06..9237a67 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_NETFILTER_XT_CONNMARK) += xt_connmark.o
# targets
obj-$(CONFIG_NETFILTER_XT_TARGET_CLASSIFY) += xt_CLASSIFY.o
+obj-$(CONFIG_NETFILTER_XT_TARGET_CONDITION) += xt_CONDITION.o
obj-$(CONFIG_NETFILTER_XT_TARGET_CONNSECMARK) += xt_CONNSECMARK.o
obj-$(CONFIG_NETFILTER_XT_TARGET_CT) += xt_CT.o
obj-$(CONFIG_NETFILTER_XT_TARGET_DSCP) += xt_DSCP.o
diff --git a/net/netfilter/xt_CONDITION.c b/net/netfilter/xt_CONDITION.c
new file mode 100644
index 0000000..8150352
--- /dev/null
+++ b/net/netfilter/xt_CONDITION.c
@@ -0,0 +1,112 @@
+/*
+ * linux/net/netfilter/xt_CONDITION.c
+ *
+ * Netfilter module to trigger a timer when packet matches.
+ * After timer expires a kevent will be sent.
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contact: Luciano Coelho <luciano.coelho@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/xt_CONDITION.h>
+
+static unsigned int condition_tg_target(struct sk_buff *skb,
+ const struct xt_action_param *par)
+{
+ const struct condition_tg_info *info = par->targinfo;
+
+ pr_debug("setting condition %s, enabled %d\n",
+ info->name, info->enabled);
+
+ xt_condition_set(info->condvar, info->enabled);
+
+ return XT_CONTINUE;
+}
+
+static int condition_tg_checkentry(const struct xt_tgchk_param *par)
+{
+ struct condition_tg_info *info = par->targinfo;
+ struct condition_variable *var;
+
+ pr_debug("checkentry %s\n", info->name);
+
+ /* Forbid certain names */
+ if (*info->name == '\0' || *info->name == '.' ||
+ info->name[sizeof(info->name)-1] != '\0' ||
+ memchr(info->name, '/', sizeof(info->name)) != NULL) {
+ pr_info("name not allowed or too long: \"%.*s\"\n",
+ (unsigned int)sizeof(info->name), info->name);
+ return -EINVAL;
+ }
+
+ var = xt_condition_insert(info->name);
+ if (var == NULL)
+ return -ENOMEM;
+
+ info->condvar = var;
+ return 0;
+}
+
+static void condition_tg_destroy(const struct xt_tgdtor_param *par)
+{
+ const struct condition_tg_info *info = par->targinfo;
+
+ pr_debug("destroy %s\n", info->name);
+
+ xt_condition_put(info->condvar);
+}
+
+static struct xt_target condition_tg __read_mostly = {
+ .name = "CONDITION",
+ .family = NFPROTO_UNSPEC,
+ .target = condition_tg_target,
+ .targetsize = sizeof(struct condition_tg_info),
+ .checkentry = condition_tg_checkentry,
+ .destroy = condition_tg_destroy,
+ .me = THIS_MODULE,
+};
+
+static int __init condition_tg_init(void)
+{
+ int err;
+
+ err = xt_register_target(&condition_tg);
+ if (err < 0) {
+ pr_debug("couldn't register xt target\n");
+ return err;
+ }
+
+ return 0;
+}
+
+static void __exit condition_tg_exit(void)
+{
+ xt_unregister_target(&condition_tg);
+}
+
+module_init(condition_tg_init);
+module_exit(condition_tg_exit);
+
+MODULE_AUTHOR("Luciano Coelho <luciano.coelho@nokia.com>");
+MODULE_DESCRIPTION("Xtables: condition target");
+MODULE_LICENSE("GPL v2");
--
1.7.0.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC 0/2] netfilter: xtables: CONDITION target implementation
2010-07-19 14:15 [RFC 0/2] netfilter: xtables: CONDITION target implementation Luciano Coelho
2010-07-19 14:15 ` [RFC 1/2] netfilter: xt_condition: export list management code Luciano Coelho
2010-07-19 14:15 ` [RFC 2/2] netfilter: xtables: implement CONDITION target Luciano Coelho
@ 2010-07-19 14:27 ` Changli Gao
2010-07-19 14:31 ` Luciano Coelho
2 siblings, 1 reply; 8+ messages in thread
From: Changli Gao @ 2010-07-19 14:27 UTC (permalink / raw)
To: Luciano Coelho; +Cc: netfilter-devel, netdev, kaber, jengelh, sameo
On Mon, Jul 19, 2010 at 10:15 PM, Luciano Coelho
<luciano.coelho@nokia.com> wrote:
> From: Luciano Coelho <coelho@testbed.(none)>
>
> Hi all,
>
> As discussed earlier, I've been looking for a way to enable and disable the
> condition match automatically, in the netfilter tables themselves (ie. without
> the need to use procfs).
>
> This is my initial implementation. Please let me know how it looks. The first
> patch is based on the xt_condition patch that Jan sent to the list (but which
> has not been finalized for inclusion yet). Once the condition match gets
> applied, I'll forward port my patch and submit it again.
>
> Cheers,
> Luca.
>
> Luciano Coelho (2):
> netfilter: xt_condition: export list management code
> netfilter: xtables: implement CONDITION target
>
> include/linux/netfilter/Kbuild | 1 +
> include/linux/netfilter/xt_CONDITION.h | 39 +++++++++++
> include/linux/netfilter/xt_condition.h | 17 +++++-
> net/netfilter/Kconfig | 12 ++++
> net/netfilter/Makefile | 1 +
> net/netfilter/xt_CONDITION.c | 112 ++++++++++++++++++++++++++++++++
> net/netfilter/xt_condition.c | 82 ++++++++++++++----------
Why not combine xt_CONDITION.c and xt_condition.c into xt_condition.c,
like xt_mark.c?
--
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC 0/2] netfilter: xtables: CONDITION target implementation
2010-07-19 14:27 ` [RFC 0/2] netfilter: xtables: CONDITION target implementation Changli Gao
@ 2010-07-19 14:31 ` Luciano Coelho
0 siblings, 0 replies; 8+ messages in thread
From: Luciano Coelho @ 2010-07-19 14:31 UTC (permalink / raw)
To: ext Changli Gao
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
kaber@trash.net, jengelh@medozas.de, sameo@linux.intel.com
On Mon, 2010-07-19 at 16:27 +0200, ext Changli Gao wrote:
> On Mon, Jul 19, 2010 at 10:15 PM, Luciano Coelho
> <luciano.coelho@nokia.com> wrote:
> > From: Luciano Coelho <coelho@testbed.(none)>
> >
> > Hi all,
> >
> > As discussed earlier, I've been looking for a way to enable and disable the
> > condition match automatically, in the netfilter tables themselves (ie. without
> > the need to use procfs).
> >
> > This is my initial implementation. Please let me know how it looks. The first
> > patch is based on the xt_condition patch that Jan sent to the list (but which
> > has not been finalized for inclusion yet). Once the condition match gets
> > applied, I'll forward port my patch and submit it again.
> >
> > Cheers,
> > Luca.
> >
> > Luciano Coelho (2):
> > netfilter: xt_condition: export list management code
> > netfilter: xtables: implement CONDITION target
> >
> > include/linux/netfilter/Kbuild | 1 +
> > include/linux/netfilter/xt_CONDITION.h | 39 +++++++++++
> > include/linux/netfilter/xt_condition.h | 17 +++++-
> > net/netfilter/Kconfig | 12 ++++
> > net/netfilter/Makefile | 1 +
> > net/netfilter/xt_CONDITION.c | 112 ++++++++++++++++++++++++++++++++
> > net/netfilter/xt_condition.c | 82 ++++++++++++++----------
>
> Why not combine xt_CONDITION.c and xt_condition.c into xt_condition.c,
> like xt_mark.c?
I just thought that someone may want to use the condition match without
using the CONDITION target, that's why I've put it in a different
module.
But I don't have a strong opinion about this. If everybody agrees on
that, I can merge the code into a single module.
Thanks for your comment.
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC 1/2] netfilter: xt_condition: export list management code
2010-07-19 14:15 ` [RFC 1/2] netfilter: xt_condition: export list management code Luciano Coelho
@ 2010-07-19 16:13 ` Jan Engelhardt
2010-07-19 19:14 ` Luciano Coelho
0 siblings, 1 reply; 8+ messages in thread
From: Jan Engelhardt @ 2010-07-19 16:13 UTC (permalink / raw)
To: Luciano Coelho
Cc: Netfilter Developer Mailing List, netdev, Patrick McHardy, sameo
On Monday 2010-07-19 16:15, Luciano Coelho wrote:
>From: Luciano Coelho <coelho@testbed>
>
>This patch isolates and exports the condition list management code, in
>preparation for the CONDITION target to use it. No functional change,
>just reorganization of the code.
Well, I guess it would make more sense if the two extensions be in a
single file. That would alleviate the need for export reorganizations,
and also works because the module metadata overhead is large already.
>@@ -3,12 +3,27 @@
>
> #include <linux/types.h>
>
>+#define XT_CONDITION_MAX_NAME_SIZE 30
>+
> struct xt_condition_mtinfo {
>- char name[31];
>+ char name[XT_CONDITION_MAX_NAME_SIZE + 1];
> __u8 invert;
Oh noes. Please please avoid any math operations inside []. It has
already driven XT_FUNCTION_MAXNAMELEN into nuts ("was it now +1 or -1,
or even -2 that we needed to pass for various functions?"). Just let MAX
be 31 and have name[MAX].
> MODULE_ALIAS("ip6t_condition");
>
>-struct condition_variable {
>- struct list_head list;
>- struct proc_dir_entry *status_proc;
>- unsigned int refcount;
>- bool enabled;
>-};
Given your excellent usage example of a CONDITION target, I think it
even makes sense to enlarge the "enabled" variable to a full-fledged
32-bit value that can be &, | and ^'d, similar to nfmark.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC 1/2] netfilter: xt_condition: export list management code
2010-07-19 16:13 ` Jan Engelhardt
@ 2010-07-19 19:14 ` Luciano Coelho
2010-07-19 19:31 ` Luciano Coelho
0 siblings, 1 reply; 8+ messages in thread
From: Luciano Coelho @ 2010-07-19 19:14 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: Netfilter Developer Mailing List, netdev@vger.kernel.org,
Patrick McHardy, sameo@linux.intel.com
On Mon, 2010-07-19 at 18:13 +0200, ext Jan Engelhardt wrote:
> On Monday 2010-07-19 16:15, Luciano Coelho wrote:
>
> >From: Luciano Coelho <coelho@testbed>
> >
> >This patch isolates and exports the condition list management code, in
> >preparation for the CONDITION target to use it. No functional change,
> >just reorganization of the code.
>
> Well, I guess it would make more sense if the two extensions be in a
> single file. That would alleviate the need for export reorganizations,
> and also works because the module metadata overhead is large already.
Right. I'll change the code so that the two extensions are in the same
file/module. You're the second person to mention this already. ;)
> >@@ -3,12 +3,27 @@
> >
> > #include <linux/types.h>
> >
> >+#define XT_CONDITION_MAX_NAME_SIZE 30
> >+
> > struct xt_condition_mtinfo {
> >- char name[31];
> >+ char name[XT_CONDITION_MAX_NAME_SIZE + 1];
> > __u8 invert;
>
> Oh noes. Please please avoid any math operations inside []. It has
> already driven XT_FUNCTION_MAXNAMELEN into nuts ("was it now +1 or -1,
> or even -2 that we needed to pass for various functions?"). Just let MAX
> be 31 and have name[MAX].
Yeah, I had already done as you suggested in my previous module
(IDLETIMER), I don't know what I had in my head today when I did it
differently. Even the name of the macro is totally wrong (_SIZE), it
would make a tiny little bit more sense if it was _LEN. I'll change it.
> > MODULE_ALIAS("ip6t_condition");
> >
> >-struct condition_variable {
> >- struct list_head list;
> >- struct proc_dir_entry *status_proc;
> >- unsigned int refcount;
> >- bool enabled;
> >-};
>
> Given your excellent usage example of a CONDITION target, I think it
> even makes sense to enlarge the "enabled" variable to a full-fledged
> 32-bit value that can be &, | and ^'d, similar to nfmark.
Ok, that's a good idea, I'll do that.
Thanks for your comments!
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC 1/2] netfilter: xt_condition: export list management code
2010-07-19 19:14 ` Luciano Coelho
@ 2010-07-19 19:31 ` Luciano Coelho
0 siblings, 0 replies; 8+ messages in thread
From: Luciano Coelho @ 2010-07-19 19:31 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: Netfilter Developer Mailing List, netdev@vger.kernel.org,
Patrick McHardy, sameo@linux.intel.com
On Mon, 2010-07-19 at 21:14 +0200, Coelho Luciano (Nokia-MS/Helsinki)
wrote:
> On Mon, 2010-07-19 at 18:13 +0200, ext Jan Engelhardt wrote:
> > On Monday 2010-07-19 16:15, Luciano Coelho wrote:
> > >@@ -3,12 +3,27 @@
> > >
> > > #include <linux/types.h>
> > >
> > >+#define XT_CONDITION_MAX_NAME_SIZE 30
> > >+
> > > struct xt_condition_mtinfo {
> > >- char name[31];
> > >+ char name[XT_CONDITION_MAX_NAME_SIZE + 1];
> > > __u8 invert;
> >
> > Oh noes. Please please avoid any math operations inside []. It has
> > already driven XT_FUNCTION_MAXNAMELEN into nuts ("was it now +1 or -1,
> > or even -2 that we needed to pass for various functions?"). Just let MAX
> > be 31 and have name[MAX].
>
> Yeah, I had already done as you suggested in my previous module
> (IDLETIMER), I don't know what I had in my head today when I did it
> differently. Even the name of the macro is totally wrong (_SIZE), it
> would make a tiny little bit more sense if it was _LEN. I'll change it.
I was not very clear here, I meant I'll change to what you proposed, ie.
keep it _SIZE and use 31, of course.
--
Cheers,
Luca.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-07-19 19:31 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-19 14:15 [RFC 0/2] netfilter: xtables: CONDITION target implementation Luciano Coelho
2010-07-19 14:15 ` [RFC 1/2] netfilter: xt_condition: export list management code Luciano Coelho
2010-07-19 16:13 ` Jan Engelhardt
2010-07-19 19:14 ` Luciano Coelho
2010-07-19 19:31 ` Luciano Coelho
2010-07-19 14:15 ` [RFC 2/2] netfilter: xtables: implement CONDITION target Luciano Coelho
2010-07-19 14:27 ` [RFC 0/2] netfilter: xtables: CONDITION target implementation Changli Gao
2010-07-19 14:31 ` Luciano Coelho
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).