netdev - switchdev fib offload issues

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-next>] [day] [month] [year] [list]

Date:	Mon, 18 Apr 2016 17:47:57 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	netdev@...r.kernel.org
Cc:	davem@...emloft.net, idosch@...lanox.com, eladr@...lanox.com,
	yotamg@...lanox.com, ogerlitz@...lanox.com,
	roopa@...ulusnetworks.com, nikolay@...ulusnetworks.com,
	jhs@...atatu.com, john.fastabend@...il.com, rami.rosen@...el.com,
	gospo@...ulusnetworks.com, stephen@...workplumber.org,
	sfeldma@...il.com, dsa@...ulusnetworks.com, f.fainelli@...il.com,
	andrew@...n.ch, vivien.didelot@...oirfairelinux.com, tgraf@...g.ch,
	aduyck@...antis.com
Subject: switchdev fib offload issues

Hi all.

The current situation of fib offloading is not good, I believe we need
to make some changes, therefore I'm writing this email. Please read,
think and comment.

Currently what we have is that for every fib entry inserted into a table,
there is a call to switchdev:
 fib_table_insert->switchdev_fib_ipv4_add
Driver then pushes fib entry down to HW. So far good.

However, if for any reason the switchdev add operation fails, there is an
abort function called (switchdev_fib_ipv4_abort). This function does two
things which are both unfortunate in many usecases:
1) evicts all fib entries from HW leaving all processing done in kernel
    - For Spectrum ASIC this means that all traffic running at 100G between
      all ports is immediately downgraded to ~1-3Gbits
    - Also this happens silently, user knows nothing about anything went wrong,
      only forwarding performance suddenly sucks.

2) sets net->ipv4.fib_offload_disabled = true
    - That results in no other fib entry being offloaded, forever,
      until net is removed and added again, machine reboot is required
      in case if init_ns

These 2 issues makes fib offload completely unusable. So I propose
to start thinking about fixing this.

I believe that although the current behaviour might be good for default,
user should be able to change it by setting a different policy. This
policy will allow to propagate offload error to user.

Note that user already has to handle fib add errors which are independent
on particular fib entry. That is a case of insufficient memory (-ENOBUFS).
In fact, when offload fails, that is most likely also due to insufficient
resources in HW.

Proposed solutions (ideas):
1) per-netns. Add a procfs file:
	/proc/sys/net/ipv4/route/fib_offload_error_policy
	  with values: "evict" - default, current behaviour
                       "fail" - propagate offload error to user
	The policy value would be stored in struct net.

2) per-VRF/table
	When user creates a VRF master, he specifies a table ID
	this VRF is going to use. I propose to extend this so
	he can pass a policy ("evict"/"fail").
	The policy value would be stored in struct fib_table or
	struct fib6_table. The problem is that vfr only saves
	table ID, allocates dst but does not actually create
	table. That might be created later. But I think this
	could be resolved.

3) per-VFR/master_netdev
	In this case, the policy would be also set during
	the creation of VFR master. From user perspective,
	this looks same as 2)
	The policy value would be stored in struct net_vrf (vrf private).

Thoughts?

Thanks!

Jiri