On Sunday 2008-10-05 12:26, Patrick McHardy wrote:
> Jan Engelhardt wrote:
>> On Sunday 2008-10-05 11:12, Patrick McHardy wrote:
>> > Thanks. As an added explanation - the benchmarking I did for nftables
>> > indicated that we're somewhere between 50 and 110 cycles for a "usual"
>> > rule on my x2. So its really easy to degrade performance significantly
>> > by just requiring a few more cycles. The upside is that it works in both
>> > directions :)
>>
>> Hm there is actually a reason I did it this way.
>>
>> I did not want to make the struct xt_match_param constant that
>> extensions receive, so they can tamper with the arguments just like
>> they were able to before. (Actually, just a single outoftree
>> extension does this right now.)
>
> The devices are const currently, so its no change to the current
> situation, is it?
Mh, want to take a C course again? ;-) There is a difference between
const struct net_device *in; vs.
struct net_device *const in;
>> Constructing the xt_match_param for example in ipt_do_table() instead
>> of do_match() would mean that iff some extension trashed, say,
>> par->in, then all future extensions would get that new value, which
>> is of course not what we wanted.
>
> It isn't what we want? Why does it change the global value then?
For example, ebt_snat does this:
bool ebt_snat_tg_check(table, e, target, data, hookmask)
{
if (hookmask & base_chain_bit && tmp == EBT_RETURN)
return false;
hookmask &= ~base_chain_bit;
if (hookmask & yada)
whatever;
else
foo;
}
hookmask was coiped to the function by use of the stack (the usual stuff),
but with the argument consolidation, it now reads like:
par->hookmask &= ~base_chain_bit;
which means that the caller will see the altered hook mask. This is why
the struct is thrown away everytime.
>> Requesting verdict from you. :)
>> Make it const so that extensions don't tamper with it?
>
> Preferrably, yes.
>
Perfect.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
