Skip to contentSkip to conversation

printk message cleanup's

PATCH In review Stephen Hemminger 3 participants 8 messages Aug 29, 2014 – Sep 2, 2014

Subsystems (7)
Networking Network drivers IPv4/IPv6 TCP +3 more subsystems
lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

cover printk message cleanup's

To support space and clutter reduction get rid of some useless messages on boot, and be more precise about message level, and don't use bare printk in network code.

author
#1 lore raw

PATCH 1/5 net: fix message priorities

Using obsolete socket type should be notice not info level. Registering is normal and should only be debug level.

Signed-off-by Stephen Hemminger <[email protected]>
net/socket.c +4 -4
@@ -1276,8 +1276,8 @@ int __sock_create(struct net *net, int f
 		static int warned;
 		if (!warned) {
 			warned = 1;
-			pr_info("%s uses obsolete (PF_INET,SOCK_PACKET)\n",
-				current->comm);
+			pr_notice("%s uses obsolete (PF_INET,SOCK_PACKET)\n",
+				  current->comm);
 		}
 		family = PF_PACKET;
 	}

@@ -2624,7 +2624,7 @@ int sock_register(const struct net_proto
 	}
 	spin_unlock(&net_family_lock);
 
-	pr_info("NET: Registered protocol family %d\n", ops->family);
+	pr_debug("NET: Registered protocol family %d\n", ops->family);
 	return err;
 }
 EXPORT_SYMBOL(sock_register);

@@ -2652,7 +2652,7 @@ void sock_unregister(int family)
 
 	synchronize_rcu();
 
-	pr_info("NET: Unregistered protocol family %d\n", family);
+	pr_debug("NET: Unregistered protocol family %d\n", family);
 }
 EXPORT_SYMBOL(sock_unregister);
author
#2 lore raw

PATCH 2/5 tcp: silence useless debug message

In reply to Stephen Hemminger

This message isn't really needed.

Signed-off-by Stephen Hemminger <[email protected]>
net/ipv4/tcp_cong.c +1 -1
@@ -51,7 +51,7 @@ int tcp_register_congestion_control(stru
 		ret = -EEXIST;
 	} else {
 		list_add_tail_rcu(&ca->list, &tcp_cong_list);
-		pr_info("%s registered\n", ca->name);
+		pr_debug("%s registered\n", ca->name);
 	}
 	spin_unlock(&tcp_cong_list_lock);
author
#3 lore raw

PATCH 3/5 ipv6: silence tunnel loading messages

In reply to Stephen Hemminger

Chatter on boot is not necessary, change it to debug.

