netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] slight optimization and avoid spam for bond fast path
@ 2014-03-25  7:03 Ding Tianhong
  2014-03-25  7:03 ` [PATCH net-next 1/3] bonding: slight optimization for bond xmit path Ding Tianhong
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ding Tianhong @ 2014-03-25  7:03 UTC (permalink / raw)
  To: fubar, vfalico, andy, joe, kaber; +Cc: davem, netdev

Ding Tianhong (3):
  bonding: slight optimization for bond xmit path
  bonding: ratelimit pr_err() for bond xmit broadcast
  bonding: add ratelimit for bond_arp_rcv()

 drivers/net/bonding/bond_main.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

-- 
1.8.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next 1/3] bonding: slight optimization for bond xmit path
  2014-03-25  7:03 [PATCH net-next 0/3] slight optimization and avoid spam for bond fast path Ding Tianhong
@ 2014-03-25  7:03 ` Ding Tianhong
  2014-03-25  7:03 ` [PATCH net-next 2/3] bonding: ratelimit pr_err() for bond xmit broadcast Ding Tianhong
  2014-03-25  7:03 ` [PATCH net-next 3/3] bonding: add ratelimit for bond_arp_rcv() Ding Tianhong
  2 siblings, 0 replies; 8+ messages in thread
From: Ding Tianhong @ 2014-03-25  7:03 UTC (permalink / raw)
  To: fubar, vfalico, andy, joe, kaber; +Cc: davem, netdev

Add unlikely() micro to the unlikely conditions in the bond
xmit path for slight optimization.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e717db3..ee17c24 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2957,7 +2957,7 @@ static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
 	fk->ports = 0;
 	noff = skb_network_offset(skb);
 	if (skb->protocol == htons(ETH_P_IP)) {
-		if (!pskb_may_pull(skb, noff + sizeof(*iph)))
+		if (unlikely(!pskb_may_pull(skb, noff + sizeof(*iph))))
 			return false;
 		iph = ip_hdr(skb);
 		fk->src = iph->saddr;
@@ -2966,7 +2966,7 @@ static bool bond_flow_dissect(struct bonding *bond, struct sk_buff *skb,
 		if (!ip_is_fragment(iph))
 			proto = iph->protocol;
 	} else if (skb->protocol == htons(ETH_P_IPV6)) {
-		if (!pskb_may_pull(skb, noff + sizeof(*iph6)))
+		if (unlikely(!pskb_may_pull(skb, noff + sizeof(*iph6))))
 			return false;
 		iph6 = ipv6_hdr(skb);
 		fk->src = (__force __be32)ipv6_addr_hash(&iph6->saddr);
@@ -3768,7 +3768,7 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * If we risk deadlock from transmitting this in the
 	 * netpoll path, tell netpoll to queue the frame for later tx
 	 */
-	if (is_netpoll_tx_blocked(dev))
+	if (unlikely(is_netpoll_tx_blocked(dev)))
 		return NETDEV_TX_BUSY;
 
 	rcu_read_lock();
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 2/3] bonding: ratelimit pr_err() for bond xmit broadcast
  2014-03-25  7:03 [PATCH net-next 0/3] slight optimization and avoid spam for bond fast path Ding Tianhong
  2014-03-25  7:03 ` [PATCH net-next 1/3] bonding: slight optimization for bond xmit path Ding Tianhong
@ 2014-03-25  7:03 ` Ding Tianhong
  2014-03-25  7:19   ` Joe Perches
  2014-03-25  7:03 ` [PATCH net-next 3/3] bonding: add ratelimit for bond_arp_rcv() Ding Tianhong
  2 siblings, 1 reply; 8+ messages in thread
From: Ding Tianhong @ 2014-03-25  7:03 UTC (permalink / raw)
  To: fubar, vfalico, andy, joe, kaber; +Cc: davem, netdev

It may spam if the system is out of the memory, add ratelimit for it.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ee17c24..9e8b888 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3656,8 +3656,8 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
 			struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
 
 			if (!skb2) {
-				pr_err("%s: Error: bond_xmit_broadcast(): skb_clone() failed\n",
-				       bond_dev->name);
+				pr_err_ratelimited("%s: Error: bond_xmit_broadcast(): skb_clone() failed\n",
+						   bond_dev->name);
 				continue;
 			}
 			/* bond_dev_queue_xmit always returns 0 */
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 3/3] bonding: add ratelimit for bond_arp_rcv()
  2014-03-25  7:03 [PATCH net-next 0/3] slight optimization and avoid spam for bond fast path Ding Tianhong
  2014-03-25  7:03 ` [PATCH net-next 1/3] bonding: slight optimization for bond xmit path Ding Tianhong
  2014-03-25  7:03 ` [PATCH net-next 2/3] bonding: ratelimit pr_err() for bond xmit broadcast Ding Tianhong
