Multicast routing stops functioning after 4G multicast packets recived.

DISCUSSION Bob Falken 3 participants 6 messages Jan 7, 2014

Subsystems (3)

Networking Network drivers IPv4/IPv6

lore

Share & tools

lore

Bob Falken

author

2014-01-07 17:01 UTC #0 lore raw

Hello,

I patched, kernel 3.2.53 yesterday,
8774002632packets, and ~9.1TB later, the multicast routing seems to function properly. :)

Kudos for fixing this.

I will keep checking the next days.

As for IPv6 MR, my current setup i.e: the Multicast source, does not support IPv6, so cannot do a check for that natively.

Unless I can translate IPv4 multicast into IPv6 multicast easily, using some iptable prerouting rules(?).

Thanks again.

----- Original Message -----
From: Hannes Frederic Sowa
Sent: 01/05/14 12:38 AM
To: Julian Anastasov
Subject: Re: Multicast routing stops functioning after 4G multicast packets recived.

Quoted 15 lines · Julian Anastasov

Hello,
Quoted 3 lines · Bob Falken
Hello, I have an issue after kernel 2.6.37 and above.
If i roll back to kernel 2.6.36.4 everything is fine.
if recive more than 4294967295 multicast packets, the kernel does not register the multicast packets. and multicast routing does not functioning.
...
Quoted 4 lines
I think there might be a variable in the kernel that get overflown. though i cannot be sure as im not a programmer.
Let me know if you need more debug information.

Quoted 7 lines

Quoted 1 line
Please help.
As Hannes guessed, may be it is really the missing flags = FIB_LOOKUP_NOREF in ipmr_fib_lookup() when calling fib_rules_lookup(), we have an atomic_inc_not_zero() there that can stop to work after 4G lookups when rule's refcnt reaches 0. As result, fib_rules_lookup() can start to return -ESRCH.

I guess we should just try. I somehow forgot to look after that. Thanks for reminding, Julian.

Bob, can you try with this patch again?

net/ipv4/ipmr.c +5 -2

index 421a249..b9b3472 100644
@@ -157,9 +157,12 @@ static struct mr_table *ipmr_get_table(struct net *net, u32 id)
 static int ipmr_fib_lookup(struct net *net, struct flowi4 *flp4,
 struct mr_table **mrt)
 {
- struct ipmr_result res;
- struct fib_lookup_arg arg = { .result = &res};
 int err;
+ struct ipmr_result res;
+ struct fib_lookup_arg arg = {
+ .result = &res,
+ .flags = FIB_LOOKUP_NOREF,
+ };
 
 err = fib_rules_lookup(net->ipv4.mr_rules_ops,
 flowi4_to_flowi(flp4), 0, &arg);

Hannes Frederic Sowa

2014-01-07 17:43 UTC #1 lore raw

Quoted 5 lines · Bob Falken

Hello,
I patched, kernel 3.2.53 yesterday,
8774002632packets, and ~9.1TB later, the multicast routing seems to function properly. :)

Quoted 7 lines

Kudos for fixing this.
I will keep checking the next days.
As for IPv6 MR, my current setup i.e: the Multicast source, does not support IPv6, so cannot do a check for that natively.
Unless I can translate IPv4 multicast into IPv6 multicast easily, using some iptable prerouting rules(?).

Thank you for testing!

I'll review the RCU regions again and will prepare the patches (before introduction of ebc0ffae5dfb44 ("fib: RCU conversion of fib_lookup()") and after.

Hannes Frederic Sowa

2014-01-07 20:11 UTC #2 lore raw

Quoted 12 lines · Hannes Frederic Sowa

Quoted 11 lines · Bob Falken
Hello,
I patched, kernel 3.2.53 yesterday,
8774002632packets, and ~9.1TB later, the multicast routing seems to function properly. :)
Kudos for fixing this.
I will keep checking the next days.
As for IPv6 MR, my current setup i.e: the Multicast source, does not support IPv6, so cannot do a check for that natively.

