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: <BN9PR18MB4251ED95CE62843B994614BFDB0FA@BN9PR18MB4251.namprd18.prod.outlook.com>
Date:   Sun, 6 Aug 2023 06:33:57 +0000
From:   Elad Nachman <enachman@...vell.com>
To:     Jonas Gorski <jonas.gorski@...dn.de>,
        Taras Chornyi <taras.chornyi@...ision.eu>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Yevhen Orlov <yevhen.orlov@...ision.eu>,
        Oleksandr Mazur <oleksandr.mazur@...ision.eu>
CC:     "Taras Chornyi [C]" <tchornyi@...vell.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXT] [PATCH] net: marvell: prestera: fix handling IPv4 routes
 with nhid



> -----Original Message-----
> From: Jonas Gorski <jonas.gorski@...dn.de>
> Sent: Friday, August 4, 2023 1:12 PM
> To: Elad Nachman <enachman@...vell.com>; Taras Chornyi
> <taras.chornyi@...ision.eu>; David S. Miller <davem@...emloft.net>; Eric
> Dumazet <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>;
> Paolo Abeni <pabeni@...hat.com>; Yevhen Orlov
> <yevhen.orlov@...ision.eu>; Oleksandr Mazur
> <oleksandr.mazur@...ision.eu>
> Cc: Taras Chornyi [C] <tchornyi@...vell.com>; netdev@...r.kernel.org;
> linux-kernel@...r.kernel.org
> 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 <jonas.gorski@...dn.de>
> ---
>  .../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 <enachman@...vell.com>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