Re: [PATCH net-next 12/13] net: mctp: add gateway routing support

[Simon Horman <horms@kernel.org>]

On Wed, Jun 11, 2025 at 02:30:39PM +0800, Jeremy Kerr wrote:
> This change allows for gateway routing, where a route table entry
> may reference a routable endpoint (by network and EID), instead of
> routing directly to a netdevice.
> 
> We add support for a RTM_GATEWAY attribute for netlink route updates,
> with an attribute format of:
> 
>     struct mctp_fq_addr {
>         unsigned int net;
>         mctp_eid_t eid;
>     }
> 
> - we need the net here to uniquely identify the target EID, as we no
> longer have the device reference directly (which would provide the net
> id in the case of direct routes).
> 
> This makes route lookups recursive, as a route lookup that returns a
> gateway route must be resolved into a direct route (ie, to a device)
> eventually. We provide a limit to the route lookups, to prevent infinite
> loop routing.
> 
> The route lookup populates a new 'nexthop' field in the dst structure,
> which now specifies the key for the neighbour table lookup on device
> output, rather than using the packet destination address directly.
> 
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

...

> diff --git a/net/mctp/route.c b/net/mctp/route.c

...

> -/* base parsing; common to both _lookup and _populate variants */
> +/* base parsing; common to both _lookup and _populate variants.
> + *
> + * For gateway routes (which have a RTA_GATEWAY, and no RTA_OIF), we populate
> + * *gatweayp. for direct routes (RTA_OIF, no RTA_GATEWAY), we populate *mdev.
> + */
>  static int mctp_route_nlparse_common(struct net *net, struct nlmsghdr *nlh,
>  				     struct netlink_ext_ack *extack,
>  				     struct nlattr **tb, struct rtmsg **rtm,
>  				     struct mctp_dev **mdev,
> +				     struct mctp_fq_addr *gatewayp,
>  				     mctp_eid_t *daddr_start)
>  {
> +	struct mctp_fq_addr *gateway;
> +	unsigned int ifindex = 0;
>  	struct net_device *dev;
> -	unsigned int ifindex;
>  	int rc;
>  
>  	rc = nlmsg_parse(nlh, sizeof(struct rtmsg), tb, RTA_MAX,
> @@ -1321,11 +1372,44 @@ static int mctp_route_nlparse_common(struct net *net, struct nlmsghdr *nlh,
>  	}
>  	*daddr_start = nla_get_u8(tb[RTA_DST]);
>  
> -	if (!tb[RTA_OIF]) {
> -		NL_SET_ERR_MSG(extack, "ifindex missing");
> +	if (tb[RTA_OIF])
> +		ifindex = nla_get_u32(tb[RTA_OIF]);
> +
> +	if (tb[RTA_GATEWAY])
> +		gateway = nla_data(tb[RTA_GATEWAY]);
> +
> +	if (ifindex && gateway) {

Hi Jeremy,

gateway may be uninitialised here...

> +		NL_SET_ERR_MSG(extack,
> +			       "cannot specify both ifindex and gateway");
> +		return -EINVAL;
> +
> +	} else if (ifindex) {
> +		dev = __dev_get_by_index(net, ifindex);
> +		if (!dev) {
> +			NL_SET_ERR_MSG(extack, "bad ifindex");
> +			return -ENODEV;
> +		}
> +		*mdev = mctp_dev_get_rtnl(dev);
> +		if (!*mdev)
> +			return -ENODEV;
> +		gatewayp->eid = 0;
> +
> +	} else if (gateway) {

... and here.

Flagged by Smatch.

> +		if (!mctp_address_unicast(gateway->eid)) {
> +			NL_SET_ERR_MSG(extack, "bad gateway");
> +			return -EINVAL;
> +		}
> +
> +		gatewayp->eid = gateway->eid;
> +		gatewayp->net = gateway->net != MCTP_NET_ANY ?
> +			gateway->net :
> +			READ_ONCE(net->mctp.default_net);
> +		*mdev = NULL;
> +
> +	} else {
> +		NL_SET_ERR_MSG(extack, "no route output provided");
>  		return -EINVAL;
>  	}
> -	ifindex = nla_get_u32(tb[RTA_OIF]);
>  
>  	*rtm = nlmsg_data(nlh);
>  	if ((*rtm)->rtm_family != AF_MCTP) {

...