Quoted 7 lines

Quoted 1 line
Unless I can translate IPv4 multicast into IPv6 multicast easily, using some iptable prerouting rules(?).
Thank you for testing!
I'll review the RCU regions again and will prepare the patches (before introduction of ebc0ffae5dfb44 ("fib: RCU conversion of fib_lookup()") and after.

It seems ip(6)mr_fib_lookup is not always called from rcu section (ndo_start_xmit), so I had to restructure a bit. Could you retest this patch as preparation for a submission to stable? Thanks!

RCU conversion can be done later then.

net/ipv4/ipmr.c +19 -6

index 421a249..9ae4ae7 100644
@@ -157,9 +157,12 @@ static struct mr_table *ipmr_get_table(struct net *net, u32 id)
 static int ipmr_fib_lookup(struct net *net, struct flowi4 *flp4,
 			   struct mr_table **mrt)
 {
-	struct ipmr_result res;
-	struct fib_lookup_arg arg = { .result = &res, };
 	int err;
+	struct ipmr_result res;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
 
 	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
 			       flowi4_to_flowi(flp4), 0, &arg);

@@ -448,16 +451,23 @@ failure:
 
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 {
+	int err;
+	struct ipmr_result res;
 	struct net *net = dev_net(dev);
-	struct mr_table *mrt;
+
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
+
 	struct flowi4 fl4 = {
 		.flowi4_oif	= dev->ifindex,
 		.flowi4_iif	= skb->skb_iif,
 		.flowi4_mark	= skb->mark,
 	};
-	int err;
 
-	err = ipmr_fib_lookup(net, &fl4, &mrt);
+	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
+			       flowi4_to_flowi(&fl4), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;

@@ -466,9 +476,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ipmr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, IGMPMSG_WHOLEPKT);
+	ipmr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			  IGMPMSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }

net/ipv6/ip6mr.c +16 -6

index f365310..45ec621 100644
@@ -141,9 +141,12 @@ static struct mr6_table *ip6mr_get_table(struct net *net, u32 id)
 static int ip6mr_fib_lookup(struct net *net, struct flowi6 *flp6,
 			    struct mr6_table **mrt)
 {
-	struct ip6mr_result res;
-	struct fib_lookup_arg arg = { .result = &res, };
 	int err;
+	struct ip6mr_result res;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
 
 	err = fib_rules_lookup(net->ipv6.mr6_rules_ops,
 			       flowi6_to_flowi(flp6), 0, &arg);

@@ -693,16 +696,20 @@ static const struct inet6_protocol pim6_protocol = {
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb,
 				      struct net_device *dev)
 {
+	int err;
+	struct ip6mr_result res;
 	struct net *net = dev_net(dev);
-	struct mr6_table *mrt;
 	struct flowi6 fl6 = {
 		.flowi6_oif	= dev->ifindex,
 		.flowi6_iif	= skb->skb_iif,
 		.flowi6_mark	= skb->mark,
 	};
-	int err;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+	};
 
-	err = ip6mr_fib_lookup(net, &fl6, &mrt);
+	err = fib_rules_lookup(net->ipv6.mr6_rules_ops,
+			flowi6_to_flowi(&fl6), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;

@@ -711,9 +718,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb,
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ip6mr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, MRT6MSG_WHOLEPKT);
+	ip6mr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			   MRT6MSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }

Hannes Frederic Sowa

2014-01-07 20:20 UTC #3 lore raw

Quoted 21 lines · Hannes Frederic Sowa

Quoted 12 lines · Hannes Frederic Sowa
Quoted 11 lines · Bob Falken
Hello,
I patched, kernel 3.2.53 yesterday,
8774002632packets, and ~9.1TB later, the multicast routing seems to function properly. :)
Kudos for fixing this.
I will keep checking the next days.
As for IPv6 MR, my current setup i.e: the Multicast source, does not support IPv6, so cannot do a check for that natively.
Quoted 7 lines
Quoted 1 line
Unless I can translate IPv4 multicast into IPv6 multicast easily, using some iptable prerouting rules(?).
Thank you for testing!
I'll review the RCU regions again and will prepare the patches (before introduction of ebc0ffae5dfb44 ("fib: RCU conversion of fib_lookup()") and after.

