[PATCH libnl 05/11] route: Add support for MPLS address family

[Thomas Haller]

On Fri, 2017-06-30 at 10:48 -0600, David Ahern wrote:
> Add support for route in MPLS family. New attributes:
> - RTA_NEWDST - label stack for a nexthop
> - RTA_VIA - nexthop address (e.g., IPv4 or IPv6)
> 
> Other changes required:
> - scope has to be universe for MPLS routes so fixup
> rtnl_route_guess_scope
> - priority attribute should not be required for routes. For MPLS it
> can not
>   be set. Change ROUTE_ATTR_PRIO to only be set if priority is set.
> - table attribute should not be set unless something other than the
> default
>   table. For MPLS this attribute can not be set.
> 
> '/' is the separator in label stacks for consistency with iproute2.
> 
> Signed-off-by: David Ahern <dsahern at gmail.com>
> ---
>  include/netlink-private/types.h |   2 +
>  include/netlink/route/nexthop.h |   6 ++
>  lib/route/nexthop.c             |  97 +++++++++++++++++++++++++++++
>  lib/route/route.c               |   1 +
>  lib/route/route_obj.c           | 132
> +++++++++++++++++++++++++++++++++++-----
>  libnl-route-3.sym               |   4 ++
>  6 files changed, 228 insertions(+), 14 deletions(-)
> 



> --- a/lib/route/route_obj.c
> +++ b/lib/route/route_obj.c
> @@ -646,8 +646,11 @@ uint8_t rtnl_route_get_protocol(struct
> rtnl_route *route)
>  
>  void rtnl_route_set_priority(struct
> rtnl_route *route, uint32_t prio)
>  {
> -	route->rt_prio = prio;
> -	route->ce_mask |= ROUTE_ATTR_PRIO;
> +	/* MPLS address family does not allow RTA_PRIORITY to be set
> */
> +	if (route->rt_family != AF_MPLS) {
> +		route->rt_prio = prio;
> +		route->ce_mask |= ROUTE_ATTR_PRIO;
> +	}

it's a bit strange that the setter does nothing for MPLS family.

There are countless ways how to create invalid/incomplete objects.

If the user first calls rtnl_route_set_priority() before set_family(),
he still could set ROUTE_ATTR_PRIO.

Anyway, I don't mind this too much. Fine as is.


> @@ -1310,8 +1415,7 @@ struct nl_object_ops route_obj_ops = {
>  	.oo_update		= route_update,
>  	.oo_attrs2str		= route_attrs2str,
>  	.oo_id_attrs		= (ROUTE_ATTR_FAMILY |
> ROUTE_ATTR_TOS |
> -				   ROUTE_ATTR_TABLE | ROUTE_ATTR_DST
> |
> -				   ROUTE_ATTR_PRIO),
> +				   ROUTE_ATTR_TABLE |
> ROUTE_ATTR_DST),

as you surely are aware, these ID attributes for routes are not how
kernel handles routes (see your other patchset "route cache: support
for non-exclusive and append routes").

It seems dangerous to change the ID attributes of all routes (at this
point, without careful consideration).

Maybe we should instead pretend that for MPLS routes, the priority is
always implicitly set to zero?
Or maybe better: implement oo_id_attrs_get() instead, which continues
to give ROUTE_ATTR_PRIO for regular routes, but not for MPLS routes.



Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/libnl/attachments/20170814/0127674d/attachment.sig>