Signed-off-by Stephen Hemminger <[email protected]>
net/ipv6/ip6_gre.c +1 -1
@@ -1678,7 +1678,7 @@ static int __init ip6gre_init(void)
 {
 	int err;
 
-	pr_info("GRE over IPv6 tunneling driver\n");
+	pr_debug("GRE over IPv6 tunneling driver\n");
 
 	err = register_pernet_device(&ip6gre_net_ops);
 	if (err < 0)
net/ipv6/mip6.c +1 -1
@@ -482,7 +482,7 @@ static const struct xfrm_type mip6_rthdr
 
 static int __init mip6_init(void)
 {
-	pr_info("Mobile IPv6\n");
+	pr_debug("Mobile IPv6\n");
 
 	if (xfrm_register_type(&mip6_destopt_type, AF_INET6) < 0) {
 		pr_info("%s: can't add xfrm type(destopt)\n", __func__);
net/ipv6/sit.c +1 -1
@@ -1796,7 +1796,7 @@ static int __init sit_init(void)
 {
 	int err;
 
-	pr_info("IPv6 over IPv4 tunneling driver\n");
+	pr_debug("IPv6 over IPv4 tunneling driver\n");
 
 	err = register_pernet_device(&sit_net_ops);
 	if (err < 0)
author
#4 lore raw

PATCH 4/5 xfrm: replace printk with pr_ macro

In reply to Stephen Hemminger

Use standard macro's for notices.
Silence useless debug message on module load.

Signed-off-by Stephen Hemminger <[email protected]>
net/xfrm/xfrm_user.c +3 -5
@@ -2582,8 +2582,7 @@ static int xfrm_send_state_notify(struct
 	case XFRM_MSG_FLUSHSA:
 		return xfrm_notify_sa_flush(c);
 	default:
-		printk(KERN_NOTICE "xfrm_user: Unknown SA event %d\n",
-		       c->event);
+		pr_notice("xfrm_user: Unknown SA event %d\n", c->event);
 		break;
 	}
 

@@ -2875,8 +2874,7 @@ static int xfrm_send_policy_notify(struc
 	case XFRM_MSG_POLEXPIRE:
 		return xfrm_exp_policy_notify(xp, dir, c);
 	default:
-		printk(KERN_NOTICE "xfrm_user: Unknown Policy event %d\n",
-		       c->event);
+		pr_notice("xfrm_user: Unknown Policy event %d\n", c->event);
 	}
 
 	return 0;

@@ -3031,7 +3029,7 @@ static int __init xfrm_user_init(void)
 {
 	int rv;
 
-	printk(KERN_INFO "Initializing XFRM netlink socket\n");
+	pr_debug("Initializing XFRM netlink socket\n");
 
 	rv = register_pernet_subsys(&xfrm_user_net_ops);
 	if (rv < 0)
author
#5 lore raw

PATCH 5/5 sched: replace bare printks

In reply to Stephen Hemminger

Don't use bare printk for error messages.
Make GRED error an info level message rather than debug which is likely to be suppressed.

Signed-off-by Stephen Hemminger <[email protected]>
net/sched/act_ipt.c +2 -2
@@ -290,10 +290,10 @@ static int __init ipt_init_module(void)
 
 	ret1 = tcf_register_action(&act_xt_ops, IPT_TAB_MASK);
 	if (ret1 < 0)
-		printk("Failed to load xt action\n");
+		pr_err("Failed to load xt action\n");
 	ret2 = tcf_register_action(&act_ipt_ops, IPT_TAB_MASK);
 	if (ret2 < 0)
-		printk("Failed to load ipt action\n");
+		pr_err("Failed to load ipt action\n");
 
 	if (ret1 < 0 && ret2 < 0) {
 		return ret1;
net/sched/act_mirred.c +1 -1
@@ -140,7 +140,7 @@ static int tcf_mirred(struct sk_buff *sk
 
 	dev = m->tcfm_dev;
 	if (!dev) {
-		printk_once(KERN_NOTICE "tc mirred: target device is gone\n");
+		netdev_notice(dev, "tc mirred: target device is gone\n");
 		goto out;
 	}
net/sched/sch_gred.c +2 -2
@@ -456,8 +456,8 @@ static int gred_change(struct Qdisc *sch
 			if (table->tab[table->def])
 				def_prio = table->tab[table->def]->prio;
 
-			printk(KERN_DEBUG "GRED: DP %u does not have a prio "
-			       "setting default to %d\n", ctl->DP, def_prio);
+			pr_info("GRED: DP %u does not have a prio "
+				"setting default to %d\n", ctl->DP, def_prio);
 
 			prio = def_prio;
 		} else
#6 lore raw

Re: [PATCH net-next 5/5] sched: replace bare printks

Am Freitag, 29. August 2014, 00:09:23 schrieb Stephen Hemminger:

Quoted 25 lines

Don't use bare printk for error messages.
Make GRED error an info level message rather than debug which is likely to be suppressed.

Signed-off-by Stephen Hemminger <[email protected]>
net/sched/act_ipt.c +2 -2
@@ -290,10 +290,10 @@ static int __init ipt_init_module(void)
 
 	ret1 = tcf_register_action(&act_xt_ops, IPT_TAB_MASK);
 	if (ret1 < 0)
-		printk("Failed to load xt action\n");
+		pr_err("Failed to load xt action\n");
 	ret2 = tcf_register_action(&act_ipt_ops, IPT_TAB_MASK);
 	if (ret2 < 0)
-		printk("Failed to load ipt action\n");
+		pr_err("Failed to load ipt action\n");
 
 	if (ret1 < 0 && ret2 < 0) {
 		return ret1;

net/sched/act_mirred.c static int tcf_mirred(struct sk_buff *sk · line 145

Quoted 7 lines
net/sched/act_mirred.c +1 -1
@@ -140,7 +140,7 @@ static int tcf_mirred(struct sk_buff *sk
 
 	dev = m->tcfm_dev;
 	if (!dev) {
-		printk_once(KERN_NOTICE "tc mirred: target device is gone\n");
+		netdev_notice(dev, "tc mirred: target device is gone\n");
 		goto out;
 	}

At my first look, I was wondering whether this could work as 'dev' is not a valid pointer inside the if statement body. But after trying to trace to call down, I found that it ends up as
printk("%s(NULL net_device): %pV" ...
within __netdev_printk.
Is this the desired output in this case?
Or did I miss anything?

Quoted 5 lines

net/sched/sch_gred.c static int gred_change(struct Qdisc *sch · line 463

Quoted 10 lines
net/sched/sch_gred.c +2 -2
@@ -456,8 +456,8 @@ static int gred_change(struct Qdisc *sch
 			if (table->tab[table->def])
 				def_prio = table->tab[table->def]->prio;
 
-			printk(KERN_DEBUG "GRED: DP %u does not have a prio "
-			       "setting default to %d\n", ctl->DP, def_prio);
+			pr_info("GRED: DP %u does not have a prio "
+				"setting default to %d\n", ctl->DP, def_prio);
 
 			prio = def_prio;
 		} else

Best regards,
Michael

maintainer
#7 lore raw

Re: [PATCH net-next 5/5] sched: replace bare printks

In reply to Stephen Hemminger

From: Stephen Hemminger <[email protected]>
Date: Fri, 29 Aug 2014 00:09:23 -0700

net/sched/act_mirred.c static int tcf_mirred(struct sk_buff *sk · line 144

Quoted 7 lines
net/sched/act_mirred.c +1 -1
@@ -140,7 +140,7 @@ static int tcf_mirred(struct sk_buff *sk
 
 	dev = m->tcfm_dev;
 	if (!dev) {
-		printk_once(KERN_NOTICE "tc mirred: target device is gone\n");
+		netdev_notice(dev, "tc mirred: target device is gone\n");
 		goto out;

As pointed out by others, doing netdev_*() in a path where we know 'dev' is NULL doesn't make much sense.

Please respin this series with this adjusted.

Thanks.