Quoted 5 lines

It seems ip(6)mr_fib_lookup is not always called from rcu section (ndo_start_xmit), so I had to restructure a bit. Could you retest this patch as preparation for a submission to stable? Thanks!
RCU conversion can be done later then.

Broken patch, sorry. Please try this one:

net/ipv4/ipmr.c +18 -6

index 421a249..e5e9071 100644
@@ -157,9 +157,12 @@ static struct mr_table *ipmr_get_table(struct net *net, u32 id)
 static int ipmr_fib_lookup(struct net *net, struct flowi4 *flp4,
 			   struct mr_table **mrt)
 {
-	struct ipmr_result res;
-	struct fib_lookup_arg arg = { .result = &res, };
 	int err;
+	struct ipmr_result res;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
 
 	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
 			       flowi4_to_flowi(flp4), 0, &arg);

@@ -448,16 +451,22 @@ failure:
 
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 {
+	int err;
+	struct ipmr_result res;
 	struct net *net = dev_net(dev);
-	struct mr_table *mrt;
+
+	struct fib_lookup_arg arg = {
+		.result = &res,
+	};
+
 	struct flowi4 fl4 = {
 		.flowi4_oif	= dev->ifindex,
 		.flowi4_iif	= skb->skb_iif,
 		.flowi4_mark	= skb->mark,
 	};
-	int err;
 
-	err = ipmr_fib_lookup(net, &fl4, &mrt);
+	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
+			       flowi4_to_flowi(&fl4), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;

@@ -466,9 +475,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ipmr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, IGMPMSG_WHOLEPKT);
+	ipmr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			  IGMPMSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }

net/ipv6/ip6mr.c +16 -6

index f365310..45ec621 100644
@@ -141,9 +141,12 @@ static struct mr6_table *ip6mr_get_table(struct net *net, u32 id)
 static int ip6mr_fib_lookup(struct net *net, struct flowi6 *flp6,
 			    struct mr6_table **mrt)
 {
-	struct ip6mr_result res;
-	struct fib_lookup_arg arg = { .result = &res, };
 	int err;
+	struct ip6mr_result res;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
 
 	err = fib_rules_lookup(net->ipv6.mr6_rules_ops,
 			       flowi6_to_flowi(flp6), 0, &arg);

@@ -693,16 +696,20 @@ static const struct inet6_protocol pim6_protocol = {
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb,
 				      struct net_device *dev)
 {
+	int err;
+	struct ip6mr_result res;
 	struct net *net = dev_net(dev);
-	struct mr6_table *mrt;
 	struct flowi6 fl6 = {
 		.flowi6_oif	= dev->ifindex,
 		.flowi6_iif	= skb->skb_iif,
 		.flowi6_mark	= skb->mark,
 	};
-	int err;
+	struct fib_lookup_arg arg = {
+		.result = &res,
+	};
 
-	err = ip6mr_fib_lookup(net, &fl6, &mrt);
+	err = fib_rules_lookup(net->ipv6.mr6_rules_ops,
+			flowi6_to_flowi(&fl6), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;

@@ -711,9 +718,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb,
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ip6mr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, MRT6MSG_WHOLEPKT);
+	ip6mr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			   MRT6MSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }

Eric Dumazet

2014-01-07 20:26 UTC #4 lore raw

In reply to Hannes Frederic Sowa

Quoted 20 lines · Hannes Frederic Sowa

It seems ip(6)mr_fib_lookup is not always called from rcu section (ndo_start_xmit), so I had to restructure a bit. Could you retest this patch as preparation for a submission to stable? Thanks!
RCU conversion can be done later then.
net/ipv4/ipmr.c +4 -1
index 421a249..9ae4ae7 100644
 
 
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 {
+	int err;
+	struct ipmr_result res;
 	struct net *net = dev_net(dev);
-	struct mr_table *mrt;
+
+	struct fib_lookup_arg arg = {

net/ipv4/ipmr.c failure: · line 458

Quoted 10 lines

net/ipv4/ipmr.c +4 -1

+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
+
 	struct flowi4 fl4 = {
 		.flowi4_oif	= dev->ifindex,
 		.flowi4_iif	= skb->skb_iif,
 		.flowi4_mark	= skb->mark,
 	};
-	int err;

Technically speaking, I don't think reg_vif_xmit() is enclosed in rcu_read_lock() section.

Quoted 11 lines

net/ipv4/ipmr.c +2 -1

-	err = ipmr_fib_lookup(net, &fl4, &mrt);
+	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
+			       flowi4_to_flowi(&fl4), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;

@@ -466,9 +476,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;

net/ipv4/ipmr.c static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev) · line 487

Quoted 10 lines

net/ipv4/ipmr.c +4 -1

-	ipmr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, IGMPMSG_WHOLEPKT);
+	ipmr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			  IGMPMSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }

