netdev - Re: [PATCH 2.6.23+] ingress classify to [nf]mark

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1200487509.4457.33.camel@localhost>
Date:	Wed, 16 Jan 2008 07:45:09 -0500
From:	jamal <hadi@...erus.ca>
To:	mahatma@...by
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH 2.6.23+] ingress classify to [nf]mark

On Mon, 2008-14-01 at 20:20 -0200, Dzianis Kahanovich wrote:
> jamal wrote:
[..] 

> > Did that make sense?
>
> After current "#endif" - may be.

I am afraid that would be counter to expected behavior. 
Default is meant to apply when no value has been defined. Mark of 0 for
example doesnt mean "default". Let me demonstrate with the ifdefs again
with some arbitrary example:

-----------------
#ifdef CONFIG_NET_CLS_ACT
..classify ...
    .. action 1 sets mark to 0x11111
    .. action 2 checks some state and conditionally let action 3 execute
    .. action 3 sets mark to 0

if OK is returned set tc_index based on classid

#else // no actions compiled
..classify
.... jamal suggests: set default mark and tc_index for ingress here
#endif

// mahatma wants to set default for mark and tcindex here 
// so it works for both actions and none-action code
------------------------

Lets look at the case of actions compiled in:
I have defined my policies (in user space) so that the mark can be set
to either 0 or 0x1111 depending on some runtime state. 
Your default (kernel) code is now going to overide my policy - which is
bad. Even in the case of OK being returned, it is wrong to set tc_index;
unfortunately, we dont have an action that can set tc_index today; if we
did, we would need to remove that setting.

You other intent was to set the value of mark based on the value of
classid. You _can do that today already_ with no changes via a policy in
user space. You suggested to do an ifdef so you wont have to type in the
line which says how to mark, and i said that was a bad idea (we need
less ifdefs not more). 

For the case of no actions compiled in:
nothing can write into the values of either tcindex or mark after
classification (on ingress), so it is ok to override. If you did this
for egress as well, that would be wrong because it is expected that some
qdiscs may set or utilize these metadatum.

I am not sure if it made more sense this time?

> What "result" are with:
> 1) no filters?
> 2) 1 filter only, with "action continue"?

Please refer to above verbosity and see if it all makes better sense.

cheers,
jamal

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html