Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Elad Nachman <enachman@marvell.com>
To:     Jonas Gorski <jonas.gorski@bisdn.de>,
        Taras Chornyi <taras.chornyi@plvision.eu>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Yevhen Orlov <yevhen.orlov@plvision.eu>,
        Oleksandr Mazur <oleksandr.mazur@plvision.eu>
CC:     "Taras Chornyi [C]" <tchornyi@marvell.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [EXT] [PATCH] net: marvell: prestera: fix handling IPv4 routes
 with nhid
Thread-Topic: [EXT] [PATCH] net: marvell: prestera: fix handling IPv4 routes
 with nhid
Thread-Index: AQHZxrw7PDULGwpVhUa3n50lD7b1Z6/c0mSA
Date:   Sun, 6 Aug 2023 06:33:57 +0000
Message-ID: <BN9PR18MB4251ED95CE62843B994614BFDB0FA@BN9PR18MB4251.namprd18.prod.outlook.com>
References: <20230804101220.247515-1-jonas.gorski@bisdn.de>
In-Reply-To: <20230804101220.247515-1-jonas.gorski@bisdn.de>
Accept-Language: he-IL, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?EuWZ+d+lQcuD8ULkoDGwf20KsGk5ZMiILddifBUqBBj/Wgc2QoG5Q0YsXv?=
 =?iso-8859-1?Q?jNrG1Xaw0MqlohFf+AWv3fkCdfwYrxea971//63CVyfz18pon6VbVAursH?=
 =?iso-8859-1?Q?s8EhalUe7lleH//mom1PsLcdARyCJfkpsHJIsJypghsl1yI4M0a4NS8uko?=
 =?iso-8859-1?Q?CqtpiSWDTCQYvjb17K9J3+X/THqdjovGw5dGYb7cpbZ28XtXvD1xajScyA?=
 =?iso-8859-1?Q?D+VL0pDUP6crZ3BneOvb5F1nBy+NPLSGnNuT1H1I5VUbFqOX4Jl4CT+/h1?=
 =?iso-8859-1?Q?6lHMWgYxDSHVyVbWqpkhStY8o2OcXanY0pOHBB3ciK1TvQNeiLjER1mVEy?=
 =?iso-8859-1?Q?JWOtzvsU8hHrJ5nUxkaVAGJ7P/tOP5CfTCfqibaxYtQjIiWD9PLSpqVt2U?=
 =?iso-8859-1?Q?2dfmu6sXBJzSMxCQ3m/JwcNfwL96JmRWI8zpMwK3okMTxcAnBhZwMxz2ds?=
 =?iso-8859-1?Q?0dIjcru83KBz1kmzcvdyqfcVhitnzRDXj/ydMMLfdwL2X2kL98P9yoYad2?=
 =?iso-8859-1?Q?3oZ2M8VoegFZ5M9eXyZDzMu2/2UUY2EmPYQzji9KzGtIOoeHIkKxQpynTw?=
 =?iso-8859-1?Q?ZS9LrXZIfftfcz1wtIQZJJlgS9cvpCrBVELqndk0V/U1eWS+cWwpYSq8vJ?=
 =?iso-8859-1?Q?4UKPIuu3fBE+iTAVR3RRQ8S6fCVWXJbEBHNJByBdkAtxPpTig+ipttw8Bk?=
 =?iso-8859-1?Q?vCgbUGE6iNLk44UZe20WQzJNbMimoxsnplRfLMmruaJpMAn6fP0TxBQzCv?=
 =?iso-8859-1?Q?aTRDlwHZ15cVw0X8DhEKM2fONbwxpZpfo02FJwEi3xvsnJNtOxKnaz3Hqo?=
 =?iso-8859-1?Q?Hll6lxIC9c/K6cwVEMlex6MLll/k1cQJe3OjPKriKNwXRMeY20hIWDDkMr?=
 =?iso-8859-1?Q?PFLOYaF608TCwjYtkBNP2pEyqO6DL5omoTADDNI3BqseMROVo71DxrZp6n?=
 =?iso-8859-1?Q?M9X1JlB8EGvsG3xcVfHvDhRTg3wvIMSMsfuo7cVAJFGbq7Iu5xDlfXtdlw?=
 =?iso-8859-1?Q?+YVtzhvC0635J5wFQz7r85cxC3G020tvr4UW1DCuxYy0Ck+G2ijDIk8va3?=
 =?iso-8859-1?Q?DUicI/a0BWAPhIkJaNXgcCTMzlSvbi6r/l+QpGGFGckCE5ITlBQmdVsZ45?=
 =?iso-8859-1?Q?aGhXR8GC3/8WV54PHzdGHmSU0VG/lFKMDBuNrJKa+a8RF3Gs0Og7AMB/k/?=
 =?iso-8859-1?Q?g+Vk9LX5+g4QOrN2h9oG4EQzwapwdF5GGMbTBkm5p+s8mL/v91d3NgwVjG?=
 =?iso-8859-1?Q?tl2H3gUFxV8a/jpfGGFM/4igVe9gBS8PV1F2+8x1i2HqMDg2GJhkXwdDKz?=
 =?iso-8859-1?Q?yNyljyV1kfXenGYa78Sb/Kx5PV67hPAb2LbmgXH2lxmeFI2I2wxugf3ya7?=
 =?iso-8859-1?Q?xHPvOJjwiPbFSbmfsCbsHoIdKlbhMRMVzsfSM5+hjbzs1NqPiN6ubOCphK?=
 =?iso-8859-1?Q?Lx6e9r7KbtE6RxymTdz9T8uoBoDt+ipVKERSrALlyTPGJOdidXAKxXvyJM?=
 =?iso-8859-1?Q?LPhvCUYI35XZh0uMeqDkIm2xAjWR2m9Tqc6T2STzPhM7A1uTdMiOx2S7ZR?=
 =?iso-8859-1?Q?fuNdJnvPMbi8nQUa4s1QwyVdBs0xDWb9nctaeBeewVK1yxSMqm3hVqvBtV?=
 =?iso-8859-1?Q?DQfFPsEY2XjeoU8pQFT6ITdeTBe02kBL9D?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR18MB4251.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 62d7d04c-8876-400d-2931-08db96471d39
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Aug 2023 06:33:57.9755
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ewGmnpEqG95ZBGX3NIjLPJuy3cT3ez5PCYGagrIFxt2VLbngwyup+kxeNa7kMnuyjZmIElQNAhBdzIiQOBvAaw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR18MB4405
X-Proofpoint-GUID: HymuAD46Wo9EO6o0Q2fwjldVBF8jF8jW
X-Proofpoint-ORIG-GUID: HymuAD46Wo9EO6o0Q2fwjldVBF8jF8jW
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-08-06_01,2023-08-03_01,2023-05-22_02
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Jonas Gorski <jonas.gorski@bisdn.de>
> Sent: Friday, August 4, 2023 1:12 PM
> To: Elad Nachman <enachman@marvell.com>; Taras Chornyi
> <taras.chornyi@plvision.eu>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Yevhen Orlov
> <yevhen.orlov@plvision.eu>; Oleksandr Mazur
> <oleksandr.mazur@plvision.eu>
> Cc: Taras Chornyi [C] <tchornyi@marvell.com>; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: [EXT] [PATCH] net: marvell: prestera: fix handling IPv4 routes w=
ith
> nhid
>=20
> External Email
>=20
> ----------------------------------------------------------------------
> Fix handling IPv4 routes referencing a nexthop via its id by replacing ca=
lls to
> fib_info_nh() with fib_info_nhc().
>=20
> Trying to add an IPv4 route referencing a nextop via nhid:
>=20
>     $ 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
>=20
> triggers warnings when trying to handle the route:
>=20
> [  528.805763] ------------[ cut here ]------------ [  528.810437] WARNIN=
G: 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_po=
lice
> 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=3D--) [  528.864347] pc : __prestera_fi_is_direct+0x2c/0x68 [preste=
ra] [
> 528.870135] lr : prestera_k_arb_fib_evt+0xb20/0xd50 [prestera] [
> 528.876007] sp : ffff80000b20bc90 [  528.879336] x29: ffff80000b20bc90 x2=
8:
> 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.95895=
2]
> __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] WARN=
ING: 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_po=
lice
> 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=3D--) [  529.043533] pc : __prestera_fi_is_direct+0x2c/0x68 [preste=
ra] [
> 529.049318] lr : __prestera_k_arb_fc_apply+0x280/0x2f8 [prestera] [
> 529.055452] sp : ffff80000b20bc60 [  529.058781] x29: ffff80000b20bc60 x2=
8:
> 0000000000000000 x27: ffff0001374d3a48 [  529.065953] x26:
> ffff000105604000 x25: ffff000134af8a28 x24: ffff0001374d3800 [
> 529.073126] x23: ffff000101c89148 x22: ffff000101c89148 x21:
> ffff00013641fda0 [  529.080299] x20: ffff000101c89000 x19: ffff000101c890=
20
> 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.13839=
4]
> 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 ]-=
-
> -
>=20
> and results in a non offloaded route:
>=20
>     $ 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
>=20
> 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 abo=
ut
> it.
>=20
> 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 f=
rom
> 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.
>=20
> This allows handling IPv4 routes with an external nexthop, and they now g=
et
> offloaded as expected:
>=20
>     $ 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
>=20
> Fixes: 396b80cb5cc8 ("net: marvell: prestera: Add neighbour cache
> accounting")
> Signed-off-by: Jonas Gorski <jonas.gorski@bisdn.de>
> ---
>  .../ethernet/marvell/prestera/prestera_router.c    | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>=20
> 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,
>=20
>  static bool __prestera_fi_is_direct(struct fib_info *fi)  {
> -	struct fib_nh *fib_nh;
> +	struct fib_nh_common *fib_nhc;
>=20
>  	if (fib_info_num_path(fi) =3D=3D 1) {
> -		fib_nh =3D fib_info_nh(fi, 0);
> -		if (fib_nh->fib_nh_gw_family =3D=3D AF_UNSPEC)
> +		fib_nhc =3D fib_info_nhc(fi, 0);
> +		if (fib_nhc->nhc_gw_family =3D=3D AF_UNSPEC)
>  			return true;
>  	}
>=20
> @@ -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;
>=20
> @@ -269,8 +269,8 @@ __prestera_util_kern_n_is_reachable_v4(u32 tb_id,
> __be32 *addr,
>=20
>  	if (!prestera_util_kern_get_route(&res, tb_id, addr))
>  		if (prestera_fi_is_direct(res.fi)) {
> -			fib_nh =3D fib_info_nh(res.fi, 0);
> -			if (dev =3D=3D fib_nh->fib_nh_dev)
> +			fib_nhc =3D fib_info_nhc(res.fi, 0);
> +			if (dev =3D=3D fib_nhc->nhc_dev)
>  				reachable =3D true;
>  		}
>=20
> @@ -324,7 +324,7 @@ prestera_kern_fib_info_nhc(struct fib_notifier_info
> *info, int n)
>  	if (info->family =3D=3D AF_INET) {
>  		fen4_info =3D 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 =3D=3D AF_INET6) {
>  		fen6_info =3D container_of(info, struct
> fib6_entry_notifier_info,
>  					 info);
> --
> 2.41.0
>=20
>=20
> --
> BISDN GmbH
> K=F6rnerstra=DFe 7-10
> 10785 Berlin
> Germany
>=20
>=20
> Phone:
> +49-30-6108-1-6100
>=20
>=20
> Managing Directors:
> Dr.-Ing. Hagen Woesner, Andreas
> K=F6psel
>=20
>=20
> Commercial register:
> Amtsgericht Berlin-Charlottenburg HRB 141569
> B
> VAT ID No:=A0DE283257294

Acked-by: Elad Nachman <enachman@marvell.com>