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
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to