* Bugfix: libnetfilter_conntrack getter and setter
@ 2007-01-04 10:03 Victor Stinner
2007-01-05 14:45 ` Pablo Neira Ayuso
0 siblings, 1 reply; 3+ messages in thread
From: Victor Stinner @ 2007-01-04 10:03 UTC (permalink / raw)
To: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 564 bytes --]
Hi,
Libnetfilter_conntrack getters and setters of new API are not complete:
* it's not possible to set counter attributes value
* it's not possible to set or read 'use' and 'id' attributes value
I can understand that setting counter values is not very useful, but trying to
set them would lead to a crash (call NULL function). Same problem when trying
to read use/id attribute value.
I attached two patches to fix that.
---
An alternative for nfct_set_attr() is to do nothing if the getter in NULL (and
set an error?).
Victor Stinner
http://www.inl.fr/
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: libnetfilter_conntrack_getter.patch --]
[-- Type: text/x-diff; charset="us-ascii"; name="libnetfilter_conntrack_getter.patch", Size: 1059 bytes --]
Index: src/conntrack/getter.c
===================================================================
--- src/conntrack/getter.c (révision 6720)
+++ src/conntrack/getter.c (copie de travail)
@@ -157,6 +157,16 @@
return &ct->counters[__DIR_REPL].bytes;
}
+static const void *get_attr_use(const struct nf_conntrack *ct)
+{
+ return &ct->use;
+}
+
+static const void *get_attr_id(const struct nf_conntrack *ct)
+{
+ return &ct->id;
+}
+
static const void *get_attr_status(const struct nf_conntrack *ct)
{
return &ct->status;
@@ -190,8 +200,10 @@
[ATTR_TIMEOUT] = get_attr_timeout,
[ATTR_MARK] = get_attr_mark,
[ATTR_ORIG_COUNTER_PACKETS] = get_attr_orig_counter_packets,
- [ATTR_ORIG_COUNTER_BYTES] = get_attr_orig_counter_bytes,
[ATTR_REPL_COUNTER_PACKETS] = get_attr_repl_counter_packets,
+ [ATTR_ORIG_COUNTER_BYTES] = get_attr_orig_counter_bytes,
[ATTR_REPL_COUNTER_BYTES] = get_attr_repl_counter_bytes,
+ [ATTR_USE] = get_attr_use,
+ [ATTR_ID] = get_attr_id,
[ATTR_STATUS] = get_attr_status,
};
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: libnetfilter_conntrack_setter.patch --]
[-- Type: text/x-diff; charset="us-ascii"; name="libnetfilter_conntrack_setter.patch", Size: 4193 bytes --]
Index: src/conntrack/setter.c
===================================================================
--- src/conntrack/setter.c (révision 6720)
+++ src/conntrack/setter.c (copie de travail)
@@ -137,37 +137,73 @@
ct->mark = *((u_int32_t *) value);
}
+static void set_attr_orig_counter_packets(struct nf_conntrack *ct, const void *value)
+{
+ ct->counters[__DIR_ORIG].packets = *((u_int64_t *) value);
+}
+
+static void set_attr_repl_counter_packets(struct nf_conntrack *ct, const void *value)
+{
+ ct->counters[__DIR_REPL].packets = *((u_int64_t *) value);
+}
+
+static void set_attr_orig_counter_bytes(struct nf_conntrack *ct, const void *value)
+{
+ ct->counters[__DIR_ORIG].bytes = *((u_int64_t *) value);
+}
+
+static void set_attr_repl_counter_bytes(struct nf_conntrack *ct, const void *value)
+{
+ ct->counters[__DIR_REPL].bytes = *((u_int64_t *) value);
+}
+
+static void set_attr_use(struct nf_conntrack *ct, const void *value)
+{
+ ct->use = *((u_int32_t *) value);
+}
+
+static void set_attr_id(struct nf_conntrack *ct, const void *value)
+{
+ ct->id = *((u_int32_t *) value);
+}
+
static void set_attr_status(struct nf_conntrack *ct, const void *value)
{
ct->status |= *((u_int32_t *) value);
}
set_attr set_attr_array[] = {
- [ATTR_ORIG_IPV4_SRC] = set_attr_orig_ipv4_src,
- [ATTR_ORIG_IPV4_DST] = set_attr_orig_ipv4_dst,
- [ATTR_REPL_IPV4_SRC] = set_attr_repl_ipv4_src,
- [ATTR_REPL_IPV4_DST] = set_attr_repl_ipv4_dst,
- [ATTR_ORIG_IPV6_SRC] = set_attr_orig_ipv6_src,
- [ATTR_ORIG_IPV6_DST] = set_attr_orig_ipv6_dst,
- [ATTR_REPL_IPV6_SRC] = set_attr_repl_ipv6_src,
- [ATTR_REPL_IPV6_DST] = set_attr_repl_ipv6_dst,
- [ATTR_ORIG_PORT_SRC] = set_attr_orig_port_src,
- [ATTR_ORIG_PORT_DST] = set_attr_orig_port_dst,
- [ATTR_REPL_PORT_SRC] = set_attr_repl_port_src,
- [ATTR_REPL_PORT_DST] = set_attr_repl_port_dst,
- [ATTR_ICMP_TYPE] = set_attr_icmp_type,
- [ATTR_ICMP_CODE] = set_attr_icmp_code,
- [ATTR_ICMP_ID] = set_attr_icmp_id,
- [ATTR_ORIG_L3PROTO] = set_attr_orig_l3proto,
- [ATTR_REPL_L3PROTO] = set_attr_repl_l3proto,
- [ATTR_ORIG_L4PROTO] = set_attr_orig_l4proto,
- [ATTR_REPL_L4PROTO] = set_attr_repl_l4proto,
- [ATTR_TCP_STATE] = set_attr_tcp_state,
- [ATTR_SNAT_IPV4] = set_attr_snat_ipv4,
- [ATTR_DNAT_IPV4] = set_attr_dnat_ipv4,
- [ATTR_SNAT_PORT] = set_attr_snat_port,
- [ATTR_DNAT_PORT] = set_attr_dnat_port,
- [ATTR_TIMEOUT] = set_attr_timeout,
- [ATTR_MARK] = set_attr_mark,
- [ATTR_STATUS] = set_attr_status,
+ [ATTR_ORIG_IPV4_SRC] = set_attr_orig_ipv4_src,
+ [ATTR_ORIG_IPV4_DST] = set_attr_orig_ipv4_dst,
+ [ATTR_REPL_IPV4_SRC] = set_attr_repl_ipv4_src,
+ [ATTR_REPL_IPV4_DST] = set_attr_repl_ipv4_dst,
+ [ATTR_ORIG_IPV6_SRC] = set_attr_orig_ipv6_src,
+ [ATTR_ORIG_IPV6_DST] = set_attr_orig_ipv6_dst,
+ [ATTR_REPL_IPV6_SRC] = set_attr_repl_ipv6_src,
+ [ATTR_REPL_IPV6_DST] = set_attr_repl_ipv6_dst,
+ [ATTR_ORIG_PORT_SRC] = set_attr_orig_port_src,
+ [ATTR_ORIG_PORT_DST] = set_attr_orig_port_dst,
+ [ATTR_REPL_PORT_SRC] = set_attr_repl_port_src,
+ [ATTR_REPL_PORT_DST] = set_attr_repl_port_dst,
+ [ATTR_ICMP_TYPE] = set_attr_icmp_type,
+ [ATTR_ICMP_CODE] = set_attr_icmp_code,
+ [ATTR_ICMP_ID] = set_attr_icmp_id,
+ [ATTR_ORIG_L3PROTO] = set_attr_orig_l3proto,
+ [ATTR_REPL_L3PROTO] = set_attr_repl_l3proto,
+ [ATTR_ORIG_L4PROTO] = set_attr_orig_l4proto,
+ [ATTR_REPL_L4PROTO] = set_attr_repl_l4proto,
+ [ATTR_TCP_STATE] = set_attr_tcp_state,
+ [ATTR_SNAT_IPV4] = set_attr_snat_ipv4,
+ [ATTR_DNAT_IPV4] = set_attr_dnat_ipv4,
+ [ATTR_SNAT_PORT] = set_attr_snat_port,
+ [ATTR_DNAT_PORT] = set_attr_dnat_port,
+ [ATTR_TIMEOUT] = set_attr_timeout,
+ [ATTR_MARK] = set_attr_mark,
+ [ATTR_ORIG_COUNTER_PACKETS] = set_attr_orig_counter_packets,
+ [ATTR_REPL_COUNTER_PACKETS] = set_attr_repl_counter_packets,
+ [ATTR_ORIG_COUNTER_BYTES] = set_attr_orig_counter_bytes,
+ [ATTR_REPL_COUNTER_BYTES] = set_attr_repl_counter_bytes,
+ [ATTR_USE] = set_attr_use,
+ [ATTR_ID] = set_attr_id,
+ [ATTR_STATUS] = set_attr_status,
};
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: Bugfix: libnetfilter_conntrack getter and setter
2007-01-04 10:03 Bugfix: libnetfilter_conntrack getter and setter Victor Stinner
@ 2007-01-05 14:45 ` Pablo Neira Ayuso
0 siblings, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2007-01-05 14:45 UTC (permalink / raw)
To: Victor Stinner; +Cc: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]
Hi,
Victor Stinner wrote:
> Libnetfilter_conntrack getters and setters of new API are not complete:
> * it's not possible to set counter attributes value
Because of ctnetlink, the kernel part of this whole thing, doesn't
support this. Anyway, as you pointed out below, I can't see how this
could be useful.
> * it's not possible to set or read 'use' and 'id' attributes value
The 'use' attribute must be possible to be get, but not set. I'll commit
the patch for the getter mangled, I prefer dropping the 'id' support
since it's planned to be removed.
> I can understand that setting counter values is not very useful, but trying to
> set them would lead to a crash (call NULL function). Same problem when trying
> to read use/id attribute value.
OK, I think that the patch attached should be enough.
> An alternative for nfct_set_attr() is to do nothing if the getter in NULL (and
> set an error?).
I prefer doing nothing and documenting this issue, perhaps doing some
kind of warning or assertion, although that would be too much I think.
Moreover, the error thing would pollute the code with tons of error
checkings in the set operations.
Thanks again Victor.
--
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1527 bytes --]
Index: src/conntrack/api.c
===================================================================
--- src/conntrack/api.c (revisión: 6716)
+++ src/conntrack/api.c (copia de trabajo)
@@ -185,6 +185,12 @@
* @ct: pointer to a valid conntrack
* @type: attribute type
* @value: pointer to the attribute value
+ *
+ * Note that certain attributes are unsettable:
+ * - ATTR_USE
+ * - ATTR_ID
+ * - ATTR_*_COUNTER_*
+ * The call of this function for such attributes do nothing.
*/
void nfct_set_attr(struct nf_conntrack *ct,
const enum nf_conntrack_attr type,
@@ -196,8 +202,10 @@
if (type >= ATTR_MAX)
return;
- set_attr_array[type](ct, value);
- set_bit(type, ct->set);
+ if (set_attr_array[type]) {
+ set_attr_array[type](ct, value);
+ set_bit(type, ct->set);
+ }
}
/**
Index: src/conntrack/getter.c
===================================================================
--- src/conntrack/getter.c (revisión: 6716)
+++ src/conntrack/getter.c (copia de trabajo)
@@ -162,6 +162,11 @@
return &ct->status;
}
+static const void *get_attr_use(const struct nf_conntrack *ct)
+{
+ return &ct->use;
+}
+
get_attr get_attr_array[] = {
[ATTR_ORIG_IPV4_SRC] = get_attr_orig_ipv4_src,
[ATTR_ORIG_IPV4_DST] = get_attr_orig_ipv4_dst,
@@ -193,5 +198,6 @@
[ATTR_ORIG_COUNTER_BYTES] = get_attr_orig_counter_bytes,
[ATTR_REPL_COUNTER_PACKETS] = get_attr_repl_counter_packets,
[ATTR_REPL_COUNTER_BYTES] = get_attr_repl_counter_bytes,
+ [ATTR_USE] = get_attr_use,
[ATTR_STATUS] = get_attr_status,
};
^ permalink raw reply [flat|nested] 3+ messages in thread
* Bugfix: libnetfilter_conntrack getter and setter
@ 2007-01-03 18:42 Victor Stinner
0 siblings, 0 replies; 3+ messages in thread
From: Victor Stinner @ 2007-01-03 18:42 UTC (permalink / raw)
To: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 575 bytes --]
Hi,
Libnetfilter_conntrack getters and setters of new API are not complete:
* it's not possible to set counter attributes value
* it's not possible to set or read 'use' and 'id' attributes value
I can understand that setting counter values is not very useful, but trying to
set them would lead to a crash (call NULL function). Same problem when trying
to read use/id attribute value.
I attached two patches to fix that.
---
An alternative for nfct_set_attr() is to do nothing if the getter in NULL (and
set an error?).
Victor Stinner aka haypo
http://hachoir.org/
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: libnetfilter_conntrack_getter.patch --]
[-- Type: text/x-diff; charset="us-ascii"; name="libnetfilter_conntrack_getter.patch", Size: 1059 bytes --]
Index: src/conntrack/getter.c
===================================================================
--- src/conntrack/getter.c (révision 6720)
+++ src/conntrack/getter.c (copie de travail)
@@ -157,6 +157,16 @@
return &ct->counters[__DIR_REPL].bytes;
}
+static const void *get_attr_use(const struct nf_conntrack *ct)
+{
+ return &ct->use;
+}
+
+static const void *get_attr_id(const struct nf_conntrack *ct)
+{
+ return &ct->id;
+}
+
static const void *get_attr_status(const struct nf_conntrack *ct)
{
return &ct->status;
@@ -190,8 +200,10 @@
[ATTR_TIMEOUT] = get_attr_timeout,
[ATTR_MARK] = get_attr_mark,
[ATTR_ORIG_COUNTER_PACKETS] = get_attr_orig_counter_packets,
- [ATTR_ORIG_COUNTER_BYTES] = get_attr_orig_counter_bytes,
[ATTR_REPL_COUNTER_PACKETS] = get_attr_repl_counter_packets,
+ [ATTR_ORIG_COUNTER_BYTES] = get_attr_orig_counter_bytes,
[ATTR_REPL_COUNTER_BYTES] = get_attr_repl_counter_bytes,
+ [ATTR_USE] = get_attr_use,
+ [ATTR_ID] = get_attr_id,
[ATTR_STATUS] = get_attr_status,
};
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: libnetfilter_conntrack_setter.patch --]
[-- Type: text/x-diff; charset="us-ascii"; name="libnetfilter_conntrack_setter.patch", Size: 4193 bytes --]
Index: src/conntrack/setter.c
===================================================================
--- src/conntrack/setter.c (révision 6720)
+++ src/conntrack/setter.c (copie de travail)
@@ -137,37 +137,73 @@
ct->mark = *((u_int32_t *) value);
}
+static void set_attr_orig_counter_packets(struct nf_conntrack *ct, const void *value)
+{
+ ct->counters[__DIR_ORIG].packets = *((u_int64_t *) value);
+}
+
+static void set_attr_repl_counter_packets(struct nf_conntrack *ct, const void *value)
+{
+ ct->counters[__DIR_REPL].packets = *((u_int64_t *) value);
+}
+
+static void set_attr_orig_counter_bytes(struct nf_conntrack *ct, const void *value)
+{
+ ct->counters[__DIR_ORIG].bytes = *((u_int64_t *) value);
+}
+
+static void set_attr_repl_counter_bytes(struct nf_conntrack *ct, const void *value)
+{
+ ct->counters[__DIR_REPL].bytes = *((u_int64_t *) value);
+}
+
+static void set_attr_use(struct nf_conntrack *ct, const void *value)
+{
+ ct->use = *((u_int32_t *) value);
+}
+
+static void set_attr_id(struct nf_conntrack *ct, const void *value)
+{
+ ct->id = *((u_int32_t *) value);
+}
+
static void set_attr_status(struct nf_conntrack *ct, const void *value)
{
ct->status |= *((u_int32_t *) value);
}
set_attr set_attr_array[] = {
- [ATTR_ORIG_IPV4_SRC] = set_attr_orig_ipv4_src,
- [ATTR_ORIG_IPV4_DST] = set_attr_orig_ipv4_dst,
- [ATTR_REPL_IPV4_SRC] = set_attr_repl_ipv4_src,
- [ATTR_REPL_IPV4_DST] = set_attr_repl_ipv4_dst,
- [ATTR_ORIG_IPV6_SRC] = set_attr_orig_ipv6_src,
- [ATTR_ORIG_IPV6_DST] = set_attr_orig_ipv6_dst,
- [ATTR_REPL_IPV6_SRC] = set_attr_repl_ipv6_src,
- [ATTR_REPL_IPV6_DST] = set_attr_repl_ipv6_dst,
- [ATTR_ORIG_PORT_SRC] = set_attr_orig_port_src,
- [ATTR_ORIG_PORT_DST] = set_attr_orig_port_dst,
- [ATTR_REPL_PORT_SRC] = set_attr_repl_port_src,
- [ATTR_REPL_PORT_DST] = set_attr_repl_port_dst,
- [ATTR_ICMP_TYPE] = set_attr_icmp_type,
- [ATTR_ICMP_CODE] = set_attr_icmp_code,
- [ATTR_ICMP_ID] = set_attr_icmp_id,
- [ATTR_ORIG_L3PROTO] = set_attr_orig_l3proto,
- [ATTR_REPL_L3PROTO] = set_attr_repl_l3proto,
- [ATTR_ORIG_L4PROTO] = set_attr_orig_l4proto,
- [ATTR_REPL_L4PROTO] = set_attr_repl_l4proto,
- [ATTR_TCP_STATE] = set_attr_tcp_state,
- [ATTR_SNAT_IPV4] = set_attr_snat_ipv4,
- [ATTR_DNAT_IPV4] = set_attr_dnat_ipv4,
- [ATTR_SNAT_PORT] = set_attr_snat_port,
- [ATTR_DNAT_PORT] = set_attr_dnat_port,
- [ATTR_TIMEOUT] = set_attr_timeout,
- [ATTR_MARK] = set_attr_mark,
- [ATTR_STATUS] = set_attr_status,
+ [ATTR_ORIG_IPV4_SRC] = set_attr_orig_ipv4_src,
+ [ATTR_ORIG_IPV4_DST] = set_attr_orig_ipv4_dst,
+ [ATTR_REPL_IPV4_SRC] = set_attr_repl_ipv4_src,
+ [ATTR_REPL_IPV4_DST] = set_attr_repl_ipv4_dst,
+ [ATTR_ORIG_IPV6_SRC] = set_attr_orig_ipv6_src,
+ [ATTR_ORIG_IPV6_DST] = set_attr_orig_ipv6_dst,
+ [ATTR_REPL_IPV6_SRC] = set_attr_repl_ipv6_src,
+ [ATTR_REPL_IPV6_DST] = set_attr_repl_ipv6_dst,
+ [ATTR_ORIG_PORT_SRC] = set_attr_orig_port_src,
+ [ATTR_ORIG_PORT_DST] = set_attr_orig_port_dst,
+ [ATTR_REPL_PORT_SRC] = set_attr_repl_port_src,
+ [ATTR_REPL_PORT_DST] = set_attr_repl_port_dst,
+ [ATTR_ICMP_TYPE] = set_attr_icmp_type,
+ [ATTR_ICMP_CODE] = set_attr_icmp_code,
+ [ATTR_ICMP_ID] = set_attr_icmp_id,
+ [ATTR_ORIG_L3PROTO] = set_attr_orig_l3proto,
+ [ATTR_REPL_L3PROTO] = set_attr_repl_l3proto,
+ [ATTR_ORIG_L4PROTO] = set_attr_orig_l4proto,
+ [ATTR_REPL_L4PROTO] = set_attr_repl_l4proto,
+ [ATTR_TCP_STATE] = set_attr_tcp_state,
+ [ATTR_SNAT_IPV4] = set_attr_snat_ipv4,
+ [ATTR_DNAT_IPV4] = set_attr_dnat_ipv4,
+ [ATTR_SNAT_PORT] = set_attr_snat_port,
+ [ATTR_DNAT_PORT] = set_attr_dnat_port,
+ [ATTR_TIMEOUT] = set_attr_timeout,
+ [ATTR_MARK] = set_attr_mark,
+ [ATTR_ORIG_COUNTER_PACKETS] = set_attr_orig_counter_packets,
+ [ATTR_REPL_COUNTER_PACKETS] = set_attr_repl_counter_packets,
+ [ATTR_ORIG_COUNTER_BYTES] = set_attr_orig_counter_bytes,
+ [ATTR_REPL_COUNTER_BYTES] = set_attr_repl_counter_bytes,
+ [ATTR_USE] = set_attr_use,
+ [ATTR_ID] = set_attr_id,
+ [ATTR_STATUS] = set_attr_status,
};
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-01-05 14:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-04 10:03 Bugfix: libnetfilter_conntrack getter and setter Victor Stinner
2007-01-05 14:45 ` Pablo Neira Ayuso
-- strict thread matches above, loose matches on Subject: below --
2007-01-03 18:42 Victor Stinner
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.