Re: xt_MARK useless line

[Patrick McHardy <kaber@trash.net>]

Philip Craig wrote:
> Jan Engelhardt wrote:
> 
>>On Jan 7 2007 15:15, Harald Welte wrote:
>>
>>>On Fri, Dec 29, 2006 at 05:54:07PM +0100, Jan Engelhardt wrote:
>>>
>>>>Hi,
>>>>
>>>>in linux-2.6.20-rc2/net/netfilter/xt_MARK.c one can find
>>>>
>>>>        if((*pskb)->mark != markinfo->mark)
>>>>                (*pskb)->mark = markinfo->mark;
>>>>
>>>>would not it be simpler just to set the mark? Or are comparison-jumps 
>>>>like these 'cheaper' than assignments? (I don't think so.)
>>>
>>>I don't really know. I think the intention might have been to prevent
>>>writes to shared data structures, in order to reduce cache ping-pong on
>>>SMP.  However, unlesss some really complex and rare event happens, this
>>>skb will never be processed asynchronously and thus not be used on any
>>>other CPU.
>>
>>In other words, your conclusion is ... - (R)emove/(K)eep?
>>(For the record, it's anywhere where something is set, that is, MARK, CONNMARK,
>>SECMARK, TOS, ECN, and so on, everywhere you have XT_CONTINUE)
> 
> 
> Some of these may be leftover from when NFC_ALTERED existed?

That sounds like the most likely explanation. Looking at 2.4:

        if((*pskb)->nfmark != markinfo->mark) {
                (*pskb)->nfmark = markinfo->mark;
                (*pskb)->nfcache |= NFC_ALTERED;

        }

I vote for removing this (also in CONNMARK, CONNSECMARK, ...).