| Message ID | 1395139433-19576-4-git-send-email-dingtianhong@huawei.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2014-03-18 at 18:43 +0800, Ding Tianhong wrote: > The debug log in the arp interval should be rate limited, otherwise would > occur spam the log, so convert them. Hi Ding. pr_<level>_ratelimited adds a per-use rate limit control so each line is separately rate limited. Another way to do this is to use the more global net_ratelimit() before each existing pr_<level>. Not suggesting one or the other is right or wrong here, it's just an option. > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > @@ -2131,32 +2131,33 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, > { > struct sk_buff *skb; > > - pr_debug("arp %d on slave %s: dst %pI4 src %pI4\n", > - arp_op, slave_dev->name, &dest_ip, &src_ip); > + pr_debug_ratelimited("arp %d on slave %s: dst %pI4 src %pI4\n", > + arp_op, slave_dev->name, &dest_ip, &src_ip); if (net_ratelimit()) pr_debug(etc...) etc.. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2014-03-18 at 04:01 -0700, Joe Perches wrote: > On Tue, 2014-03-18 at 18:43 +0800, Ding Tianhong wrote: > > The debug log in the arp interval should be rate limited, otherwise would > > occur spam the log, so convert them. [] > Another way to do this is to use the > more global net_ratelimit() before > each existing pr_<level>. > > Not suggesting one or the other is > right or wrong here, it's just an option. Another option is to use the net_<level>_ratelimited functions, but I'm not sure these are used much outside of net/. > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > > > @@ -2131,32 +2131,33 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, > > { > > struct sk_buff *skb; > > > > - pr_debug("arp %d on slave %s: dst %pI4 src %pI4\n", > > - arp_op, slave_dev->name, &dest_ip, &src_ip); > > + pr_debug_ratelimited("arp %d on slave %s: dst %pI4 src %pI4\n", > > + arp_op, slave_dev->name, &dest_ip, &src_ip); > > if (net_ratelimit()) > pr_debug(etc...) or net_dbg_ratelimited(etc...) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/3/18 19:14, Joe Perches wrote: > On Tue, 2014-03-18 at 04:01 -0700, Joe Perches wrote: >> On Tue, 2014-03-18 at 18:43 +0800, Ding Tianhong wrote: >>> The debug log in the arp interval should be rate limited, otherwise would >>> occur spam the log, so convert them. > [] >> Another way to do this is to use the >> more global net_ratelimit() before >> each existing pr_<level>. >> >> Not suggesting one or the other is >> right or wrong here, it's just an option. > > Another option is to use the net_<level>_ratelimited > functions, but I'm not sure these are used much > outside of net/. > >>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >> >>> @@ -2131,32 +2131,33 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, >>> { >>> struct sk_buff *skb; >>> >>> - pr_debug("arp %d on slave %s: dst %pI4 src %pI4\n", >>> - arp_op, slave_dev->name, &dest_ip, &src_ip); >>> + pr_debug_ratelimited("arp %d on slave %s: dst %pI4 src %pI4\n", >>> + arp_op, slave_dev->name, &dest_ip, &src_ip); >> >> if (net_ratelimit()) >> pr_debug(etc...) > > or > > net_dbg_ratelimited(etc...) > > Thanks for these opinion. It looks like the net_ratelimit() is more reasonable. It really need to spend time to distinguish which one is better here. sometimes more choices more headaches. :) Regards Ding > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 6758c2d..8707bb3 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2131,32 +2131,33 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, { struct sk_buff *skb; - pr_debug("arp %d on slave %s: dst %pI4 src %pI4\n", - arp_op, slave_dev->name, &dest_ip, &src_ip); + pr_debug_ratelimited("arp %d on slave %s: dst %pI4 src %pI4\n", + arp_op, slave_dev->name, &dest_ip, &src_ip); skb = arp_create(arp_op, ETH_P_ARP, dest_ip, slave_dev, src_ip, NULL, slave_dev->dev_addr, NULL); if (!skb) { - pr_err("ARP packet allocation failed\n"); + pr_err_ratelimited("ARP packet allocation failed\n"); return; } if (outer->vlan_id) { if (inner->vlan_id) { - pr_debug("inner tag: proto %X vid %X\n", - ntohs(inner->vlan_proto), inner->vlan_id); + pr_debug_ratelimited("inner tag: proto %X vid %X\n", + ntohs(inner->vlan_proto), + inner->vlan_id); skb = __vlan_put_tag(skb, inner->vlan_proto, inner->vlan_id); if (!skb) { - pr_err("failed to insert inner VLAN tag\n"); + pr_err_ratelimited("failed to insert inner VLAN tag\n"); return; } } - pr_debug("outer reg: proto %X vid %X\n", - ntohs(outer->vlan_proto), outer->vlan_id); + pr_debug_ratelimited("outer reg: proto %X vid %X\n", + ntohs(outer->vlan_proto), outer->vlan_id); skb = vlan_put_tag(skb, outer->vlan_proto, outer->vlan_id); if (!skb) { - pr_err("failed to insert outer VLAN tag\n"); + pr_err_ratelimited("failed to insert outer VLAN tag\n"); return; } } @@ -2174,7 +2175,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave) int i; for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i]; i++) { - pr_debug("basa: target %pI4\n", &targets[i]); + pr_debug_ratelimited("basa: target %pI4\n", &targets[i]); inner.vlan_proto = 0; inner.vlan_id = 0; outer.vlan_proto = 0; @@ -2187,9 +2188,10 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave) /* there's no route to target - try to send arp * probe to generate any traffic (arp_validate=0) */ - if (bond->params.arp_validate && net_ratelimit()) - pr_warn("%s: no route to arp_ip_target %pI4 and arp_validate is set\n", - bond->dev->name, &targets[i]); + if (bond->params.arp_validate) + pr_warn_ratelimited("%s: no route to arp_ip_target %pI4 and arp_validate is set\n", + bond->dev->name, + &targets[i]); bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i], 0, &inner, &outer); continue; } @@ -2246,9 +2248,9 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave) rcu_read_unlock(); /* Not our device - skip */ - pr_debug("%s: no path to arp_ip_target %pI4 via rt.dev %s\n", - bond->dev->name, &targets[i], - rt->dst.dev ? rt->dst.dev->name : "NULL"); + pr_debug_ratelimited("%s: no path to arp_ip_target %pI4 via rt.dev %s\n", + bond->dev->name, &targets[i], + rt->dst.dev ? rt->dst.dev->name : "NULL"); ip_rt_put(rt); continue;
The debug log in the arp interval should be rate limited, otherwise would occur spam the log, so convert them. Cc: Jay Vosburgh <fubar@us.ibm.com> Cc: Veaceslav Falico <vfalico@redhat.com> Cc: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> --- drivers/net/bonding/bond_main.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-)