[PATCH net-next 0/3] Minor ipv6 mcast cleanups

[PATCH net-next 0/3] Minor ipv6 mcast cleanups

[Daniel Borkmann]

Daniel Borkmann (3):
  net: ipv6: igmp6_event_query: use msecs_to_jiffies
  net: ipv6: minor: *_start_timer: rather use unsigned long
  net: ipv6: mcast: minor: use defines for rfc3810/8.1 lengths

 net/ipv6/mcast.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

-- 
1.7.11.7

[PATCH net-next 1/3] net: ipv6: igmp6_event_query: use msecs_to_jiffies

[Daniel Borkmann]

Use proper API functions to calculate jiffies from milliseconds and
not the crude method of dividing HZ by a value. This ensures more
accurate values even in the case of strange HZ values. While at it,
also simplify code in the mlh2 case by using max().

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/ipv6/mcast.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 6c76df9..57863e2 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1150,9 +1150,7 @@ int igmp6_event_query(struct sk_buff *skb)
 		int switchback;
 		/* MLDv1 router present */
 
-		/* Translate milliseconds to jiffies */
-		max_delay = (ntohs(mld->mld_maxdelay)*HZ)/1000;
-
+		max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
 		switchback = (idev->mc_qrv + 1) * max_delay;
 		idev->mc_v1_seen = jiffies + switchback;
 
@@ -1169,10 +1167,11 @@ int igmp6_event_query(struct sk_buff *skb)
 			return -EINVAL;
 
 		mlh2 = (struct mld2_query *)skb_transport_header(skb);
-		max_delay = (MLDV2_MRC(ntohs(mlh2->mld2q_mrc))*HZ)/1000;
-		if (!max_delay)
-			max_delay = 1;
+
+		max_delay = max(msecs_to_jiffies(MLDV2_MRC(ntohs(mlh2->mld2q_mrc))), 1UL);
+
 		idev->mc_maxdelay = max_delay;
+
 		if (mlh2->mld2q_qrv)
 			idev->mc_qrv = mlh2->mld2q_qrv;
 		if (group_type == IPV6_ADDR_ANY) { /* general query */
-- 
1.7.11.7

[PATCH net-next 2/3] net: ipv6: minor: *_start_timer: rather use unsigned long

[Daniel Borkmann]

For the functions mld_gq_start_timer(), mld_ifc_start_timer(),
and mld_dad_start_timer(), rather use unsigned long than int
as we operate only on unsigned values anyway. This seems more
appropriate as there is no good reason to do type conversions
to int, that could lead to future errors.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/ipv6/mcast.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 57863e2..33f98d9 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -996,24 +996,24 @@ bool ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
 
 static void mld_gq_start_timer(struct inet6_dev *idev)
 {
-	int tv = net_random() % idev->mc_maxdelay;
+	unsigned long tv = net_random() % idev->mc_maxdelay;
 
 	idev->mc_gq_running = 1;
 	if (!mod_timer(&idev->mc_gq_timer, jiffies+tv+2))
 		in6_dev_hold(idev);
 }
 
-static void mld_ifc_start_timer(struct inet6_dev *idev, int delay)
+static void mld_ifc_start_timer(struct inet6_dev *idev, unsigned long delay)
 {
-	int tv = net_random() % delay;
+	unsigned long tv = net_random() % delay;
 
 	if (!mod_timer(&idev->mc_ifc_timer, jiffies+tv+2))
 		in6_dev_hold(idev);
 }
 
-static void mld_dad_start_timer(struct inet6_dev *idev, int delay)
+static void mld_dad_start_timer(struct inet6_dev *idev, unsigned long delay)
 {
-	int tv = net_random() % delay;
+	unsigned long tv = net_random() % delay;
 
 	if (!mod_timer(&idev->mc_dad_timer, jiffies+tv+2))
 		in6_dev_hold(idev);
-- 
1.7.11.7

[PATCH net-next 3/3] net: ipv6: mcast: minor: use defines for rfc3810/8.1 lengths

[Daniel Borkmann]

Instead of hard-coding length values, use a define to make it clear
where those lengths come from.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/ipv6/mcast.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 33f98d9..98ead2b 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -107,9 +107,12 @@ static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *pmca,
 static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 			    struct inet6_dev *idev);
 
-
 #define MLD_QRV_DEFAULT		2
 
+/* RFC3810, 8.1 Query Version Distinctions */
+#define MLD_V1_QUERY_LEN	24
+#define MLD_V2_QUERY_LEN_MIN	28
+
 #define MLD_V1_SEEN(idev) (dev_net((idev)->dev)->ipv6.devconf_all->force_mld_version == 1 || \
 		(idev)->cnf.force_mld_version == 1 || \
 		((idev)->mc_v1_seen && \
@@ -1146,7 +1149,7 @@ int igmp6_event_query(struct sk_buff *skb)
 	    !(group_type&IPV6_ADDR_MULTICAST))
 		return -EINVAL;
 
-	if (len == 24) {
+	if (len == MLD_V1_QUERY_LEN) {
 		int switchback;
 		/* MLDv1 router present */
 
@@ -1160,7 +1163,7 @@ int igmp6_event_query(struct sk_buff *skb)
 			__in6_dev_put(idev);
 		/* clear deleted report items */
 		mld_clear_delrec(idev);
-	} else if (len >= 28) {
+	} else if (len >= MLD_V2_QUERY_LEN_MIN) {
 		int srcs_offset = sizeof(struct mld2_query) -
 				  sizeof(struct icmp6hdr);
 		if (!pskb_may_pull(skb, srcs_offset))
-- 
1.7.11.7

Re: [PATCH net-next 1/3] net: ipv6: igmp6_event_query: use msecs_to_jiffies

[Hannes Frederic Sowa]

On Tue, Aug 20, 2013 at 12:22:00PM +0200, Daniel Borkmann wrote:
> Use proper API functions to calculate jiffies from milliseconds and
> not the crude method of dividing HZ by a value. This ensures more
> accurate values even in the case of strange HZ values. While at it,
> also simplify code in the mlh2 case by using max().
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Re: [PATCH net-next 2/3] net: ipv6: minor: *_start_timer: rather use unsigned long

[Hannes Frederic Sowa]

On Tue, Aug 20, 2013 at 12:22:01PM +0200, Daniel Borkmann wrote:
> For the functions mld_gq_start_timer(), mld_ifc_start_timer(),
> and mld_dad_start_timer(), rather use unsigned long than int
> as we operate only on unsigned values anyway. This seems more
> appropriate as there is no good reason to do type conversions
> to int, that could lead to future errors.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Re: [PATCH net-next 3/3] net: ipv6: mcast: minor: use defines for rfc3810/8.1 lengths

[Hannes Frederic Sowa]

On Tue, Aug 20, 2013 at 12:22:02PM +0200, Daniel Borkmann wrote:
> Instead of hard-coding length values, use a define to make it clear
> where those lengths come from.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Nice cleanups, thanks Daniel!

Re: [PATCH net-next 0/3] Minor ipv6 mcast cleanups

[David Miller]

From: Daniel Borkmann <dborkman@redhat.com>
Date: Tue, 20 Aug 2013 12:21:59 +0200

> Daniel Borkmann (3):
>   net: ipv6: igmp6_event_query: use msecs_to_jiffies
>   net: ipv6: minor: *_start_timer: rather use unsigned long
>   net: ipv6: mcast: minor: use defines for rfc3810/8.1 lengths

All applied, thanks Daniel.