On 13/08/15 04:16, roopa wrote:
On 8/12/15, 12:15 PM, Robert Shearman wrote:
On 11/08/15 22:45, Roopa Prabhu wrote:
From: Roopa Prabhu <ro...@cumulusnetworks.com>
moves mpls_route nexthop fields to a new mpls_nhlfe
struct. mpls_nhlfe represents a mpls nexthop label forwarding entry.
It prepares mpls route structure for multipath support.
In the process moves mpls_route structure into internal.h.
Is there a requirement for moving this and the new datastructures into
internal.h? I may have missed it, but I don't see any dependency on
this in this patch series.
No dependency really. In my initial implementation of iptunnels I had
some shared code and it had been in internal.h since then.
i don't share any of this with iptunnels now. But, if you see patch
3/3, there is a lot more macros I add with struct nhlfe etc and it is
cleaner
to move all this to a header file than keeping it in the .c file.
Ok, I have no strong preference.
Moves some of the code from mpls_route_add into a separate mpls
nhlfe build function. changed mpls_rt_alloc to take number of
nexthops as argument.
A mpls route can point to multiple mpls_nhlfe. This patch
does not support multipath yet, hence the rest of the changes
assume that a mpls route points to a single mpls_nhlfe
Signed-off-by: Roopa Prabhu <ro...@cumulusnetworks.com>
---
net/mpls/af_mpls.c | 225
++++++++++++++++++++++++++++-----------------------
net/mpls/internal.h | 35 ++++++++
2 files changed, 158 insertions(+), 102 deletions(-)
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 8c5707d..cf86e9d 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -21,35 +21,6 @@
#endif
#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 */
-#define MAX_VIA_ALEN (ALIGN(MAX_ADDR_LEN, sizeof(unsigned long)))
-
-enum mpls_payload_type {
- MPT_UNSPEC, /* IPv4 or IPv6 */
- MPT_IPV4 = 4,
- MPT_IPV6 = 6,
-
- /* Other types not implemented:
- * - Pseudo-wire with or without control word (RFC4385)
- * - GAL (RFC5586)
- */
-};
-
-struct mpls_route { /* next hop label forwarding entry */
- struct net_device __rcu *rt_dev;
- struct rcu_head rt_rcu;
- u32 rt_label[MAX_NEW_LABELS];
- u8 rt_protocol; /* routing protocol that set this
entry */
- u8 rt_payload_type;
- u8 rt_labels;
- u8 rt_via_alen;
- u8 rt_via_table;
- u8 rt_via[0];
-};
-
static int zero = 0;
static int label_limit = (1 << 20) - 1;
...
@@ -281,13 +254,15 @@ struct mpls_route_config {
struct nl_info rc_nlinfo;
};
-static struct mpls_route *mpls_rt_alloc(size_t alen)
+static struct mpls_route *mpls_rt_alloc(int num_nh)
{
struct mpls_route *rt;
- rt = kzalloc(sizeof(*rt) + alen, GFP_KERNEL);
+ rt = kzalloc(sizeof(*rt) + (num_nh * sizeof(struct mpls_nhlfe)),
How about this instead:
offsetof(typeof(*rt), rt_nh[num_nh])
?
That way, you don't need to write out the type of rt_nh here.
I don't mind, but i followed existing convention for this (especially
the fib code).
would prefer keeping it the current way.
I don't think we have to follow the ipv4 convention here, but again I
have no strong preference.
+ GFP_KERNEL);
if (rt)
- rt->rt_via_alen = alen;
+ rt->rt_nhn = num_nh;
+
return rt;
}
Thanks for the review.
Thank you for implementing this functionality.
Rob
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
