| Message ID | 20180311194104.GA17450@seema-Inspiron-15-3567 |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [v2] net: netfilter: Replace printk() with appropriate pr_*() macro | expand |
On Mon, 2018-03-12 at 01:11 +0530, Arushi Singhal wrote: > Using pr_<loglevel>() is more concise than > printk(KERN_<LOGLEVEL>). > Replace printks having a log level with the appropriate > pr_*() macros. > > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com> > --- > changes in v2 > *in v1 printk() were replaced with netdev_*() > net/netfilter/nf_conntrack_acct.c | 2 +- > net/netfilter/nf_conntrack_ecache.c | 2 +- > net/netfilter/nf_conntrack_timestamp.c | 2 +- > net/netfilter/nf_nat_core.c | 2 +- > net/netfilter/nfnetlink_queue.c | 4 ++-- > 5 files changed, 6 insertions(+), 6 deletions(-) None of these files have a #define for pr_fmt so this should be OK. Perhaps coalesce the formats and remove the unnecessary periods too. > diff --git a/net/netfilter/nf_conntrack_acct.c b/net/netfilter/nf_conntrack_acct.c [] > @@ -80,7 +80,7 @@ static int nf_conntrack_acct_init_sysctl(struct net *net) > net->ct.acct_sysctl_header = register_net_sysctl(net, "net/netfilter", > table); > if (!net->ct.acct_sysctl_header) { > - printk(KERN_ERR "nf_conntrack_acct: can't register to sysctl.\n"); > + pr_err("nf_conntrack_acct: can't register to sysctl.\n"); > goto out_register; > } > return 0; etc... > diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c [] > @@ -834,8 +834,8 @@ nfqnl_mangle(void *data, int data_len, struct nf_queue_entry *e, int diff) > nskb = skb_copy_expand(e->skb, skb_headroom(e->skb), > diff, GFP_ATOMIC); > if (!nskb) { > - printk(KERN_WARNING "nf_queue: OOM " > - "in mangle, dropping packet\n"); > + pr_warn("nf_queue: OOM " > + "in mangle, dropping packet\n"); > return -ENOMEM; > } > kfree_skb(e->skb); -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Joe, On Sun, Mar 11, 2018 at 12:52:41PM -0700, Joe Perches wrote: > On Mon, 2018-03-12 at 01:11 +0530, Arushi Singhal wrote: > > Using pr_<loglevel>() is more concise than > > printk(KERN_<LOGLEVEL>). > > Replace printks having a log level with the appropriate > > pr_*() macros. > > > > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com> > > --- > > changes in v2 > > *in v1 printk() were replaced with netdev_*() > > > net/netfilter/nf_conntrack_acct.c | 2 +- > > net/netfilter/nf_conntrack_ecache.c | 2 +- > > net/netfilter/nf_conntrack_timestamp.c | 2 +- > > net/netfilter/nf_nat_core.c | 2 +- > > net/netfilter/nfnetlink_queue.c | 4 ++-- > > 5 files changed, 6 insertions(+), 6 deletions(-) > > None of these files have a #define for pr_fmt so this > should be OK. I think Arushi could add pr_fmt in the same go, so we skip another follow up patch for this. @Arushi: I suggested this in my previous email, please have a look. This would also probably allows us to save the line break in the error message, which IIRC is not a good practise either, eg. pr_warn("nf_queue: OOM " "in mangle, dropping packet\n"); > Perhaps coalesce the formats and remove the unnecessary periods too. Yes, removing periods seems fine. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 12, 2018 at 03:56:15AM +0530, Arushi Singhal wrote: > On Mon, Mar 12, 2018 at 2:17 AM, Pablo Neira Ayuso <pablo@netfilter.org> > wrote: > > > Hi Joe, > > > > On Sun, Mar 11, 2018 at 12:52:41PM -0700, Joe Perches wrote: > > > On Mon, 2018-03-12 at 01:11 +0530, Arushi Singhal wrote: > > > > Using pr_<loglevel>() is more concise than > > > > printk(KERN_<LOGLEVEL>). > > > > Replace printks having a log level with the appropriate > > > > pr_*() macros. > > > > > > > > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com> > > > > --- > > > > changes in v2 > > > > *in v1 printk() were replaced with netdev_*() > > > > > > > net/netfilter/nf_conntrack_acct.c | 2 +- > > > > net/netfilter/nf_conntrack_ecache.c | 2 +- > > > > net/netfilter/nf_conntrack_timestamp.c | 2 +- > > > > net/netfilter/nf_nat_core.c | 2 +- > > > > net/netfilter/nfnetlink_queue.c | 4 ++-- > > > > 5 files changed, 6 insertions(+), 6 deletions(-) > > > > > > None of these files have a #define for pr_fmt so this > > > should be OK. > > > > I think Arushi could add pr_fmt in the same go, so we skip another > > follow up patch for this. @Arushi: I suggested this in my previous > > email, please have a look. > > > > Hello Pablo > > Should I send two patches, one with the conversion of printk() to pr_() and > another for defining pr_fmt(). > > Or > > only one patch with all the changes? I think adding pr_fmt and use pr_() belongs to the same logical change, so one patch for this is fine. Thanks Arushi. P.S: Please, just send your patch netfilter-devel@vger.kernel.org next time, no need to Cc every list. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Arushi Singhal <arushisinghal19971997@gmail.com> wrote: > On Mon, Mar 12, 2018 at 2:17 AM, Pablo Neira Ayuso <pablo@netfilter.org> > wrote: > > > Hi Joe, > > > > On Sun, Mar 11, 2018 at 12:52:41PM -0700, Joe Perches wrote: > > > On Mon, 2018-03-12 at 01:11 +0530, Arushi Singhal wrote: > > > > Using pr_<loglevel>() is more concise than > > > > printk(KERN_<LOGLEVEL>). > > > > Replace printks having a log level with the appropriate > > > > pr_*() macros. > > > > > > > > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com> > > > > --- > > > > changes in v2 > > > > *in v1 printk() were replaced with netdev_*() > > > > > > > net/netfilter/nf_conntrack_acct.c | 2 +- > > > > net/netfilter/nf_conntrack_ecache.c | 2 +- > > > > net/netfilter/nf_conntrack_timestamp.c | 2 +- > > > > net/netfilter/nf_nat_core.c | 2 +- > > > > net/netfilter/nfnetlink_queue.c | 4 ++-- > > > > 5 files changed, 6 insertions(+), 6 deletions(-) > > > > > > None of these files have a #define for pr_fmt so this > > > should be OK. > > > > I think Arushi could add pr_fmt in the same go, so we skip another > > follow up patch for this. @Arushi: I suggested this in my previous > > email, please have a look. > > > > Hello Pablo > > Should I send two patches, one with the conversion of printk() to pr_() and > another for defining pr_fmt(). > > Or > > only one patch with all the changes? Both in one, it reduces code churn. With pr_* + pr_fmt, the module name will be prefixed automatically, see e.g. commit e016c5e43db51875c2b541b59bd217494d213174 as an example. > > This would also probably allows us to save the line break in the error > > message, which IIRC is not a good practise either, eg. > > > > pr_warn("nf_queue: OOM " > > "in mangle, dropping packet\n"); > > > > > Perhaps coalesce the formats and remove the unnecessary periods too. The above message should be removed, its useless (on oom allocator already warns). -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" 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/net/netfilter/nf_conntrack_acct.c b/net/netfilter/nf_conntrack_acct.c index 8669167..b1c3286 100644 --- a/net/netfilter/nf_conntrack_acct.c +++ b/net/netfilter/nf_conntrack_acct.c @@ -80,7 +80,7 @@ static int nf_conntrack_acct_init_sysctl(struct net *net) net->ct.acct_sysctl_header = register_net_sysctl(net, "net/netfilter", table); if (!net->ct.acct_sysctl_header) { - printk(KERN_ERR "nf_conntrack_acct: can't register to sysctl.\n"); + pr_err("nf_conntrack_acct: can't register to sysctl.\n"); goto out_register; } return 0; diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c index caac41a..21a3048 100644 --- a/net/netfilter/nf_conntrack_ecache.c +++ b/net/netfilter/nf_conntrack_ecache.c @@ -372,7 +372,7 @@ static int nf_conntrack_event_init_sysctl(struct net *net) net->ct.event_sysctl_header = register_net_sysctl(net, "net/netfilter", table); if (!net->ct.event_sysctl_header) { - printk(KERN_ERR "nf_ct_event: can't register to sysctl.\n"); + pr_err("nf_ct_event: can't register to sysctl.\n"); goto out_register; } return 0; diff --git a/net/netfilter/nf_conntrack_timestamp.c b/net/netfilter/nf_conntrack_timestamp.c index 4c4734b..f32cc86 100644 --- a/net/netfilter/nf_conntrack_timestamp.c +++ b/net/netfilter/nf_conntrack_timestamp.c @@ -58,7 +58,7 @@ static int nf_conntrack_tstamp_init_sysctl(struct net *net) net->ct.tstamp_sysctl_header = register_net_sysctl(net, "net/netfilter", table); if (!net->ct.tstamp_sysctl_header) { - printk(KERN_ERR "nf_ct_tstamp: can't register to sysctl.\n"); + pr_err("nf_ct_tstamp: can't register to sysctl.\n"); goto out_register; } return 0; diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c index 6c38421..dcda5ac 100644 --- a/net/netfilter/nf_nat_core.c +++ b/net/netfilter/nf_nat_core.c @@ -814,7 +814,7 @@ static int __init nf_nat_init(void) ret = nf_ct_extend_register(&nat_extend); if (ret < 0) { nf_ct_free_hashtable(nf_nat_bysource, nf_nat_htable_size); - printk(KERN_ERR "nf_nat_core: Unable to register extension\n"); + pr_err("nf_nat_core: Unable to register extension\n"); return ret; } diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c index 8bba231..f5ddab1 100644 --- a/net/netfilter/nfnetlink_queue.c +++ b/net/netfilter/nfnetlink_queue.c @@ -834,8 +834,8 @@ nfqnl_mangle(void *data, int data_len, struct nf_queue_entry *e, int diff) nskb = skb_copy_expand(e->skb, skb_headroom(e->skb), diff, GFP_ATOMIC); if (!nskb) { - printk(KERN_WARNING "nf_queue: OOM " - "in mangle, dropping packet\n"); + pr_warn("nf_queue: OOM " + "in mangle, dropping packet\n"); return -ENOMEM; } kfree_skb(e->skb);
Using pr_<loglevel>() is more concise than printk(KERN_<LOGLEVEL>). Replace printks having a log level with the appropriate pr_*() macros. Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com> --- changes in v2 *in v1 printk() were replaced with netdev_*() net/netfilter/nf_conntrack_acct.c | 2 +- net/netfilter/nf_conntrack_ecache.c | 2 +- net/netfilter/nf_conntrack_timestamp.c | 2 +- net/netfilter/nf_nat_core.c | 2 +- net/netfilter/nfnetlink_queue.c | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-)