[LEDE-DEV] [PATCH] netifd: Set source address for static address routes

[Felix Fietkau]

On 2016-10-06 10:52, Kristian Evensen wrote:
> When using UCI to configure static addresses, netifd does not set the source
> address of the gateway route. In order to be consistent, also set the source
> address for subnet routes (this applies to all protocols).
> 
> Signed-off-by: Kristian Evensen <kristian.evensen at gmail.com>
Sorry that it took me this long to get around to reviewing this.

> diff --git a/proto.c b/proto.c
> index 23304f3..03973e5 100644
> --- a/proto.c
> +++ b/proto.c

> @@ -148,7 +152,8 @@ error:
>  
>  static int
>  parse_static_address_option(struct interface *iface, struct blob_attr *attr,
> -			    bool v6, int netmask, bool ext, uint32_t broadcast)
> +			    bool v6, int netmask, bool ext, uint32_t broadcast,
> +				struct device_addr **addr_to_set)
>  {
>  	struct blob_attr *cur;
>  	int n_addr = 0;
> @@ -160,7 +165,7 @@ parse_static_address_option(struct interface *iface, struct blob_attr *attr,
>  
>  		n_addr++;
>  		if (!parse_addr(iface, blobmsg_data(cur), v6, netmask, ext,
> -				broadcast))
> +				broadcast, addr_to_set))
>  			return -1;
>  	}
>  
This can return multiple addresses. I think arbitrarily using the last
one may not be such a good idea.

> @@ -268,7 +273,8 @@ parse_address_list(struct interface *iface, struct blob_attr *attr, bool v6,
>  }
>  
>  static bool
> -parse_gateway_option(struct interface *iface, struct blob_attr *attr, bool v6)
> +parse_gateway_option(struct interface *iface, struct blob_attr *attr, bool v6,
> +					struct device_addr *addr_to_set)
Broken indentation, and src_addr is probably a better name here.

> @@ -402,6 +413,7 @@ proto_apply_static_ip_settings(struct interface *iface, struct blob_attr *attr)
>  	unsigned int netmask = 32;
>  	int n_v4 = 0, n_v6 = 0;
>  	struct in_addr bcast = {};
> +	struct device_addr *addr_to_set;
Also here.

>  
>  	blobmsg_parse(proto_ip_attributes, __OPT_MAX, tb, blob_data(attr), blob_len(attr));
>  
> @@ -422,11 +434,11 @@ proto_apply_static_ip_settings(struct interface *iface, struct blob_attr *attr)
>  
>  	if ((cur = tb[OPT_IPADDR]))
>  		n_v4 = parse_static_address_option(iface, cur, false,
> -			netmask, false, bcast.s_addr);
> +			netmask, false, bcast.s_addr, &addr_to_set);
>  
>  	if ((cur = tb[OPT_IP6ADDR]))
>  		n_v6 = parse_static_address_option(iface, cur, true,
> -			128, false, 0);
> +			128, false, 0, &addr_to_set);
>  
>  	if ((cur = tb[OPT_IP6PREFIX]))
>  		if (parse_prefix_list(iface, cur) < 0)
This can easily mix up v4 and v6 (they are allowed on the same interface).

> @@ -436,12 +448,12 @@ proto_apply_static_ip_settings(struct interface *iface, struct blob_attr *attr)
>  		goto out;
>  
>  	if ((cur = tb[OPT_GATEWAY])) {
> -		if (n_v4 && !parse_gateway_option(iface, cur, false))
> +		if (n_v4 && !parse_gateway_option(iface, cur, false, addr_to_set))
>  			goto out;
>  	}
>  
>  	if ((cur = tb[OPT_IP6GW])) {
> -		if (n_v6 && !parse_gateway_option(iface, cur, true))
> +		if (n_v6 && !parse_gateway_option(iface, cur, true, addr_to_set))
>  			goto out;
>  	}
>  
Same here.

- Felix