@ 2014-03-25  7:03 ` Ding Tianhong
  2014-03-25  7:34   ` Veaceslav Falico
  2 siblings, 1 reply; 8+ messages in thread
From: Ding Tianhong @ 2014-03-25  7:03 UTC (permalink / raw)
  To: fubar, vfalico, andy, joe, kaber; +Cc: davem, netdev

Add ratelimit to avoid spam when processing the arp package.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/bonding/bond_main.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9e8b888..7328a3f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2239,13 +2239,15 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
 	int i;
 
 	if (!sip || !bond_has_this_ip(bond, tip)) {
-		pr_debug("bva: sip %pI4 tip %pI4 not found\n", &sip, &tip);
+		pr_debug_ratelimited("bva: sip %pI4 tip %pI4 not found\n",
+				     &sip, &tip);
 		return;
 	}
 
 	i = bond_get_targets_ip(bond->params.arp_targets, sip);
 	if (i == -1) {
-		pr_debug("bva: sip %pI4 not found in targets\n", &sip);
+		pr_debug_ratelimited("bva: sip %pI4 not found in targets\n",
+				     &sip);
 		return;
 	}
 	slave->last_rx = jiffies;
@@ -2272,8 +2274,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
 
 	alen = arp_hdr_len(bond->dev);
 
-	pr_debug("bond_arp_rcv: bond %s skb->dev %s\n",
-		 bond->dev->name, skb->dev->name);
+	pr_debug_ratelimited("bond_arp_rcv: bond %s skb->dev %s\n",
+			     bond->dev->name, skb->dev->name);
 
 	if (alen > skb_headlen(skb)) {
 		arp = kmalloc(alen, GFP_ATOMIC);
@@ -2297,10 +2299,11 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
 	arp_ptr += 4 + bond->dev->addr_len;
 	memcpy(&tip, arp_ptr, 4);
 
-	pr_debug("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
-		 bond->dev->name, slave->dev->name, bond_slave_state(slave),
-		 bond->params.arp_validate, slave_do_arp_validate(bond, slave),
-		 &sip, &tip);
+	pr_debug_ratelimited("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
+			     bond->dev->name, slave->dev->name,
+			     bond_slave_state(slave),
+			     bond->params.arp_validate,
+			     slave_do_arp_validate(bond, slave), &sip, &tip);
 
 	curr_active_slave = rcu_dereference(bond->curr_active_slave);
 
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 2/3] bonding: ratelimit pr_err() for bond xmit broadcast
  2014-03-25  7:03 ` [PATCH net-next 2/3] bonding: ratelimit pr_err() for bond xmit broadcast Ding Tianhong
@ 2014-03-25  7:19   ` Joe Perches
  2014-03-25  8:12     ` Ding Tianhong
  0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2014-03-25  7:19 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: fubar, vfalico, andy, kaber, davem, netdev

On Tue, 2014-03-25 at 15:03 +0800, Ding Tianhong wrote:
> It may spam if the system is out of the memory, add ratelimit for it.

Sorry, I didn't follow whatever discussion occurred here.

I thought you were going to use net_err_ratelimited
for these?

> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
[]
> @@ -3656,8 +3656,8 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
> -				pr_err("%s: Error: bond_xmit_broadcast(): skb_clone() failed\n",
> -				       bond_dev->name);
> +				pr_err_ratelimited("%s: Error: bond_xmit_broadcast(): skb_clone() failed\n",
> +						   bond_dev->name);

also btw, it's better I think to use
%s, __func__ instead of embedding
explicit function names

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 3/3] bonding: add ratelimit for bond_arp_rcv()
  2014-03-25  7:03 ` [PATCH net-next 3/3] bonding: add ratelimit for bond_arp_rcv() Ding Tianhong
