On 27-10-2014 20:23, Pablo Neira Ayuso wrote:
On Fri, Oct 24, 2014 at 04:46:32PM -0200, Marcelo Ricardo Leitner wrote:And also remove PRINTR macro in favor of net_*_ratelimited(). Signed-off-by: Marcelo Ricardo Leitner <mleitner@xxxxxxxxxx> --- Notes: v1-v2: make use of net_err_ratelimited() net/netfilter/nfnetlink_log.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c index b1e3a05794169283ed50d1c0fb4f44d9e7753eeb..0eb10bf295e92ad29541f58ca1de67e3380157c0 100644 --- a/net/netfilter/nfnetlink_log.c +++ b/net/netfilter/nfnetlink_log.c @@ -12,6 +12,8 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include <linux/module.h> #include <linux/skbuff.h> #include <linux/if_arp.h> @@ -45,9 +47,6 @@ #define NFULNL_QTHRESH_DEFAULT 100 /* 100 packets */ #define NFULNL_COPY_RANGE_MAX 0xFFFF /* max packet size is limited by 16-bit struct nfattr nfa_len field */ -#define PRINTR(x, args...) do { if (net_ratelimit()) \ - printk(x, ## args); } while (0); - struct nfulnl_instance { struct hlist_node hlist; /* global list of instances */ spinlock_t lock; @@ -335,8 +334,7 @@ nfulnl_alloc_skb(struct net *net, u32 peer_portid, unsigned int inst_size, skb = nfnetlink_alloc_skb(net, pkt_size, peer_portid, GFP_ATOMIC); if (!skb) - pr_err("nfnetlink_log: can't even alloc %u bytes\n", - pkt_size); + pr_err("can't even alloc %u bytes\n", pkt_size); } } @@ -569,7 +567,7 @@ __build_packet_message(struct nfnl_log_net *log, int size = nla_attr_size(data_len); if (skb_tailroom(inst->skb) < nla_total_size(data_len)) { - printk(KERN_WARNING "nfnetlink_log: no tailroom!\n"); + pr_warn("no tailroom!\n"); return -1;I think we can just goto nla_put_failure and remove this warning.
ok
} @@ -585,7 +583,7 @@ __build_packet_message(struct nfnl_log_net *log, return 0; nla_put_failure: - PRINTR(KERN_ERR "nfnetlink_log: error creating log nlmsg\n"); + net_err_ratelimited("error creating log nlmsg\n");I'd remove this one too. But before doing any error message cleanup, we should also call nlmsg_cancel() here to leave the nflog netlink batch message in consistent state in case of errors. And we don't check for errors after __build_packet_message() which may result in sending an incomplete message to userspace.
okay
Would you send us patches to address this? Thanks.
Sure thing. Thanks Pablo. Marcelo -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
