On Thu, 28 Sep 2006, Venkat Yekkirala wrote:

+		if (connsecmark)
+			if (*connsecmark != skb->secmark) {
 				*connsecmark = skb->secmark;
+			}

Please remove the braces if re-submitting.

+printk(KERN_ERR "IN HOOK (%d) (%u) (%u)\n", hooknum, skb->secmark, *psecmark);\

Please remove debugging (here and elsewhere).

+		/* Set secmark on inbound and filter it on outbound */
+		if ((target->family == AF_INET &&
+			(hooknum == NF_IP_POST_ROUTING ||
+			 hooknum == NF_IP_LOCAL_OUT ||
+			 hooknum == NF_IP_FORWARD)) ||
+		    (target->family == AF_INET6 &&
+			(hooknum == NF_IP6_POST_ROUTING ||
+			 hooknum == NF_IP6_LOCAL_OUT ||
+			 hooknum == NF_IP6_FORWARD))) {

I think this should be a separate helper function, so the logic can be 
changed/evaluated in isolation (preferred, but not a blocker).

+		secmark_save(skb, hooknum, target);

It seems that the target parameter is not needed.

+		return secmark_restore(skb, hooknum, in, target);

Please pass a family parameter instead of target.



-- 
James Morris
[off-list ref]