| Message ID | 20201104133040.1125369-1-idosch@idosch.org |
|---|---|
| Headers | show |
| Series | nexthop: Add support for nexthop objects offload | expand |
On Wed, 4 Nov 2020 15:30:25 +0200 Ido Schimmel wrote: > + if (info->is_grp) > + return nh_notifier_grp_info_init(info, nh); > + else > + return nh_notifier_single_info_init(info, nh); nit: please try to avoid unnecessary else branches, it obviously doesn't matter but people send patches to refactor this :(
On Wed, 4 Nov 2020 15:30:22 +0200 Ido Schimmel wrote: > From: Ido Schimmel <idosch@nvidia.com> > > This patch set adds support for nexthop objects offload with a dummy > implementation over netdevsim. mlxsw support will be added later. > > The general idea is very similar to route offload in that notifications > are sent whenever nexthop objects are changed. A listener can veto the > change and the error will be communicated to user space with extack. > > To keep listeners as simple as possible, they not only receive > notifications for the nexthop object that is changed, but also for all > the other objects affected by this change. For example, when a single > nexthop is replaced, a replace notification is sent for the single > nexthop, but also for all the nexthop groups this nexthop is member in. > This relieves listeners from the need to track such dependencies. > > To simplify things further for listeners, the notification info does not > contain the raw nexthop data structures (e.g., 'struct nexthop'), but > less complex data structures into which the raw data structures are > parsed into. Applied, thank you! BTW no need to follow up on my else-after-return comment, just something to keep in mind.
On Fri, Nov 06, 2020 at 11:31:59AM -0800, Jakub Kicinski wrote: > On Wed, 4 Nov 2020 15:30:22 +0200 Ido Schimmel wrote: > > From: Ido Schimmel <idosch@nvidia.com> > > > > This patch set adds support for nexthop objects offload with a dummy > > implementation over netdevsim. mlxsw support will be added later. > > > > The general idea is very similar to route offload in that notifications > > are sent whenever nexthop objects are changed. A listener can veto the > > change and the error will be communicated to user space with extack. > > > > To keep listeners as simple as possible, they not only receive > > notifications for the nexthop object that is changed, but also for all > > the other objects affected by this change. For example, when a single > > nexthop is replaced, a replace notification is sent for the single > > nexthop, but also for all the nexthop groups this nexthop is member in. > > This relieves listeners from the need to track such dependencies. > > > > To simplify things further for listeners, the notification info does not > > contain the raw nexthop data structures (e.g., 'struct nexthop'), but > > less complex data structures into which the raw data structures are > > parsed into. > > Applied, thank you! Great, thank you. And thanks David for the awesome work on the nexthop infrastructure. > > BTW no need to follow up on my else-after-return comment, > just something to keep in mind. Ack
On 11/9/20 8:41 AM, Ido Schimmel wrote: > Great, thank you. And thanks David for the awesome work on the nexthop > infrastructure. thanks. Looking forward to the next round of patches for h/w offload.
From: Ido Schimmel <idosch@nvidia.com> This patch set adds support for nexthop objects offload with a dummy implementation over netdevsim. mlxsw support will be added later. The general idea is very similar to route offload in that notifications are sent whenever nexthop objects are changed. A listener can veto the change and the error will be communicated to user space with extack. To keep listeners as simple as possible, they not only receive notifications for the nexthop object that is changed, but also for all the other objects affected by this change. For example, when a single nexthop is replaced, a replace notification is sent for the single nexthop, but also for all the nexthop groups this nexthop is member in. This relieves listeners from the need to track such dependencies. To simplify things further for listeners, the notification info does not contain the raw nexthop data structures (e.g., 'struct nexthop'), but less complex data structures into which the raw data structures are parsed into. Tested with a new selftest over netdevsim and with fib_nexthops.sh: Tests passed: 164 Tests failed: 0 Patch set overview: Patches #1-#4 introduce the aforementioned data structures and convert existing listeners (i.e., the VXLAN driver) to use them. Patches #5-#6 add a new RTNH_F_TRAP flag and the ability to set it and RTNH_F_OFFLOAD on nexthops. This flag is used by netdevsim for testing purposes and will also be used by mlxsw. These flags are consistent with the existing RTM_F_OFFLOAD and RTM_F_TRAP flags. Patches #7-#14 gradually add the new nexthop notifications. Patches #15-#18 add a dummy implementation for nexthop offload over netdevsim and a selftest to exercise both good and bad flows. Changes since RFC [1]: Patch #1: s/is_encap/has_encap/ Patch #3: Add a blank line in __nh_notifier_single_info_init() Patch #5: Reword commit message Patch #6: s/nexthop_hw_flags_set/nexthop_set_hw_flags/ Patch #7: Reword commit message Patch #11: Allocate extack on the stack Follow-up patch sets: selftests: forwarding: Add nexthop objects tests mlxsw: Preparations for nexthop objects support - part 1/2 mlxsw: Preparations for nexthop objects support - part 2/2 mlxsw: Add support for nexthop objects mlxsw: Add support for blackhole nexthops mlxsw: Update adjacency index more efficiently [1] https://lore.kernel.org/netdev/20200908091037.2709823-1-idosch@idosch.org/ Ido Schimmel (18): nexthop: Add nexthop notification data structures nexthop: Pass extack to nexthop notifier nexthop: Prepare new notification info nexthop: vxlan: Convert to new notification info rtnetlink: Add RTNH_F_TRAP flag nexthop: Allow setting "offload" and "trap" indications on nexthops nexthop: Emit a notification when a nexthop is added nexthop: Emit a notification when a nexthop group is replaced nexthop: Emit a notification when a single nexthop is replaced nexthop: Emit a notification when a nexthop group is modified nexthop: Emit a notification when a nexthop group is reduced nexthop: Pass extack to register_nexthop_notifier() nexthop: Replay nexthops when registering a notifier nexthop: Remove in-kernel route notifications when nexthop changes netdevsim: Add devlink resource for nexthops netdevsim: Add dummy implementation for nexthop offload netdevsim: Allow programming routes with nexthop objects selftests: netdevsim: Add test for nexthop offload API .../networking/devlink/netdevsim.rst | 3 +- drivers/net/netdevsim/dev.c | 6 + drivers/net/netdevsim/fib.c | 265 ++++++++++- drivers/net/netdevsim/netdevsim.h | 1 + drivers/net/vxlan.c | 12 +- include/net/nexthop.h | 42 +- include/uapi/linux/rtnetlink.h | 6 +- net/ipv4/fib_semantics.c | 2 + net/ipv4/fib_trie.c | 9 - net/ipv4/nexthop.c | 255 +++++++++- net/ipv6/route.c | 5 - .../drivers/net/netdevsim/nexthop.sh | 436 ++++++++++++++++++ 12 files changed, 995 insertions(+), 47 deletions(-) create mode 100755 tools/testing/selftests/drivers/net/netdevsim/nexthop.sh