On 8/5/19 9:20 AM, Jiri Pirko wrote: > Mon, Aug 05, 2019 at 04:51:22PM CEST, dsah...@gmail.com wrote: >> On 8/5/19 8:49 AM, Jiri Pirko wrote: >>>> Your commit 5fc494225c1eb81309cc4c91f183cd30e4edb674 changed that from a >>>> per-namepace accounting to all namespaces managed by a single devlink >>>> instance in init_net - which is completely wrong. >>> No. Not "all namespaces". Only the one where the devlink is. And that is >>> always init_net, until this patchset. >>> >>> >> >> Jiri: your change to fib.c does not take into account namespace when >> doing rules and routes accounting. you broke it. fix it. > > What do you mean by "account namespace"? It's a device resource, why to > tight it with namespace? What if you have 2 netdevsim-devlink instances > in one namespace? Why the setting should be per-namespace? >
Jiri: Here's an example of how your 5.2 change to netdevsim broke the resource controller: Create a netdevsim device: $ modprobe netdevsim $ echo "0 1" > /sys/bus/netdevsim/new_device Get the current number of IPv4 routes: $ n=$(ip -4 ro ls table all | wc -l) $ echo $n 13 Prevent any more from being added. This limit should apply solely to this namespace, init_net: $ devlink resource set netdevsim/netdevsim0 path /IPv4/fib size $n $ devlink dev reload netdevsim/netdevsim0 Error: netdevsim: New size is less than current occupancy. devlink answers: Invalid argument So that is the first breakage: accounting is off - maybe. Note there are no other visible namespaces, but who knows what systemd or other processes are doing with namespaces. Perhaps this accounting is another example of your changes not properly handling namespaces: $ devlink resource show netdevsim/netdevsim0 netdevsim/netdevsim0: name IPv4 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none resources: name fib size 13 occ 17 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none name fib-rules size unlimited occ 6 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none name IPv6 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none resources: name fib size unlimited occ 10 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none name fib-rules size unlimited occ 4 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none So the occupancy does not match the tables for init_net. Reset the max to 17, the current occupancy: $ devlink resource set netdevsim/netdevsim0 path /IPv4/fib size 17 $ devlink dev reload netdevsim/netdevsim0 $ devlink resource show netdevsim/netdevsim0 netdevsim/netdevsim0: name IPv4 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none resources: name fib size 17 occ 17 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none name fib-rules size unlimited occ 6 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none name IPv6 size unlimited unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none resources: name fib size unlimited occ 10 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none name fib-rules size unlimited occ 4 unit entry size_min 0 size_max unlimited size_gran 1 dpipe_tables none Create a new namespace, bring up lo which attempts to add more route entries: $ unshare -n $ ip li set lo up If you list routes you see the lo routes failed to installed because of the limits, but it is a silent failure. Try to add a new route and you see the cross namespace accounting now: $ ip ro add 192.168.1.0/24 dev lo Error: netdevsim: Exceeded number of supported fib entries. Contrast that behavior with 5.1 and you see the new namespaces have no bearing on accounting in init_net and limits in init_net do not affect other namespaces. That behavior needs to be restored in 5.2 and 5.3.