[PATCH 3/3] Implement private protocol info changing

[PATCH 3/3] Implement private protocol info changing

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 159 bytes --]

This patch add support to change the state of the private protocol 
information via conntrack_netlink.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

[-- Attachment #2: 05change.patch --]
[-- Type: text/x-patch, Size: 1598 bytes --]

Index: netfilter-2.6.14/net/ipv4/netfilter/ip_conntrack_netlink.c
===================================================================
--- netfilter-2.6.14.orig/net/ipv4/netfilter/ip_conntrack_netlink.c	2005-08-13 10:45:09.000000000 +0200
+++ netfilter-2.6.14/net/ipv4/netfilter/ip_conntrack_netlink.c	2005-08-13 10:51:34.000000000 +0200
@@ -948,6 +948,36 @@
 	return 0;
 }
 
+static inline int
+ctnetlink_change_protoinfo(struct ip_conntrack *ct, struct nfattr *cda[])
+{
+	struct nfattr *tb[CTA_PROTOINFO_MAX], *attr = cda[CTA_PROTOINFO-1];
+	struct ip_conntrack_protocol *proto;
+	u_int16_t npt = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum;
+	int err;
+
+	if (nfattr_parse_nested(tb, CTA_PROTOINFO_MAX, attr) < 0)
+		goto nfattr_failure;
+
+	proto = ip_conntrack_proto_find_get(npt);
+	if (!proto)
+		return -EINVAL;
+
+	if (proto->from_nfattr) {
+		err = proto->from_nfattr(tb, ct);
+		if (err < 0) {
+			ip_conntrack_proto_put(proto);
+			return -EINVAL;
+		}
+	}
+	ip_conntrack_proto_put(proto);
+
+	return 0;
+
+nfattr_failure:
+	return -1;
+}
+
 static int
 ctnetlink_change_conntrack(struct ip_conntrack *ct, struct nfattr *cda[])
 {
@@ -973,6 +1003,12 @@
 			return err;
 	}
 
+	if (cda[CTA_PROTOINFO-1]) {
+		err = ctnetlink_change_protoinfo(ct, cda);
+		if (err < 0)
+			return err;
+	}
+
 	DEBUGP("all done\n");
 	return 0;
 }
@@ -1002,6 +1038,12 @@
 	if (err < 0)
 		goto err;
 
+	if (cda[CTA_PROTOINFO-1]) {
+		err = ctnetlink_change_protoinfo(ct, cda);
+		if (err < 0)
+			return err;
+	}
+
 	ct->helper = ip_conntrack_helper_find_get(rtuple);
 
 	add_timer(&ct->timeout);

Re: [PATCH 3/3] Implement private protocol info changing

[Patrick McHardy]

Pablo Neira wrote:
> This patch add support to change the state of the private protocol 
> information via conntrack_netlink.

The patches look fine to me, besides the two issues mentioned below.
I have a couple of fixes which conflict with them though, please
resend after they hit Linus's tree.

> ------------------------------------------------------------------------
> 
> Index: netfilter-2.6.14/net/ipv4/netfilter/ip_conntrack_netlink.c
> ===================================================================
> --- netfilter-2.6.14.orig/net/ipv4/netfilter/ip_conntrack_netlink.c	2005-08-13 10:45:09.000000000 +0200
> +++ netfilter-2.6.14/net/ipv4/netfilter/ip_conntrack_netlink.c	2005-08-13 10:51:34.000000000 +0200
> @@ -948,6 +948,36 @@
>  	return 0;
>  }
>  
> +static inline int
> +ctnetlink_change_protoinfo(struct ip_conntrack *ct, struct nfattr *cda[])
> +{
> +	struct nfattr *tb[CTA_PROTOINFO_MAX], *attr = cda[CTA_PROTOINFO-1];
> +	struct ip_conntrack_protocol *proto;
> +	u_int16_t npt = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum;
> +	int err;
> +
> +	if (nfattr_parse_nested(tb, CTA_PROTOINFO_MAX, attr) < 0)
> +		goto nfattr_failure;

nfattr_failure is usually used when there isn't enough space in
the skb, not when parsing. So I would prefer to only use it in
functions putting data in the skb to avoid confusion. Also
-1 (-EPERM) isn't a suitable error to return to userspace.

> +
> +	proto = ip_conntrack_proto_find_get(npt);
> +	if (!proto)
> +		return -EINVAL;
> +
> +	if (proto->from_nfattr) {
> +		err = proto->from_nfattr(tb, ct);
> +		if (err < 0) {
> +			ip_conntrack_proto_put(proto);
> +			return -EINVAL;
> +		}
> +	}
> +	ip_conntrack_proto_put(proto);
> +
> +	return 0;

This would be simpler and would propagate the error properly if you
would write it like this:

...
if (proto->from_nfattr)
	err = proto->from_nfattr(tb, ct);
ip_conntrack_proto_put(proto);

return err;

Re: [PATCH 3/3] Implement private protocol info changing

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

On Sun, Sep 04, 2005 at 08:02:22PM +0200, Patrick McHardy wrote:
> Pablo Neira wrote:
> >This patch add support to change the state of the private protocol 
> >information via conntrack_netlink.
> 
> The patches look fine to me, besides the two issues mentioned below.
> I have a couple of fixes which conflict with them though, please
> resend after they hit Linus's tree.

I also wasn't yet sure whether the current nesting is the best possible
solution.  Sorry, was too busy with non-netfilter stuff recently so I
couldn't comment on that.  I'll try to comment on it later today.
-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]