@ 2014-03-25  7:34   ` Veaceslav Falico
  2014-03-25  8:17     ` Ding Tianhong
  0 siblings, 1 reply; 8+ messages in thread
From: Veaceslav Falico @ 2014-03-25  7:34 UTC (permalink / raw)
  To: Ding Tianhong; +Cc: fubar, andy, joe, kaber, davem, netdev

On Tue, Mar 25, 2014 at 03:03:57PM +0800, Ding Tianhong wrote:
>Add ratelimit to avoid spam when processing the arp package.

You've ratelimited here only debug statements. As with the another
patchset, I think that it's a bad idea for debugging - if the user turns
debugging on he wants to see all messages, and otherwise he won't ever see
any.

>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bond_main.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 9e8b888..7328a3f 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2239,13 +2239,15 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
> 	int i;
>
> 	if (!sip || !bond_has_this_ip(bond, tip)) {
>-		pr_debug("bva: sip %pI4 tip %pI4 not found\n", &sip, &tip);
>+		pr_debug_ratelimited("bva: sip %pI4 tip %pI4 not found\n",
>+				     &sip, &tip);
> 		return;
> 	}
>
> 	i = bond_get_targets_ip(bond->params.arp_targets, sip);
> 	if (i == -1) {
>-		pr_debug("bva: sip %pI4 not found in targets\n", &sip);
>+		pr_debug_ratelimited("bva: sip %pI4 not found in targets\n",
>+				     &sip);
> 		return;
> 	}
> 	slave->last_rx = jiffies;
>@@ -2272,8 +2274,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>
> 	alen = arp_hdr_len(bond->dev);
>
>-	pr_debug("bond_arp_rcv: bond %s skb->dev %s\n",
>-		 bond->dev->name, skb->dev->name);
>+	pr_debug_ratelimited("bond_arp_rcv: bond %s skb->dev %s\n",
>+			     bond->dev->name, skb->dev->name);
>
> 	if (alen > skb_headlen(skb)) {
> 		arp = kmalloc(alen, GFP_ATOMIC);
>@@ -2297,10 +2299,11 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
> 	arp_ptr += 4 + bond->dev->addr_len;
> 	memcpy(&tip, arp_ptr, 4);
>
>-	pr_debug("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
>-		 bond->dev->name, slave->dev->name, bond_slave_state(slave),
>-		 bond->params.arp_validate, slave_do_arp_validate(bond, slave),
>-		 &sip, &tip);
>+	pr_debug_ratelimited("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
>+			     bond->dev->name, slave->dev->name,
>+			     bond_slave_state(slave),
>+			     bond->params.arp_validate,
>+			     slave_do_arp_validate(bond, slave), &sip, &tip);
>
> 	curr_active_slave = rcu_dereference(bond->curr_active_slave);
>
>-- 
>1.8.0
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 2/3] bonding: ratelimit pr_err() for bond xmit broadcast
  2014-03-25  7:19   ` Joe Perches
@ 2014-03-25  8:12     ` Ding Tianhong
  0 siblings, 0 replies; 8+ messages in thread
From: Ding Tianhong @ 2014-03-25  8:12 UTC (permalink / raw)
  To: Joe Perches; +Cc: fubar, vfalico, andy, kaber, davem, netdev

On 2014/3/25 15:19, Joe Perches wrote:
> On Tue, 2014-03-25 at 15:03 +0800, Ding Tianhong wrote:
>> It may spam if the system is out of the memory, add ratelimit for it.
> 
> Sorry, I didn't follow whatever discussion occurred here.
> 
> I thought you were going to use net_err_ratelimited
> for these?
> 
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> []
>> @@ -3656,8 +3656,8 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
>> -				pr_err("%s: Error: bond_xmit_broadcast(): skb_clone() failed\n",
>> -				       bond_dev->name);
>> +				pr_err_ratelimited("%s: Error: bond_xmit_broadcast(): skb_clone() failed\n",
>> +						   bond_dev->name);
> 
> also btw, it's better I think to use
> %s, __func__ instead of embedding
> explicit function names
> 
> 
> 
Yes, maybe I still didn't clear when to use net_err_ratelimited, thanks.

