Re: [PATCH net-next] pkt_sched: namespace aware ifb

[Jamal Hadi Salim <jhs@mojatatu.com>]

On 13-01-13 12:49 AM, Eric Dumazet wrote:

> Could you elaborate on what could be the problem ?
>
> We hold the RTNL, so I dont think another process could possibly call
> tcf_mirred_init()
>

Eric, the point probably Ben was trying to make is not about 
synchronizing rather about which namespace has the right to that action 
config. Your change is correct for the common use of actions
but does not fix the larger picture.

At the moment a dev is owned by a specific namespace; that owns a tc 
filter that in turn owns an action. So no problem with the change you 
make if all configuration follows those rules i.e something along the 
lines of:
===
tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \
match ip dst 10.0.0.229/32 flowid 1:10 \
action mirred egress redirect dev ifb0
=====

I would say most people use the above syntax.
However, there is another way to configure actions so they can be 
shared[1], example control syntax:
----
tc actions add \
action police rate 1kbit burst 90k drop index 3 \
action mirred egress mirror dev eth0 index 5
----

In such a case, the "tc actions" netlink path may be
entered from a different namespace than the one that is
using it. Then current->nsproxy->net_ns is no longer correct.

To correct this, i think what Ben points out in passing the
init() the correct namespace seem like the way to go. Feel free
to make that change - otherwise i will get to it and fix it.

cheers,
jamal

[1]
You can then have multiple filters use this action like so:
===
tc filter add dev eth0 parent 1: protocol ip prio 1 u32 \
match ip dst 10.0.0.229/32 flowid 1:10 \
action police index 3
#
tc filter add dev eth0 parent ffff: protocol ip prio 6 u32 \
match ip src 10.0.0.21/32 flowid 1:16 \
action police index 3 action mirred egress mirror dev eth0
=====