All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Shearman <rshearma@brocade.com>
To: Roopa Prabhu <roopa@cumulusnetworks.com>, <davem@davemloft.net>
Cc: <ebiederm@xmission.com>, <netdev@vger.kernel.org>
Subject: Re: [PATCH net-next 2/3] mpls: consistently use u8 to store number of labels
Date: Wed, 12 Aug 2015 20:17:18 +0100	[thread overview]
Message-ID: <55CB9BBE.2000303@brocade.com> (raw)
In-Reply-To: <1439329548-50935-3-git-send-email-roopa@cumulusnetworks.com>

On 11/08/15 22:45, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> 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 <roopa@cumulusnetworks.com>
> ---
...
> @@ -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

  reply	other threads:[~2015-08-12 19:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-11 21:45 [PATCH net-next 2/3] mpls: consistently use u8 to store number of labels Roopa Prabhu
2015-08-12 19:17 ` Robert Shearman [this message]
2015-08-13  3:25   ` roopa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55CB9BBE.2000303@brocade.com \
    --to=rshearma@brocade.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@cumulusnetworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.