Ding

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 3/3] bonding: add ratelimit for bond_arp_rcv()
  2014-03-25  7:34   ` Veaceslav Falico
@ 2014-03-25  8:17     ` Ding Tianhong
  0 siblings, 0 replies; 8+ messages in thread
From: Ding Tianhong @ 2014-03-25  8:17 UTC (permalink / raw)
  To: Veaceslav Falico; +Cc: fubar, andy, joe, kaber, davem, netdev

On 2014/3/25 15:34, Veaceslav Falico wrote:
> On Tue, Mar 25, 2014 at 03:03:57PM +0800, Ding Tianhong wrote:
>> Add ratelimit to avoid spam when processing the arp package.
> 
> You've ratelimited here only debug statements. As with the another
> patchset, I think that it's a bad idea for debugging - if the user turns
> debugging on he wants to see all messages, and otherwise he won't ever see
> any.
> 

more clear, thanks.

Ding

>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 19 +++++++++++--------
>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 9e8b888..7328a3f 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2239,13 +2239,15 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
>>     int i;
>>
>>     if (!sip || !bond_has_this_ip(bond, tip)) {
>> -        pr_debug("bva: sip %pI4 tip %pI4 not found\n", &sip, &tip);
>> +        pr_debug_ratelimited("bva: sip %pI4 tip %pI4 not found\n",
>> +                     &sip, &tip);
>>         return;
>>     }
>>
>>     i = bond_get_targets_ip(bond->params.arp_targets, sip);
>>     if (i == -1) {
>> -        pr_debug("bva: sip %pI4 not found in targets\n", &sip);
>> +        pr_debug_ratelimited("bva: sip %pI4 not found in targets\n",
>> +                     &sip);
>>         return;
>>     }
>>     slave->last_rx = jiffies;
>> @@ -2272,8 +2274,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>>
>>     alen = arp_hdr_len(bond->dev);
>>
>> -    pr_debug("bond_arp_rcv: bond %s skb->dev %s\n",
>> -         bond->dev->name, skb->dev->name);
>> +    pr_debug_ratelimited("bond_arp_rcv: bond %s skb->dev %s\n",
>> +                 bond->dev->name, skb->dev->name);
>>
>>     if (alen > skb_headlen(skb)) {
>>         arp = kmalloc(alen, GFP_ATOMIC);
>> @@ -2297,10 +2299,11 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>>     arp_ptr += 4 + bond->dev->addr_len;
>>     memcpy(&tip, arp_ptr, 4);
>>
>> -    pr_debug("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
>> -         bond->dev->name, slave->dev->name, bond_slave_state(slave),
>> -         bond->params.arp_validate, slave_do_arp_validate(bond, slave),
>> -         &sip, &tip);
>> +    pr_debug_ratelimited("bond_arp_rcv: %s %s/%d av %d sv %d sip %pI4 tip %pI4\n",
>> +                 bond->dev->name, slave->dev->name,
>> +                 bond_slave_state(slave),
>> +                 bond->params.arp_validate,
>> +                 slave_do_arp_validate(bond, slave), &sip, &tip);
>>
>>     curr_active_slave = rcu_dereference(bond->curr_active_slave);
>>
>> -- 
>> 1.8.0
>>
>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-03-25  8:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-25  7:03 [PATCH net-next 0/3] slight optimization and avoid spam for bond fast path Ding Tianhong
2014-03-25  7:03 ` [PATCH net-next 1/3] bonding: slight optimization for bond xmit path Ding Tianhong
2014-03-25  7:03 ` [PATCH net-next 2/3] bonding: ratelimit pr_err() for bond xmit broadcast Ding Tianhong
2014-03-25  7:19   ` Joe Perches
2014-03-25  8:12     ` Ding Tianhong
2014-03-25  7:03 ` [PATCH net-next 3/3] bonding: add ratelimit for bond_arp_rcv() Ding Tianhong
2014-03-25  7:34   ` Veaceslav Falico
2014-03-25  8:17     ` Ding Tianhong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).