* [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
* 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 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
* [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 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 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).