netdev - Re: [PATCH net-next 4/7] mpls: Basic support for adding and removing routes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <54F6BEAA.2080708@cumulusnetworks.com>
Date:	Wed, 04 Mar 2015 00:13:30 -0800
From:	roopa <roopa@...ulusnetworks.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
CC:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Stephen Hemminger <stephen@...workplumber.org>,
	santiago@...reenet.org, Simon Horman <horms@...ge.net.au>
Subject: Re: [PATCH net-next 4/7] mpls: Basic support for adding and removing
 routes

On 3/3/15, 5:12 PM, Eric W. Biederman wrote:
> mpls_route_add and mpls_route_del implement the basic logic for adding
> and removing Next Hop Label Forwarding Entries from the MPLS input
> label map.  The addition and subtraction is done in a way that is
> consistent with how the existing routing table in Linux are
> maintained.  Thus all of the work to deal with NLM_F_APPEND,
> NLM_F_EXCL, NLM_F_REPLACE, and NLM_F_CREATE.
>
> Cases that are not clearly defined such as changing the interpretation
> of the mpls reserved labels is not allowed.
>
> Because it seems like the right thing to do adding an MPLS route without
> specifying an input label and allowing the kernel to pick a free label
> table entry is supported.   The implementation is currently less than optimal
> but that can be changed.
>
> As I don't have anything else to test with only ethernet and the loopback
> device are the only two device types currently supported for forwarding
> MPLS over.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> ---
>   net/mpls/af_mpls.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 133 insertions(+)
>
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index b097125dfa33..e432f092f2fb 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -16,6 +16,7 @@
>   #include <net/netns/generic.h>
>   #include "internal.h"
>   
> +#define LABEL_NOT_SPECIFIED (1<<20)
>   #define MAX_NEW_LABELS 2
>   
>   /* This maximum ha length copied from the definition of struct neighbour */
> @@ -211,6 +212,19 @@ static struct packet_type mpls_packet_type __read_mostly = {
>   	.func = mpls_forward,
>   };
>   
> +struct mpls_route_config {
> +	u32		rc_protocol;
> +	u32		rc_ifindex;
> +	u16		rc_via_family;
> +	u16		rc_via_alen;
> +	u8		rc_via[MAX_VIA_ALEN];
> +	u32		rc_label;
> +	u32		rc_output_labels;
> +	u32		rc_output_label[MAX_NEW_LABELS];
> +	u32		rc_nlflags;
> +	struct nl_info	rc_nlinfo;
> +};
> +
>   static struct mpls_route *mpls_rt_alloc(size_t alen)
>   {
>   	struct mpls_route *rt;
> @@ -245,6 +259,125 @@ static void mpls_route_update(struct net *net, unsigned index,
>   	mpls_rt_free(old);
>   }
>   
> +static unsigned find_free_label(struct net *net)
> +{
> +	unsigned index;
> +	for (index = 16; index < net->mpls.platform_labels; index++) {
> +		if (!net->mpls.platform_label[index])
> +			return index;
> +	}
> +	return LABEL_NOT_SPECIFIED;
> +}
> +
> +static int mpls_route_add(struct mpls_route_config *cfg)
> +{
> +	struct net *net = cfg->rc_nlinfo.nl_net;
> +	struct net_device *dev = NULL;
> +	struct mpls_route *rt, *old;
> +	unsigned index;
> +	int i;
> +	int err = -EINVAL;
> +
> +	index = cfg->rc_label;
> +
> +	/* If a label was not specified during insert pick one */
> +	if ((index == LABEL_NOT_SPECIFIED) &&
> +	    (cfg->rc_nlflags & NLM_F_CREATE)) {
> +		index = find_free_label(net);
> +	}
> +
> +	/* The first 16 labels are reserved, and may not be set */
> +	if (index < 16)
> +		goto errout;
> +
> +	/* The full 20 bit range may not be supported. */
> +	if (index >= net->mpls.platform_labels)
> +		goto errout;
> +
> +	/* Ensure only a supported number of labels are present */
> +	if (cfg->rc_output_labels > MAX_NEW_LABELS)
> +		goto errout;
> +
> +	err = -ENODEV;
> +	dev = dev_get_by_index(net, cfg->rc_ifindex);
> +	if (!dev)
> +		goto errout;
> +
> +	/* For now just support ethernet devices */
> +	err = -EINVAL;
> +	if ((dev->type != ARPHRD_ETHER) && (dev->type != ARPHRD_LOOPBACK))
> +		goto errout;
> +
> +	err = -EINVAL;
> +	if ((cfg->rc_via_family == AF_PACKET) &&
> +	    (dev->addr_len != cfg->rc_via_alen))
> +		goto errout;
> +
> +	/* Append makes no sense with mpls */
> +	err = -EINVAL;

minor nit: should this be -ENOTSUPP in that case ? (NLM_F_REPLACE and NLM_F_APPEND are
really operations. But, one can argue that they are an attribute of the msg and hence -EINVAL might be ok).
I did not find any other such case for consistency check.

> +	if (cfg->rc_nlflags & NLM_F_APPEND)
> +		goto errout;
> +
>
Thanks,
Roopa

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html