Fix handling IPv4 routes referencing a nexthop via its id by replacing
calls to fib_info_nh() with fib_info_nhc().
Trying to add an IPv4 route referencing a nextop via nhid:
$ ip link set up swp5
$ ip a a 10.0.0.1/24 dev swp5
$ ip nexthop add dev swp5 id 20 via 10.0.0.2
$ ip route add 10.0.1.0/24 nhid 20
triggers warnings when trying to handle the route:
[ 528.805763] ------------[ cut here ]------------
[ 528.810437] WARNING: CPU: 3 PID: 53 at include/net/nexthop.h:468 __prestera_fi_is_direct+0x2c/0x68 [prestera]
[ 528.820434] Modules linked in: prestera_pci act_gact act_police sch_ingress cls_u32 cls_flower prestera arm64_delta_tn48m_dn_led(O) arm64_delta_tn48m_dn_cpld(O) [last unloaded: prestera_pci]
[ 528.837485] CPU: 3 PID: 53 Comm: kworker/u8:3 Tainted: G O 6.4.5 #1
[ 528.845178] Hardware name: delta,tn48m-dn (DT)
[ 528.849641] Workqueue: prestera_ordered __prestera_router_fib_event_work [prestera]
[ 528.857352] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 528.864347] pc : __prestera_fi_is_direct+0x2c/0x68 [prestera]
[ 528.870135] lr : prestera_k_arb_fib_evt+0xb20/0xd50 [prestera]
[ 528.876007] sp : ffff80000b20bc90
[ 528.879336] x29: ffff80000b20bc90 x28: 0000000000000000 x27: ffff0001374d3a48
[ 528.886510] x26: ffff000105604000 x25: ffff000134af8a28 x24: ffff0001374d3800
[ 528.893683] x23: ffff000101c89148 x22: ffff000101c89000 x21: ffff000101c89200
[ 528.900855] x20: ffff00013641fda0 x19: ffff800009d01088 x18: 0000000000000059
[ 528.908027] x17: 0000000000000277 x16: 0000000000000000 x15: 0000000000000000
[ 528.915198] x14: 0000000000000003 x13: 00000000000fe400 x12: 0000000000000000
[ 528.922371] x11: 0000000000000002 x10: 0000000000000aa0 x9 : ffff8000013d2020
[ 528.929543] x8 : 0000000000000018 x7 : 000000007b1703f8 x6 : 000000001ca72f86
[ 528.936715] x5 : 0000000033399ea7 x4 : 0000000000000000 x3 : ffff0001374d3acc
[ 528.943886] x2 : 0000000000000000 x1 : ffff00010200de00 x0 : ffff000134ae3f80
[ 528.951058] Call trace:
[ 528.953516] __prestera_fi_is_direct+0x2c/0x68 [prestera]
[ 528.958952] __prestera_router_fib_event_work+0x100/0x158 [prestera]
[ 528.965348] process_one_work+0x208/0x488
[ 528.969387] worker_thread+0x4c/0x430
[ 528.973068] kthread+0x120/0x138
[ 528.976313] ret_from_fork+0x10/0x20
[ 528.979909] ---[ end trace 0000000000000000 ]---
[ 528.984998] ------------[ cut here ]------------
[ 528.989645] WARNING: CPU: 3 PID: 53 at include/net/nexthop.h:468 __prestera_fi_is_direct+0x2c/0x68 [prestera]
[ 528.999628] Modules linked in: prestera_pci act_gact act_police sch_ingress cls_u32 cls_flower prestera arm64_delta_tn48m_dn_led(O) arm64_delta_tn48m_dn_cpld(O) [last unloaded: prestera_pci]
[ 529.016676] CPU: 3 PID: 53 Comm: kworker/u8:3 Tainted: G W O 6.4.5 #1
[ 529.024368] Hardware name: delta,tn48m-dn (DT)
[ 529.028830] Workqueue: prestera_ordered __prestera_router_fib_event_work [prestera]
[ 529.036539] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 529.043533] pc : __prestera_fi_is_direct+0x2c/0x68 [prestera]
[ 529.049318] lr : __prestera_k_arb_fc_apply+0x280/0x2f8 [prestera]
[ 529.055452] sp : ffff80000b20bc60
[ 529.058781] x29: ffff80000b20bc60 x28: 0000000000000000 x27: ffff0001374d3a48
[ 529.065953] x26: ffff000105604000 x25: ffff000134af8a28 x24: ffff0001374d3800
[ 529.073126] x23: ffff000101c89148 x22: ffff000101c89148 x21: ffff00013641fda0
[ 529.080299] x20: ffff000101c89000 x19: ffff000101c89020 x18: 0000000000000059
[ 529.087471] x17: 0000000000000277 x16: 0000000000000000 x15: 0000000000000000
[ 529.094642] x14: 0000000000000003 x13: 00000000000fe400 x12: 0000000000000000
[ 529.101814] x11: 0000000000000002 x10: 0000000000000aa0 x9 : ffff8000013cee80
[ 529.108985] x8 : 0000000000000018 x7 : 000000007b1703f8 x6 : 0000000000000018
[ 529.116157] x5 : 00000000d3497eb6 x4 : ffff000105604081 x3 : 000000008e979557
[ 529.123329] x2 : 0000000000000000 x1 : ffff00010200de00 x0 : ffff000134ae3f80
[ 529.130501] Call trace:
[ 529.132958] __prestera_fi_is_direct+0x2c/0x68 [prestera]
[ 529.138394] prestera_k_arb_fib_evt+0x6b8/0xd50 [prestera]
[ 529.143918] __prestera_router_fib_event_work+0x100/0x158 [prestera]
[ 529.150313] process_one_work+0x208/0x488
[ 529.154348] worker_thread+0x4c/0x430
[ 529.158030] kthread+0x120/0x138
[ 529.161274] ret_from_fork+0x10/0x20
[ 529.164867] ---[ end trace 0000000000000000 ]---
and results in a non offloaded route:
$ ip route
10.0.0.0/24 dev swp5 proto kernel scope link src 10.0.0.1 rt_trap
10.0.1.0/24 nhid 20 via 10.0.0.2 dev swp5 rt_trap
When creating a route referencing a nexthop via its ID, the nexthop will
be stored in a separate nh pointer instead of the array of nexthops in
the fib_info struct. This causes issues since fib_info_nh() only handles
the nexthops array, but not the separate nh pointer, and will loudly
WARN about it.
In contrast fib_info_nhc() handles both, but returns a fib_nh_common
pointer instead of a fib_nh pointer. Luckily we only ever access fields
from the fib_nh_common parts, so we can just replace all instances of
fib_info_nh() with fib_info_nhc() and access the fields via their
fib_nh_common names.
This allows handling IPv4 routes with an external nexthop, and they now
get offloaded as expected:
$ ip route
10.0.0.0/24 dev swp5 proto kernel scope link src 10.0.0.1 rt_trap
10.0.1.0/24 nhid 20 via 10.0.0.2 dev swp5 offload rt_offload
Fixes: 396b80cb5cc8 ("net: marvell: prestera: Add neighbour cache accounting")
Signed-off-by: Jonas Gorski <[email protected]>
---
.../ethernet/marvell/prestera/prestera_router.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c b/drivers/net/ethernet/marvell/prestera/prestera_router.c
index a9a1028cb17b..de317179a7dc 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
@@ -166,11 +166,11 @@ prestera_util_neigh2nc_key(struct prestera_switch *sw, struct neighbour *n,
static bool __prestera_fi_is_direct(struct fib_info *fi)
{
- struct fib_nh *fib_nh;
+ struct fib_nh_common *fib_nhc;
if (fib_info_num_path(fi) == 1) {
- fib_nh = fib_info_nh(fi, 0);
- if (fib_nh->fib_nh_gw_family == AF_UNSPEC)
+ fib_nhc = fib_info_nhc(fi, 0);
+ if (fib_nhc->nhc_gw_family == AF_UNSPEC)
return true;
}
@@ -261,7 +261,7 @@ static bool
__prestera_util_kern_n_is_reachable_v4(u32 tb_id, __be32 *addr,
struct net_device *dev)
{
- struct fib_nh *fib_nh;
+ struct fib_nh_common *fib_nhc;
struct fib_result res;
bool reachable;
@@ -269,8 +269,8 @@ __prestera_util_kern_n_is_reachable_v4(u32 tb_id, __be32 *addr,
if (!prestera_util_kern_get_route(&res, tb_id, addr))
if (prestera_fi_is_direct(res.fi)) {
- fib_nh = fib_info_nh(res.fi, 0);
- if (dev == fib_nh->fib_nh_dev)
+ fib_nhc = fib_info_nhc(res.fi, 0);
+ if (dev == fib_nhc->nhc_dev)
reachable = true;
}
@@ -324,7 +324,7 @@ prestera_kern_fib_info_nhc(struct fib_notifier_info *info, int n)
if (info->family == AF_INET) {
fen4_info = container_of(info, struct fib_entry_notifier_info,
info);
- return &fib_info_nh(fen4_info->fi, n)->nh_common;
+ return fib_info_nhc(fen4_info->fi, n);
} else if (info->family == AF_INET6) {
fen6_info = container_of(info, struct fib6_entry_notifier_info,
info);
--
2.41.0
--
BISDN GmbH
K?rnerstra?e 7-10
10785 Berlin
Germany
Phone:
+49-30-6108-1-6100
Managing Directors:?
Dr.-Ing. Hagen Woesner, Andreas
K?psel
Commercial register:?
Amtsgericht Berlin-Charlottenburg HRB 141569
B
VAT ID No:?DE283257294
> -----Original Message-----
> From: Jonas Gorski <[email protected]>
> Sent: Friday, August 4, 2023 1:12 PM
> To: Elad Nachman <[email protected]>; Taras Chornyi
> <[email protected]>; David S. Miller <[email protected]>; Eric
> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
> Paolo Abeni <[email protected]>; Yevhen Orlov
> <[email protected]>; Oleksandr Mazur
> <[email protected]>
> Cc: Taras Chornyi [C] <[email protected]>; [email protected];
> [email protected]
> Subject: [EXT] [PATCH] net: marvell: prestera: fix handling IPv4 routes with
> nhid
>
> External Email
>
> ----------------------------------------------------------------------
> Fix handling IPv4 routes referencing a nexthop via its id by replacing calls to
> fib_info_nh() with fib_info_nhc().
>
> Trying to add an IPv4 route referencing a nextop via nhid:
>
> $ ip link set up swp5
> $ ip a a 10.0.0.1/24 dev swp5
> $ ip nexthop add dev swp5 id 20 via 10.0.0.2
> $ ip route add 10.0.1.0/24 nhid 20
>
> triggers warnings when trying to handle the route:
>
> [ 528.805763] ------------[ cut here ]------------ [ 528.810437] WARNING: CPU: 3
> PID: 53 at include/net/nexthop.h:468 __prestera_fi_is_direct+0x2c/0x68
> [prestera] [ 528.820434] Modules linked in: prestera_pci act_gact act_police
> sch_ingress cls_u32 cls_flower prestera arm64_delta_tn48m_dn_led(O)
> arm64_delta_tn48m_dn_cpld(O) [last unloaded: prestera_pci]
> [ 528.837485] CPU: 3 PID: 53 Comm: kworker/u8:3 Tainted: G O
> 6.4.5 #1
> [ 528.845178] Hardware name: delta,tn48m-dn (DT) [ 528.849641]
> Workqueue: prestera_ordered __prestera_router_fib_event_work [prestera]
> [ 528.857352] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
> BTYPE=--) [ 528.864347] pc : __prestera_fi_is_direct+0x2c/0x68 [prestera] [
> 528.870135] lr : prestera_k_arb_fib_evt+0xb20/0xd50 [prestera] [
> 528.876007] sp : ffff80000b20bc90 [ 528.879336] x29: ffff80000b20bc90 x28:
> 0000000000000000 x27: ffff0001374d3a48 [ 528.886510] x26:
> ffff000105604000 x25: ffff000134af8a28 x24: ffff0001374d3800 [
> 528.893683] x23: ffff000101c89148 x22: ffff000101c89000 x21:
> ffff000101c89200 [ 528.900855] x20: ffff00013641fda0 x19:
> ffff800009d01088 x18: 0000000000000059 [ 528.908027] x17:
> 0000000000000277 x16: 0000000000000000 x15: 0000000000000000 [
> 528.915198] x14: 0000000000000003 x13: 00000000000fe400 x12:
> 0000000000000000 [ 528.922371] x11: 0000000000000002 x10:
> 0000000000000aa0 x9 : ffff8000013d2020 [ 528.929543] x8 :
> 0000000000000018 x7 : 000000007b1703f8 x6 : 000000001ca72f86 [
> 528.936715] x5 : 0000000033399ea7 x4 : 0000000000000000 x3 :
> ffff0001374d3acc [ 528.943886] x2 : 0000000000000000 x1 :
> ffff00010200de00 x0 : ffff000134ae3f80 [ 528.951058] Call trace:
> [ 528.953516] __prestera_fi_is_direct+0x2c/0x68 [prestera] [ 528.958952]
> __prestera_router_fib_event_work+0x100/0x158 [prestera] [ 528.965348]
> process_one_work+0x208/0x488 [ 528.969387] worker_thread+0x4c/0x430
> [ 528.973068] kthread+0x120/0x138 [ 528.976313]
> ret_from_fork+0x10/0x20 [ 528.979909] ---[ end trace 0000000000000000 ]--
> - [ 528.984998] ------------[ cut here ]------------ [ 528.989645] WARNING: CPU:
> 3 PID: 53 at include/net/nexthop.h:468 __prestera_fi_is_direct+0x2c/0x68
> [prestera] [ 528.999628] Modules linked in: prestera_pci act_gact act_police
> sch_ingress cls_u32 cls_flower prestera arm64_delta_tn48m_dn_led(O)
> arm64_delta_tn48m_dn_cpld(O) [last unloaded: prestera_pci]
> [ 529.016676] CPU: 3 PID: 53 Comm: kworker/u8:3 Tainted: G W O
> 6.4.5 #1
> [ 529.024368] Hardware name: delta,tn48m-dn (DT) [ 529.028830]
> Workqueue: prestera_ordered __prestera_router_fib_event_work [prestera]
> [ 529.036539] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
> BTYPE=--) [ 529.043533] pc : __prestera_fi_is_direct+0x2c/0x68 [prestera] [
> 529.049318] lr : __prestera_k_arb_fc_apply+0x280/0x2f8 [prestera] [
> 529.055452] sp : ffff80000b20bc60 [ 529.058781] x29: ffff80000b20bc60 x28:
> 0000000000000000 x27: ffff0001374d3a48 [ 529.065953] x26:
> ffff000105604000 x25: ffff000134af8a28 x24: ffff0001374d3800 [
> 529.073126] x23: ffff000101c89148 x22: ffff000101c89148 x21:
> ffff00013641fda0 [ 529.080299] x20: ffff000101c89000 x19: ffff000101c89020
> x18: 0000000000000059 [ 529.087471] x17: 0000000000000277 x16:
> 0000000000000000 x15: 0000000000000000 [ 529.094642] x14:
> 0000000000000003 x13: 00000000000fe400 x12: 0000000000000000 [
> 529.101814] x11: 0000000000000002 x10: 0000000000000aa0 x9 :
> ffff8000013cee80 [ 529.108985] x8 : 0000000000000018 x7 :
> 000000007b1703f8 x6 : 0000000000000018 [ 529.116157] x5 :
> 00000000d3497eb6 x4 : ffff000105604081 x3 : 000000008e979557 [
> 529.123329] x2 : 0000000000000000 x1 : ffff00010200de00 x0 :
> ffff000134ae3f80 [ 529.130501] Call trace:
> [ 529.132958] __prestera_fi_is_direct+0x2c/0x68 [prestera] [ 529.138394]
> prestera_k_arb_fib_evt+0x6b8/0xd50 [prestera] [ 529.143918]
> __prestera_router_fib_event_work+0x100/0x158 [prestera] [ 529.150313]
> process_one_work+0x208/0x488 [ 529.154348] worker_thread+0x4c/0x430
> [ 529.158030] kthread+0x120/0x138 [ 529.161274]
> ret_from_fork+0x10/0x20 [ 529.164867] ---[ end trace 0000000000000000 ]--
> -
>
> and results in a non offloaded route:
>
> $ ip route
> 10.0.0.0/24 dev swp5 proto kernel scope link src 10.0.0.1 rt_trap
> 10.0.1.0/24 nhid 20 via 10.0.0.2 dev swp5 rt_trap
>
> When creating a route referencing a nexthop via its ID, the nexthop will be
> stored in a separate nh pointer instead of the array of nexthops in the
> fib_info struct. This causes issues since fib_info_nh() only handles the
> nexthops array, but not the separate nh pointer, and will loudly WARN about
> it.
>
> In contrast fib_info_nhc() handles both, but returns a fib_nh_common
> pointer instead of a fib_nh pointer. Luckily we only ever access fields from
> the fib_nh_common parts, so we can just replace all instances of
> fib_info_nh() with fib_info_nhc() and access the fields via their
> fib_nh_common names.
>
> This allows handling IPv4 routes with an external nexthop, and they now get
> offloaded as expected:
>
> $ ip route
> 10.0.0.0/24 dev swp5 proto kernel scope link src 10.0.0.1 rt_trap
> 10.0.1.0/24 nhid 20 via 10.0.0.2 dev swp5 offload rt_offload
>
> Fixes: 396b80cb5cc8 ("net: marvell: prestera: Add neighbour cache
> accounting")
> Signed-off-by: Jonas Gorski <[email protected]>
> ---
> .../ethernet/marvell/prestera/prestera_router.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_router.c
> b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> index a9a1028cb17b..de317179a7dc 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_router.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_router.c
> @@ -166,11 +166,11 @@ prestera_util_neigh2nc_key(struct prestera_switch
> *sw, struct neighbour *n,
>
> static bool __prestera_fi_is_direct(struct fib_info *fi) {
> - struct fib_nh *fib_nh;
> + struct fib_nh_common *fib_nhc;
>
> if (fib_info_num_path(fi) == 1) {
> - fib_nh = fib_info_nh(fi, 0);
> - if (fib_nh->fib_nh_gw_family == AF_UNSPEC)
> + fib_nhc = fib_info_nhc(fi, 0);
> + if (fib_nhc->nhc_gw_family == AF_UNSPEC)
> return true;
> }
>
> @@ -261,7 +261,7 @@ static bool
> __prestera_util_kern_n_is_reachable_v4(u32 tb_id, __be32 *addr,
> struct net_device *dev)
> {
> - struct fib_nh *fib_nh;
> + struct fib_nh_common *fib_nhc;
> struct fib_result res;
> bool reachable;
>
> @@ -269,8 +269,8 @@ __prestera_util_kern_n_is_reachable_v4(u32 tb_id,
> __be32 *addr,
>
> if (!prestera_util_kern_get_route(&res, tb_id, addr))
> if (prestera_fi_is_direct(res.fi)) {
> - fib_nh = fib_info_nh(res.fi, 0);
> - if (dev == fib_nh->fib_nh_dev)
> + fib_nhc = fib_info_nhc(res.fi, 0);
> + if (dev == fib_nhc->nhc_dev)
> reachable = true;
> }
>
> @@ -324,7 +324,7 @@ prestera_kern_fib_info_nhc(struct fib_notifier_info
> *info, int n)
> if (info->family == AF_INET) {
> fen4_info = container_of(info, struct fib_entry_notifier_info,
> info);
> - return &fib_info_nh(fen4_info->fi, n)->nh_common;
> + return fib_info_nhc(fen4_info->fi, n);
> } else if (info->family == AF_INET6) {
> fen6_info = container_of(info, struct
> fib6_entry_notifier_info,
> info);
> --
> 2.41.0
>
>
> --
> BISDN GmbH
> K?rnerstra?e 7-10
> 10785 Berlin
> Germany
>
>
> Phone:
> +49-30-6108-1-6100
>
>
> Managing Directors:
> Dr.-Ing. Hagen Woesner, Andreas
> K?psel
>
>
> Commercial register:
> Amtsgericht Berlin-Charlottenburg HRB 141569
> B
> VAT ID No:?DE283257294
Acked-by: Elad Nachman <[email protected]>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
On Fri, 4 Aug 2023 12:12:20 +0200 you wrote:
> Fix handling IPv4 routes referencing a nexthop via its id by replacing
> calls to fib_info_nh() with fib_info_nhc().
>
> Trying to add an IPv4 route referencing a nextop via nhid:
>
> $ ip link set up swp5
> $ ip a a 10.0.0.1/24 dev swp5
> $ ip nexthop add dev swp5 id 20 via 10.0.0.2
> $ ip route add 10.0.1.0/24 nhid 20
>
> [...]
Here is the summary with links:
- net: marvell: prestera: fix handling IPv4 routes with nhid
https://git.kernel.org/netdev/net/c/2aa71b4b294e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html