Subject: Re: [PATCH net-next 2/3] mpls: consistently use u8 to store number of labels

From: Robert Shearman <[email-masked]>
Date: Wed, 12 Aug 2015 20:17:18 +0100

On 11/08/15 22:45, Roopa Prabhu wrote:
> From: Roopa Prabhu <[email-masked]>
>
> change all types representing number of labels to u8
> to be consistent.
>
> This also changes labels to u8 in the light weight
> mpls_tunnel_encap structure. This is because the
> light weight mpls iptunnel code shares some of the label
> encoding functions like nla_get/put_labels with the af_mpls
> code.
>
> Signed-off-by: Roopa Prabhu <[email-masked]>
> ---
...
> @@ -243,11 +243,11 @@ static const struct nla_policy rtm_mpls_policy[RTA_MAX+1] = {
>   struct mpls_route_config {
>   	u32			rc_protocol;
>   	u32			rc_ifindex;
> -	u16			rc_via_table;
> -	u16			rc_via_alen;
> +	u8			rc_via_table;
> +	u8			rc_via_alen;

IMHO, it would be better to make rc_via_alen an int to avoid overflow 
which could cause false negatives in this check on the RTA_VIA attribute:

			if (cfg->rc_via_alen > MAX_VIA_ALEN)
				goto errout;

...
>   	u8			rc_via[MAX_VIA_ALEN];
> +	u8			rc_output_labels;
>   	u32			rc_label;
> -	u32			rc_output_labels;
>   	u32			rc_output_label[MAX_NEW_LABELS];
>   	u32			rc_nlflags;
>   	enum mpls_payload_type	rc_payload_type;
> @@ -751,7 +751,7 @@ int nla_put_labels(struct sk_buff *skb, int attrtype,
>   EXPORT_SYMBOL_GPL(nla_put_labels);
>
>   int nla_get_labels(const struct nlattr *nla,
> -		   u32 max_labels, u32 *labels, u32 label[])
> +		   u32 max_labels, u8 *labels, u32 label[])

How about making max_labels a u8? That would make it even more 
consistent and avoids any problem of overflow in the number of labels.

Thanks,
Rob