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: <87h9u0lctq.fsf@x220.int.ebiederm.org>
Date:	Wed, 04 Mar 2015 14:36:33 -0600
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	roopa <roopa@...ulusnetworks.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

roopa <roopa@...ulusnetworks.com> writes:

> 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.

Yes.  IPv4 implements NLM_F_APPEND and IPv6 ignores it.

I will add a patch to change the error code.

Do you happen to know what NLM_F_APPEND means?  I couldn't figure out
when glancing through the IPv4 code.

NLM_F_REPLACE seems obvious.  Though it seems to have exactly the
oposite meaning of NLM_F_EXCL.  Which seems to make NLM_F_EXCL
redundant.

My hunch is that there are meanings here that apply when you are doing
longest prefix matching that don't quite apply when you are are doing
exact match routing.

Eric
--
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