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

[jamal <hadi@cyberus.ca>]

Hi,


Please CC me in your responses (the way i do when i respond to you),
that way my filters prioritize your email. 

On Sat, 2008-12-01 at 15:56 -0200, Dzianis Kahanovich wrote:
> I in doubts only about "action continue".
> To "and/or" behaviour one of best usage are (example):

I dont think you should be touching the action part at all primarily
because actions can set the mark after classification. 
The action code (not the default) should be the override. IOW, if i
specify a ipt mark of some value i would expect that value to be what
goes into the network stack and not the default value you want. Same if
i had a series of actions which override each others settings of mark.

When we have a metadata action, we can remove the setting of tcindex
in the action OK result case (for now it doesnt harm).

In other words, just set the #ifndef action to set both the tcindex and
mark to some policy;

> # set bit 2 of mark to 0 (mark&0xfd|0) and continue
> tc filter add ... prio 1 ... flowid fd:0 action continue
> # continue
> tc filter add ... prio 2 ...
> 
> - in current ingress_enqueue() code IMHO "case TC_ACT_OK:" will not reached 
> for action continue. I use old (mark=...) solution only by this.
> 
> I think, "skb->mark = (skb->mark&(res.classid>>16))|TC_H_MIN(res.classid);" 
> must be in the end of ingress_enqueue() before "return result". And not 
> depended to "NET_CLS_ACT". But while not test it.
> Or this:
> ---
> #ifdef CONFIG_NET_SCH_INGRESS_TC2MARK
> #ifdef CONFIG_NET_CLS_ACT
> 	skb->mark = (skb->mark&(res.classid>>16))|TC_H_MIN(res.classid);
> #else
> 	skb->mark = res.classid;
> #endif
> #endif

Please refer to what i said above; if what i said still doesnt make
sense i can create (the simple) patch.

cheers,
jamal