Re: [PATCH v2] netlink: Strip nexthop identifiers when duplicating routes

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]

On Wed, Jun 19, 2024 at 06:21:47PM +0200, Stefano Brivio wrote:
> If routing daemons set up host routes, for example FRR via OSPF as in
> the reported issue, they might add nexthop identifiers (not objects)
> that are generally not valid in the target namespace. Strip them off
> as well, otherwise we'll get EINVAL from the kernel.
> 
> Link: https://github.com/containers/podman/issues/22960
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2: oops, it looks like I didn't run this through clang-tidy :( and it
>     reported a bugprone-branch-clone if I have two branches both doing
>     the same thing (rta->rta_type = RTA_UNSPEC). I condensed comments
>     under the same branch, probably more elegant than carrying around
>     yet another suppression.
> 
>  netlink.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/netlink.c b/netlink.c
> index 2c9e71f..c082991 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -600,13 +600,22 @@ int nl_route_dup(int s_src, unsigned int ifi_src,
>  
>  				if (discard)
>  					break;
> -			} else if (rta->rta_type == RTA_PREFSRC) {
> -				/* Host routes might include a preferred source
> -				 * address, which must be one of the host's
> -				 * addresses.  However, with -a pasta will use a
> -				 * different namespace address, making such a
> -				 * route invalid in the namespace.  Strip off
> -				 * RTA_PREFSRC attributes to avoid that. */
> +			} else if (rta->rta_type == RTA_PREFSRC ||
> +				   rta->rta_type == RTA_NH_ID) {
> +				/* Strip RTA_PREFSRC attributes: host routes
> +				 * might include a preferred source address,
> +				 * which must be one of the host's addresses.
> +				 * However, with -a, pasta will use a different
> +				 * namespace address, making such a route
> +				 * invalid in the namespace.
> +				 *
> +				 * Strip RTA_NH_ID attributes: host routes set
> +				 * up via routing protocols (e.g. OSPF) might
> +				 * contain a nexthop ID (and not nexthop
> +				 * objects, which are taken care of in the
> +				 * RTA_MULTIPATH case above) that's not valid
> +				 * in the target namespace.
> +				 */
>  				rta->rta_type = RTA_UNSPEC;
>  			}
>  		}

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]