| Message ID | 1439329548-50935-3-git-send-email-roopa@cumulusnetworks.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/12/15, 12:17 PM, Robert Shearman wrote: > 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; > ok, > ... >> 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. > yes, I did want to change max_labels to u8. will do in v2. thanks, Roopa -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/mpls_iptunnel.h b/include/net/mpls_iptunnel.h index 4757997..179253f 100644 --- a/include/net/mpls_iptunnel.h +++ b/include/net/mpls_iptunnel.h @@ -18,7 +18,7 @@ struct mpls_iptunnel_encap { u32 label[MAX_NEW_LABELS]; - u32 labels; + u8 labels; }; static inline struct mpls_iptunnel_encap *mpls_lwtunnel_encap(struct lwtunnel_state *lwtstate) diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c index cf86e9d..eb089ef 100644 --- a/net/mpls/af_mpls.c +++ b/net/mpls/af_mpls.c @@ -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; 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[]) { unsigned len = nla_len(nla); unsigned nla_labels; @@ -859,7 +859,7 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh, break; case RTA_DST: { - u32 label_count; + u8 label_count; if (nla_get_labels(nla, 1, &label_count, &cfg->rc_label)) goto errout; diff --git a/net/mpls/internal.h b/net/mpls/internal.h index f05e2e8..f5dafcaf 100644 --- a/net/mpls/internal.h +++ b/net/mpls/internal.h @@ -87,7 +87,7 @@ static inline struct mpls_entry_decoded mpls_entry_decode(struct mpls_shim_hdr * int nla_put_labels(struct sk_buff *skb, int attrtype, u8 labels, const u32 label[]); -int nla_get_labels(const struct nlattr *nla, u32 max_labels, u32 *labels, +int nla_get_labels(const struct nlattr *nla, u32 max_labels, u8 *labels, u32 label[]); bool mpls_output_possible(const struct net_device *dev); unsigned int mpls_dev_mtu(const struct net_device *dev);