Hmm... Are you sure you meant to use FIB_LOOKUP_NOREF in reg_vif_xmit() ?

Hannes Frederic Sowa

2014-01-07 20:29 UTC #5 lore raw

Quoted 52 lines · Eric Dumazet

Quoted 20 lines · Hannes Frederic Sowa

It seems ip(6)mr_fib_lookup is not always called from rcu section (ndo_start_xmit), so I had to restructure a bit. Could you retest this patch as preparation for a submission to stable? Thanks!
RCU conversion can be done later then.
net/ipv4/ipmr.c +4 -1
index 421a249..9ae4ae7 100644
 
 
 static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 {
+	int err;
+	struct ipmr_result res;
 	struct net *net = dev_net(dev);
-	struct mr_table *mrt;
+
+	struct fib_lookup_arg arg = {

net/ipv4/ipmr.c failure: · line 458

Quoted 10 lines

net/ipv4/ipmr.c +4 -1

+		.result = &res,
+		.flags = FIB_LOOKUP_NOREF,
+	};
+
 	struct flowi4 fl4 = {
 		.flowi4_oif	= dev->ifindex,
 		.flowi4_iif	= skb->skb_iif,
 		.flowi4_mark	= skb->mark,
 	};
-	int err;

Technically speaking, I don't think reg_vif_xmit() is enclosed in rcu_read_lock() section.

Quoted 16 lines

net/ipv4/ipmr.c +4 -2

-	err = ipmr_fib_lookup(net, &fl4, &mrt);
+	err = fib_rules_lookup(net->ipv4.mr_rules_ops,
+			       flowi4_to_flowi(&fl4), 0, &arg);
 	if (err < 0) {
 		kfree_skb(skb);
 		return err;

@@ -466,9 +476,12 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 	read_lock(&mrt_lock);
 	dev->stats.tx_bytes += skb->len;
 	dev->stats.tx_packets++;
-	ipmr_cache_report(mrt, skb, mrt->mroute_reg_vif_num, IGMPMSG_WHOLEPKT);
+	ipmr_cache_report(res.mrt, skb, res.mrt->mroute_reg_vif_num,
+			  IGMPMSG_WHOLEPKT);
 	read_unlock(&mrt_lock);
 	kfree_skb(skb);

Quoted 8 lines

net/ipv4/ipmr.c static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev) · line 487
Quoted 5 lines
net/ipv4/ipmr.c +2 -0
+	if (arg.rule)
+		fib_rule_put(arg.rule);
 	return NETDEV_TX_OK;
 }
 
Hmm... Are you sure you meant to use FIB_LOOKUP_NOREF in reg_vif_xmit() ?

Nope, that's the reason for the second patch. Should have mentioned that.

Thanks for the review,

Hannes