[PATCH] mrp: add periodictimer to retry lost packets

[PATCH] mrp: add periodictimer to retry lost packets

[Noel Burton-Krahn]

MRP doesn't implement the periodictimer in 802.1Q, so it never retries
if packets get lost.  I ran into this problem when MRP sent a MVRP
JoinIn before the interface was fully up.  The JoinIn was lost, MRP
didn't retry, and MVRP registration failed.

(Sorry for the repost, my earlier message wasn't formatted correctly)

Cheers,
--
Noel

diff --git a/include/net/mrp.h b/include/net/mrp.h
index 4fbf02a..0f7558b 100644
--- a/include/net/mrp.h
+++ b/include/net/mrp.h
@@ -112,6 +112,7 @@ struct mrp_applicant {
     struct mrp_application    *app;
     struct net_device    *dev;
     struct timer_list    join_timer;
+    struct timer_list    periodic_timer;

     spinlock_t        lock;
     struct sk_buff_head    queue;
diff --git a/net/802/mrp.c b/net/802/mrp.c
index 1eb05d8..edcd458 100644
--- a/net/802/mrp.c
+++ b/net/802/mrp.c
@@ -24,6 +24,11 @@
 static unsigned int mrp_join_time __read_mostly = 200;
 module_param(mrp_join_time, uint, 0644);
 MODULE_PARM_DESC(mrp_join_time, "Join time in ms (default 200ms)");
+
+static unsigned int mrp_periodic_time __read_mostly = 1000;
+module_param(mrp_periodic_time, uint, 0644);
+MODULE_PARM_DESC(mrp_periodic_time, "Periodic time in ms (default 1s)");
+
 MODULE_LICENSE("GPL");

 static const u8
@@ -595,6 +600,26 @@ static void mrp_join_timer(unsigned long data)
     mrp_join_timer_arm(app);
 }

+static void mrp_periodic_timer_arm(struct mrp_applicant *app)
+{
+    unsigned long delay;
+
+    delay = (u64)msecs_to_jiffies(mrp_periodic_time);
+    mod_timer(&app->periodic_timer, jiffies + delay);
+}
+
+static void mrp_periodic_timer(unsigned long data)
+{
+    struct mrp_applicant *app = (struct mrp_applicant *)data;
+
+    spin_lock(&app->lock);
+    mrp_mad_event(app, MRP_EVENT_PERIODIC);
+    mrp_pdu_queue(app);
+    spin_unlock(&app->lock);
+
+    mrp_periodic_timer_arm(app);
+}
+
 static int mrp_pdu_parse_end_mark(struct sk_buff *skb, int *offset)
 {
     __be16 endmark;
@@ -845,6 +870,9 @@ int mrp_init_applicant(struct net_device *dev,
struct mrp_application *appl)
     rcu_assign_pointer(dev->mrp_port->applicants[appl->type], app);
     setup_timer(&app->join_timer, mrp_join_timer, (unsigned long)app);
     mrp_join_timer_arm(app);
+    setup_timer(&app->periodic_timer, mrp_periodic_timer,
+            (unsigned long)app);
+    mrp_periodic_timer_arm(app);
     return 0;

 err3:
@@ -870,6 +898,7 @@ void mrp_uninit_applicant(struct net_device *dev,
struct mrp_application *appl)
      * all pending messages before the applicant is gone.
      */
     del_timer_sync(&app->join_timer);
+    del_timer_sync(&app->periodic_timer);

     spin_lock_bh(&app->lock);
     mrp_mad_event(app, MRP_EVENT_TX);

Re: [PATCH] mrp: add periodictimer to retry lost packets

[David Miller]

From: Noel Burton-Krahn <noel@burton-krahn.com>
Date: Wed, 18 Sep 2013 10:09:25 -0700

> MRP doesn't implement the periodictimer in 802.1Q, so it never retries
> if packets get lost.  I ran into this problem when MRP sent a MVRP
> JoinIn before the interface was fully up.  The JoinIn was lost, MRP
> didn't retry, and MVRP registration failed.
> 
> (Sorry for the repost, my earlier message wasn't formatted correctly)

Still corrupted by your email client, it turned TAB characters
into spaces.

Still missing a signoff.

Re: [PATCH] mrp: add periodictimer to retry lost packets

[Joe Perches]

On Wed, 2013-09-18 at 10:09 -0700, Noel Burton-Krahn wrote:
> MRP doesn't implement the periodictimer in 802.1Q, so it never retries
> if packets get lost.  I ran into this problem when MRP sent a MVRP
> JoinIn before the interface was fully up.  The JoinIn was lost, MRP
> didn't retry, and MVRP registration failed.

Beyond the whitespace errors that make this not apply:

> diff --git a/net/802/mrp.c b/net/802/mrp.c
[]
> @@ -595,6 +600,26 @@ static void mrp_join_timer(unsigned long data)
>      mrp_join_timer_arm(app);
>  }
> 
> +static void mrp_periodic_timer_arm(struct mrp_applicant *app)
> +{
> +    unsigned long delay;
> +
> +    delay = (u64)msecs_to_jiffies(mrp_periodic_time);

Useless cast to u64

> +    mod_timer(&app->periodic_timer, jiffies + delay);

This might also be neater without the temporary

static void mrp_periodic_timer_arm(struct mrp_applicant *app)
{
	mod_timer(&app->periodic_timer,
		  jiffies + msecs_to_jiffies(mrp_periodic_timer));
}

[PATCH 1/1] mrp: add periodictimer to allow retries when packets get lost

[Noel Burton-Krahn]

MRP doesn't implement the periodictimer in 802.1Q, so it never retries
if packets get lost.  I ran into this problem when MRP sent a MVRP
JoinIn before the interface was fully up.  The JoinIn was lost, MRP
didn't retry, and MVRP registration failed.

Tested against Juniper QFabric switches

Signed-off-by: Noel Burton-Krahn <noel@burton-krahn.com>
---
 include/net/mrp.h |    1 +
 net/802/mrp.c     |   27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/net/mrp.h b/include/net/mrp.h
index 4fbf02a..0f7558b 100644
--- a/include/net/mrp.h
+++ b/include/net/mrp.h
@@ -112,6 +112,7 @@ struct mrp_applicant {
 	struct mrp_application	*app;
 	struct net_device	*dev;
 	struct timer_list	join_timer;
+	struct timer_list	periodic_timer;
 
 	spinlock_t		lock;
 	struct sk_buff_head	queue;
diff --git a/net/802/mrp.c b/net/802/mrp.c
index 1eb05d8..3ed6162 100644
--- a/net/802/mrp.c
+++ b/net/802/mrp.c
@@ -24,6 +24,11 @@
 static unsigned int mrp_join_time __read_mostly = 200;
 module_param(mrp_join_time, uint, 0644);
 MODULE_PARM_DESC(mrp_join_time, "Join time in ms (default 200ms)");
+
+static unsigned int mrp_periodic_time __read_mostly = 1000;
+module_param(mrp_periodic_time, uint, 0644);
+MODULE_PARM_DESC(mrp_periodic_time, "Periodic time in ms (default 1s)");
+
 MODULE_LICENSE("GPL");
 
 static const u8
@@ -595,6 +600,24 @@ static void mrp_join_timer(unsigned long data)
 	mrp_join_timer_arm(app);
 }
 
+static void mrp_periodic_timer_arm(struct mrp_applicant *app)
+{
+	mod_timer(&app->periodic_timer,
+		  jiffies + msecs_to_jiffies(mrp_periodic_time));
+}
+
+static void mrp_periodic_timer(unsigned long data)
+{
+	struct mrp_applicant *app = (struct mrp_applicant *)data;
+
+	spin_lock(&app->lock);
+	mrp_mad_event(app, MRP_EVENT_PERIODIC);
+	mrp_pdu_queue(app);
+	spin_unlock(&app->lock);
+
+	mrp_periodic_timer_arm(app);
+}
+
 static int mrp_pdu_parse_end_mark(struct sk_buff *skb, int *offset)
 {
 	__be16 endmark;
@@ -845,6 +868,9 @@ int mrp_init_applicant(struct net_device *dev, struct mrp_application *appl)
 	rcu_assign_pointer(dev->mrp_port->applicants[appl->type], app);
 	setup_timer(&app->join_timer, mrp_join_timer, (unsigned long)app);
 	mrp_join_timer_arm(app);
+	setup_timer(&app->periodic_timer, mrp_periodic_timer,
+		    (unsigned long)app);
+	mrp_periodic_timer_arm(app);
 	return 0;
 
 err3:
@@ -870,6 +896,7 @@ void mrp_uninit_applicant(struct net_device *dev, struct mrp_application *appl)
 	 * all pending messages before the applicant is gone.
 	 */
 	del_timer_sync(&app->join_timer);
+	del_timer_sync(&app->periodic_timer);
 
 	spin_lock_bh(&app->lock);
 	mrp_mad_event(app, MRP_EVENT_TX);
-- 
1.7.9.5

Re: [PATCH 1/1] mrp: add periodictimer to allow retries when packets get lost

[David Miller]

From: Noel Burton-Krahn <noel@burton-krahn.com>
Date: Wed, 18 Sep 2013 12:24:40 -0700

> MRP doesn't implement the periodictimer in 802.1Q, so it never retries
> if packets get lost.  I ran into this problem when MRP sent a MVRP
> JoinIn before the interface was fully up.  The JoinIn was lost, MRP
> didn't retry, and MVRP registration failed.
> 
> Tested against Juniper QFabric switches
> 
> Signed-off-by: Noel Burton-Krahn <noel@burton-krahn.com>

David W., please review this patch.

Thanks.

Re: [PATCH 1/1] mrp: add periodictimer to allow retries when packets get lost

[Ward, David - 0663 - MITLL]

[-- Attachment #1: Type: text/plain, Size: 878 bytes --]

Noel, thanks for your patch. This timer was apparently not in the GARP 
standard but added in the MRP standard and I missed it. Looks good to me.

Acked-by: David Ward <david.ward@ll.mit.edu>


On 09/20/2013 02:59 PM, David Miller wrote:
> Re: [PATCH 1/1] mrp: add periodictimer to allow retries when packets 
> get lost
>
> From: Noel Burton-Krahn <noel@burton-krahn.com>
> Date: Wed, 18 Sep 2013 12:24:40 -0700
>
> > MRP doesn't implement the periodictimer in 802.1Q, so it never retries
> > if packets get lost.  I ran into this problem when MRP sent a MVRP
> > JoinIn before the interface was fully up.  The JoinIn was lost, MRP
> > didn't retry, and MVRP registration failed.
> >
> > Tested against Juniper QFabric switches
> >
> > Signed-off-by: Noel Burton-Krahn <noel@burton-krahn.com>
>
> David W., please review this patch.
>
> Thanks.
>



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4571 bytes --]

Re: [PATCH 1/1] mrp: add periodictimer to allow retries when packets get lost

[David Miller]

From: "Ward, David - 0663 - MITLL" <david.ward@ll.mit.edu>
Date: Fri, 20 Sep 2013 21:32:13 -0400

> Noel, thanks for your patch. This timer was apparently not in the GARP
> standard but added in the MRP standard and I missed it. Looks good to
> me.
> 
> Acked-by: David Ward <david.ward@ll.mit.edu>

Applied, thanks everyone.