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

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: <50F2C860.1060602@mojatatu.com>
Date:	Sun, 13 Jan 2013 09:44:48 -0500
From:	Jamal Hadi Salim <jhs@...atatu.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Benjamin LaHaise <bcrl@...ck.org>,
	David Miller <davem@...emloft.net>, socketcan@...tkopp.net,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] pkt_sched: namespace aware ifb

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
=====

--
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