Netdev List
 help / color / mirror / Atom feed
* [PATCHv2-RFC 4/6] tun: orphan frags on xmit
From: Michael S. Tsirkin @ 2012-05-16 21:16 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jason Wang, eric.dumazet, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <cover.1337202879.git.mst@redhat.com>

tun xmit is actually receive of the internal tun
socket. Orphan the frags same as we do for normal rx path.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/tun.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 987aeef..fe5cd2f3 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -414,6 +414,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* Orphan the skb - required as we might hang on to it
 	 * for indefinite time. */
+	if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
+		goto drop;
 	skb_orphan(skb);
 
 	/* Enqueue packet */
-- 
MST

^ permalink raw reply related

* [PATCHv2-RFC 1/6] skbuff: add an api to orphan frags
From: Michael S. Tsirkin @ 2012-05-16 21:15 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jason Wang, eric.dumazet, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <cover.1337202879.git.mst@redhat.com>

Many places do
       if ((skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY))
		skb_copy_ubufs(skb, gfp_mask);
to copy and invoke frag destructors if necessary.
Add an inline helper for this.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/linux/skbuff.h |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index bb47314..2c5c69d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1665,6 +1665,22 @@ static inline void skb_orphan(struct sk_buff *skb)
 }
 
 /**
+ *	skb_orphan_frags - orphan the frags contained in a buffer
+ *	@skb: buffer to orphan frags from
+ *	@gfp_mask: allocation mask for replacement pages
+ *
+ *	For each frag in the SKB which needs a destructor (i.e. has an
+ *	owner) create a copy of that frag and release the original
+ *	page by calling the destructor.
+ */
+static inline int skb_orphan_frags(struct sk_buff *skb, gfp_t gfp_mask)
+{
+	if (likely(!(skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY)))
+		return 0;
+	return skb_copy_ubufs(skb, gfp_mask);
+}
+
+/**
  *	__skb_queue_purge - empty a list
  *	@list: list to empty
  *
-- 
MST

^ permalink raw reply related

* [PATCHv2-RFC 0/6] tun zerocopy support
From: Michael S. Tsirkin @ 2012-05-16 21:15 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jason Wang, eric.dumazet, netdev, linux-kernel, ebiederm, davem

So this still triggers some failures under stress
but I thought it might be helpful to post here
since people expressed interest.

This uses some patches from Ian's patchset
to support zerocopy with tun.
We are still trying to figure out how to make
everything work properly with tcp but tun seems
easier, and it's helpful since not everyone can
use macvtap.

There's some code duplication between tun and macvtap
now: common code could move to net/core/datagram.c,
this patch does not do this yet.

-- 
MST


Michael S. Tsirkin (6):
  skbuff: add an api to orphan frags
  skbuff: convert to skb_orphan_frags
  skbuff: export skb_copy_ubufs
  tun: orphan frags on xmit
  net: orphan frags on receive
  tun: experimental zero copy tx support

 drivers/net/tun.c      |  134 ++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/skbuff.h |   16 ++++++
 net/core/dev.c         |    2 +
 net/core/skbuff.c      |   24 +++-----
 4 files changed, 156 insertions(+), 20 deletions(-)

-- 
MST

^ permalink raw reply

* Re: [PATCH v5 2/2] decrement static keys on real destroy time
From: Andrew Morton @ 2012-05-16 21:13 UTC (permalink / raw)
  To: Glauber Costa
  Cc: cgroups, linux-mm, devel, kamezawa.hiroyu, netdev, Tejun Heo,
	Li Zefan, Johannes Weiner, Michal Hocko
In-Reply-To: <1336767077-25351-3-git-send-email-glommer@parallels.com>

On Fri, 11 May 2012 17:11:17 -0300
Glauber Costa <glommer@parallels.com> wrote:

> We call the destroy function when a cgroup starts to be removed,
> such as by a rmdir event.
> 
> However, because of our reference counters, some objects are still
> inflight. Right now, we are decrementing the static_keys at destroy()
> time, meaning that if we get rid of the last static_key reference,
> some objects will still have charges, but the code to properly
> uncharge them won't be run.
> 
> This becomes a problem specially if it is ever enabled again, because
> now new charges will be added to the staled charges making keeping
> it pretty much impossible.
> 
> We just need to be careful with the static branch activation:
> since there is no particular preferred order of their activation,
> we need to make sure that we only start using it after all
> call sites are active. This is achieved by having a per-memcg
> flag that is only updated after static_key_slow_inc() returns.
> At this time, we are sure all sites are active.
> 
> This is made per-memcg, not global, for a reason:
> it also has the effect of making socket accounting more
> consistent. The first memcg to be limited will trigger static_key()
> activation, therefore, accounting. But all the others will then be
> accounted no matter what. After this patch, only limited memcgs
> will have its sockets accounted.

So I'm scratching my head over what the actual bug is, and how
important it is.  AFAICT it will cause charging stats to exhibit some
inaccuracy when memcg's are being torn down?

I don't know how serious this in in the real world and so can't decide
which kernel version(s) we should fix.

When fixing bugs, please always fully describe the bug's end-user
impact, so that I and others can make these sorts of decisions.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v5 2/2] decrement static keys on real destroy time
From: Andrew Morton @ 2012-05-16 21:06 UTC (permalink / raw)
  To: Glauber Costa
  Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	devel-GEFAQzZX7r8dnm+yROfE0A,
	kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
	netdev-u79uwXL29TY76Z2rM5mHXA, Tejun Heo, Li Zefan,
	Johannes Weiner, Michal Hocko
In-Reply-To: <1336767077-25351-3-git-send-email-glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

On Fri, 11 May 2012 17:11:17 -0300
Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:

> We call the destroy function when a cgroup starts to be removed,
> such as by a rmdir event.
> 
> However, because of our reference counters, some objects are still
> inflight. Right now, we are decrementing the static_keys at destroy()
> time, meaning that if we get rid of the last static_key reference,
> some objects will still have charges, but the code to properly
> uncharge them won't be run.
> 
> This becomes a problem specially if it is ever enabled again, because
> now new charges will be added to the staled charges making keeping
> it pretty much impossible.
> 
> We just need to be careful with the static branch activation:
> since there is no particular preferred order of their activation,
> we need to make sure that we only start using it after all
> call sites are active. This is achieved by having a per-memcg
> flag that is only updated after static_key_slow_inc() returns.
> At this time, we are sure all sites are active.
> 
> This is made per-memcg, not global, for a reason:
> it also has the effect of making socket accounting more
> consistent. The first memcg to be limited will trigger static_key()
> activation, therefore, accounting. But all the others will then be
> accounted no matter what. After this patch, only limited memcgs
> will have its sockets accounted.
> 
> ...
>
> @@ -107,10 +104,31 @@ static int tcp_update_limit(struct mem_cgroup *memcg, u64 val)
>  		tcp->tcp_prot_mem[i] = min_t(long, val >> PAGE_SHIFT,
>  					     net->ipv4.sysctl_tcp_mem[i]);
>  
> -	if (val == RESOURCE_MAX && old_lim != RESOURCE_MAX)
> -		static_key_slow_dec(&memcg_socket_limit_enabled);
> -	else if (old_lim == RESOURCE_MAX && val != RESOURCE_MAX)
> -		static_key_slow_inc(&memcg_socket_limit_enabled);
> +	if (val == RESOURCE_MAX)
> +		cg_proto->active = false;
> +	else if (val != RESOURCE_MAX) {
> +		/*
> +		 * ->activated needs to be written after the static_key update.
> +		 *  This is what guarantees that the socket activation function
> +		 *  is the last one to run. See sock_update_memcg() for details,
> +		 *  and note that we don't mark any socket as belonging to this
> +		 *  memcg until that flag is up.
> +		 *
> +		 *  We need to do this, because static_keys will span multiple
> +		 *  sites, but we can't control their order. If we mark a socket
> +		 *  as accounted, but the accounting functions are not patched in
> +		 *  yet, we'll lose accounting.
> +		 *
> +		 *  We never race with the readers in sock_update_memcg(), because
> +		 *  when this value change, the code to process it is not patched in
> +		 *  yet.
> +		 */
> +		if (!cg_proto->activated) {
> +			static_key_slow_inc(&memcg_socket_limit_enabled);
> +			cg_proto->activated = true;
> +		}

If two threads run this code concurrently, they can both see
cg_proto->activated==false and they will both run
static_key_slow_inc().

Hopefully there's some locking somewhere which prevents this, but it is
unobvious.  We should comment this, probably at the cg_proto.activated
definition site.  Or we should fix the bug ;)


> +		cg_proto->active = true;
> +	}
>  
>  	return 0;
>  }
>
> ...
>

^ permalink raw reply

* Re: [PATCH v5 2/2] decrement static keys on real destroy time
From: Andrew Morton @ 2012-05-16 20:57 UTC (permalink / raw)
  To: Glauber Costa
  Cc: Li Zefan, cgroups, linux-mm, devel, kamezawa.hiroyu, netdev,
	Tejun Heo, Johannes Weiner, Michal Hocko
In-Reply-To: <4FB35153.3080309@parallels.com>

On Wed, 16 May 2012 11:03:47 +0400
Glauber Costa <glommer@parallels.com> wrote:

> On 05/14/2012 05:38 AM, Li Zefan wrote:
> >> +static void disarm_static_keys(struct mem_cgroup *memcg)
> > 
> >> +{
> >> +#ifdef CONFIG_INET
> >> +	if (memcg->tcp_mem.cg_proto.activated)
> >> +		static_key_slow_dec(&memcg_socket_limit_enabled);
> >> +#endif
> >> +}
> > 
> > 
> > Move this inside the ifdef/endif below ?
> > 
> > Otherwise I think you'll get compile error if !CONFIG_INET...
> 
> I don't fully get it.
> 
> We are supposed to provide a version of it for
> CONFIG_CGROUP_MEM_RES_CTLR_KMEM and an empty version for
> !CONFIG_CGROUP_MEM_RES_CTLR_KMEM
> 
> Inside the first, we take an action for CONFIG_INET, and no action for
> !CONFIG_INET.
> 
> Bear in mind that the slab patches will add another test to that place,
> and that's why I am doing it this way from the beginning.
> 
> Well, that said, I not only can be wrong, I very frequently am.
> 
> But I just compiled this one with and without CONFIG_INET, and it seems
> to be going alright.
> 

Yes, the ifdeffings in that area are rather nasty.

I wonder if it would be simpler to do away with the ifdef nesting. 
At the top-level, just do

#if defined(CONFIG_CGROUP_MEM_RES_CTLR_KMEM) && defined(CONFIG_INET)
static void disarm_static_keys(struct mem_cgroup *memcg)
{
	if (memcg->tcp_mem.cg_proto.activated)
		static_key_slow_dec(&memcg_socket_limit_enabled);
}
#else
static inline void disarm_static_keys(struct mem_cgroup *memcg)
{
}
#endif


The tcp_proto_cgroup() definition could go inside that ifdef as well.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [GIT] Networking
From: David Miller @ 2012-05-16 20:09 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, netdev, linux-kernel


1) ptp_pch driver build broke during this merge window due to missing
   slab.h header, fix from Geery Uytterhoeven.

2) If ipset passes in a bogus hash table size we crash because the
   size is not validated properly.  Compounding this, gcc-4.7 can
   miscompile ipset such that even when the user specifies legitimate
   parameters the tool passes in an out-of-range size to the kernel.

   Fix from Jozsef Kadlecsik.

3) Users have reported that the netdev watchdog can trigger with pch_gbe
   devices, and it turns out this is happening because of races in the TX
   path of the driver leading to the transmitter hanging.  Fix from
   Eric Dumazet, reported and tested by Andy Cress.

4) Novatel USB551L devices match the generic class entries for the
   cdc ethernet USB driver, but they don't work because they have
   generic descriptors and thus need FLAG_WWAN to function properly.

   Add the necessary ID table entry to fix this, from Dan Williams.

5) A recursive locking fix in the USBNET driver added a new problem,
   in that packet list traversal is now racy and we can thus access
   unlinked SKBs and crash.

   Avoid this situation by adding some extra state tracking, from
   Ming Lei.

6) The rtlwifi conversion to asynchronous firmware loading is racy,
   fix by reordering the probe procedure.  From Larry Finger.

   Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=43187

7) Fix regressions with bluetooth keyboards by notifying userland
   properly when the security level changes, from Gustavo Padovan.

8) Bluetooth needs to make sure device connected events are emitted
   before other kinds of events, otherwise userspace will think there
   is no baseband link yet and therefore abort the sockets associated
   with that connection.

   From Johan Hedberg.

There is more verbiage about the rtlwifi and 2 bluetooth fixes in
the wireless merge commit.

Please pull, thanks a lot.

The following changes since commit 568b44559d7ca269d367e694c74eb4436e7e3ccf:

  mn10300/CPU hotplug: Add missing call to notify_cpu_starting() (2012-05-15 18:16:57 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master

Dan Williams (1):
      cdc_ether: add Novatel USB551L device IDs for FLAG_WWAN

David S. Miller (1):
      Merge branch 'for-davem' of git://git.kernel.org/.../linville/wireless

Eric Dumazet (1):
      pch_gbe: fix transmit races

Geert Uytterhoeven (1):
      ptp_pch: Add missing #include <linux/slab.h>

Gustavo Padovan (1):
      Bluetooth: notify userspace of security level change

Johan Hedberg (1):
      Bluetooth: mgmt: Fix device_connected sending order

John W. Linville (1):
      Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem

Jozsef Kadlecsik (1):
      netfilter: ipset: fix hash size checking in kernel

Larry Finger (1):
      rtlwifi: fix for race condition when firmware is cached

Ming Lei (1):
      usbnet: fix skb traversing races during unlink(v2)

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |    2 -
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |   25 ++++-----
 drivers/net/usb/cdc_ether.c                        |   16 ++++++
 drivers/net/usb/usbnet.c                           |   54 ++++++++++++++------
 drivers/net/wireless/rtlwifi/pci.c                 |   16 +++---
 drivers/net/wireless/rtlwifi/usb.c                 |   10 ++--
 drivers/ptp/ptp_pch.c                              |    1 +
 include/linux/netfilter/ipset/ip_set_ahash.h       |   16 ++++++
 include/linux/usb/usbnet.h                         |    3 +-
 include/net/bluetooth/bluetooth.h                  |    1 +
 net/bluetooth/af_bluetooth.c                       |    2 +-
 net/bluetooth/hci_core.c                           |    8 +++
 net/bluetooth/hci_event.c                          |   11 +++-
 net/bluetooth/l2cap_core.c                         |    5 ++
 net/bluetooth/l2cap_sock.c                         |   12 +++--
 net/netfilter/ipset/ip_set_hash_ip.c               |   10 +++-
 net/netfilter/ipset/ip_set_hash_ipport.c           |   10 +++-
 net/netfilter/ipset/ip_set_hash_ipportip.c         |   10 +++-
 net/netfilter/ipset/ip_set_hash_ipportnet.c        |   10 +++-
 net/netfilter/ipset/ip_set_hash_net.c              |   10 +++-
 net/netfilter/ipset/ip_set_hash_netiface.c         |   10 +++-
 net/netfilter/ipset/ip_set_hash_netport.c          |   10 +++-
 22 files changed, 178 insertions(+), 74 deletions(-)

^ permalink raw reply

* Re: [PATCH net-next v4] be2net: Fix to allow get/set of debug levels in the firmware.
From: Ben Hutchings @ 2012-05-16 20:03 UTC (permalink / raw)
  To: David Miller; +Cc: somnath.kotur, netdev, suresh.reddy
In-Reply-To: <20120516.153710.2301978579715512630.davem@davemloft.net>

On Wed, 2012-05-16 at 15:37 -0400, David Miller wrote:
> From: Somnath Kotur <somnath.kotur@emulex.com>
> Date: Mon, 14 May 2012 21:59:28 +0530
> 
> > Fixed missing paranthesis warning
> > Incorporated review comments by Ben Hutchings.
> > 
> > Signed-off-by: Suresh Reddy <suresh.reddy@emulex.com>
> > Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
> 
> Ben H., please review.

I didn't spot anything wrong with it, but wouldn't go so far as to add a
reviewed-by.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* [PATCH net-next] net: l2tp: Standardize logging styles
From: Joe Perches @ 2012-05-16 19:55 UTC (permalink / raw)
  To: David S. Miller; +Cc: James Chapman, netdev, linux-kernel

Use more current logging styles.

Add pr_fmt to prefix output appropriately.
Convert printks to pr_<level>.
Convert PRINTK macros to new l2tp_<level> macros.
Neaten some <foo>_refcount debugging macros.
Use print_hex_dump_bytes instead of hand-coded loops.
Coalesce formats and align arguments.

Some KERN_DEBUG output is not now emitted unless
dynamic_debugging is enabled.

Signed-off-by: Joe Perches <joe@perches.com>
---
 net/l2tp/l2tp_core.c    |  216 ++++++++++++++++++++++-------------------------
 net/l2tp/l2tp_core.h    |   35 ++++++--
 net/l2tp/l2tp_debugfs.c |    6 +-
 net/l2tp/l2tp_eth.c     |   15 +--
 net/l2tp/l2tp_ip.c      |   15 +--
 net/l2tp/l2tp_ip6.c     |   15 +--
 net/l2tp/l2tp_netlink.c |    4 +-
 net/l2tp/l2tp_ppp.c     |  128 ++++++++++++++--------------
 8 files changed, 213 insertions(+), 221 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 0d6aedc..32b2155 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -18,6 +18,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/string.h>
 #include <linux/list.h>
@@ -86,12 +88,6 @@
 /* Default trace flags */
 #define L2TP_DEFAULT_DEBUG_FLAGS	0
 
-#define PRINTK(_mask, _type, _lvl, _fmt, args...)			\
-	do {								\
-		if ((_mask) & (_type))					\
-			printk(_lvl "L2TP: " _fmt, ##args);		\
-	} while (0)
-
 /* Private data stored for received packets in the skb.
  */
 struct l2tp_skb_cb {
@@ -141,14 +137,20 @@ static inline void l2tp_tunnel_dec_refcount_1(struct l2tp_tunnel *tunnel)
 		l2tp_tunnel_free(tunnel);
 }
 #ifdef L2TP_REFCNT_DEBUG
-#define l2tp_tunnel_inc_refcount(_t) do { \
-		printk(KERN_DEBUG "l2tp_tunnel_inc_refcount: %s:%d %s: cnt=%d\n", __func__, __LINE__, (_t)->name, atomic_read(&_t->ref_count)); \
-		l2tp_tunnel_inc_refcount_1(_t);				\
-	} while (0)
-#define l2tp_tunnel_dec_refcount(_t) do { \
-		printk(KERN_DEBUG "l2tp_tunnel_dec_refcount: %s:%d %s: cnt=%d\n", __func__, __LINE__, (_t)->name, atomic_read(&_t->ref_count)); \
-		l2tp_tunnel_dec_refcount_1(_t);				\
-	} while (0)
+#define l2tp_tunnel_inc_refcount(_t)					\
+do {									\
+	pr_debug("l2tp_tunnel_inc_refcount: %s:%d %s: cnt=%d\n",	\
+		 __func__, __LINE__, (_t)->name,			\
+		 atomic_read(&_t->ref_count));				\
+	l2tp_tunnel_inc_refcount_1(_t);					\
+} while (0)
+#define l2tp_tunnel_dec_refcount(_t)
+do {									\
+	pr_debug("l2tp_tunnel_dec_refcount: %s:%d %s: cnt=%d\n",	\
+		 __func__, __LINE__, (_t)->name,			\
+		 atomic_read(&_t->ref_count));				\
+	l2tp_tunnel_dec_refcount_1(_t);					\
+} while (0)
 #else
 #define l2tp_tunnel_inc_refcount(t) l2tp_tunnel_inc_refcount_1(t)
 #define l2tp_tunnel_dec_refcount(t) l2tp_tunnel_dec_refcount_1(t)
@@ -337,10 +339,10 @@ static void l2tp_recv_queue_skb(struct l2tp_session *session, struct sk_buff *sk
 	skb_queue_walk_safe(&session->reorder_q, skbp, tmp) {
 		if (L2TP_SKB_CB(skbp)->ns > ns) {
 			__skb_queue_before(&session->reorder_q, skbp, skb);
-			PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-			       "%s: pkt %hu, inserted before %hu, reorder_q len=%d\n",
-			       session->name, ns, L2TP_SKB_CB(skbp)->ns,
-			       skb_queue_len(&session->reorder_q));
+			l2tp_dbg(session, L2TP_MSG_SEQ,
+				 "%s: pkt %hu, inserted before %hu, reorder_q len=%d\n",
+				 session->name, ns, L2TP_SKB_CB(skbp)->ns,
+				 skb_queue_len(&session->reorder_q));
 			u64_stats_update_begin(&sstats->syncp);
 			sstats->rx_oos_packets++;
 			u64_stats_update_end(&sstats->syncp);
@@ -386,8 +388,8 @@ static void l2tp_recv_dequeue_skb(struct l2tp_session *session, struct sk_buff *
 		else
 			session->nr &= 0xffffff;
 
-		PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-		       "%s: updated nr to %hu\n", session->name, session->nr);
+		l2tp_dbg(session, L2TP_MSG_SEQ, "%s: updated nr to %hu\n",
+			 session->name, session->nr);
 	}
 
 	/* call private receive handler */
@@ -422,12 +424,11 @@ start:
 			sstats->rx_seq_discards++;
 			sstats->rx_errors++;
 			u64_stats_update_end(&sstats->syncp);
-			PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-			       "%s: oos pkt %u len %d discarded (too old), "
-			       "waiting for %u, reorder_q_len=%d\n",
-			       session->name, L2TP_SKB_CB(skb)->ns,
-			       L2TP_SKB_CB(skb)->length, session->nr,
-			       skb_queue_len(&session->reorder_q));
+			l2tp_dbg(session, L2TP_MSG_SEQ,
+				 "%s: oos pkt %u len %d discarded (too old), waiting for %u, reorder_q_len=%d\n",
+				 session->name, L2TP_SKB_CB(skb)->ns,
+				 L2TP_SKB_CB(skb)->length, session->nr,
+				 skb_queue_len(&session->reorder_q));
 			session->reorder_skip = 1;
 			__skb_unlink(skb, &session->reorder_q);
 			kfree_skb(skb);
@@ -438,20 +439,19 @@ start:
 
 		if (L2TP_SKB_CB(skb)->has_seq) {
 			if (session->reorder_skip) {
-				PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-				       "%s: advancing nr to next pkt: %u -> %u",
-				       session->name, session->nr,
-				       L2TP_SKB_CB(skb)->ns);
+				l2tp_dbg(session, L2TP_MSG_SEQ,
+					 "%s: advancing nr to next pkt: %u -> %u",
+					 session->name, session->nr,
+					 L2TP_SKB_CB(skb)->ns);
 				session->reorder_skip = 0;
 				session->nr = L2TP_SKB_CB(skb)->ns;
 			}
 			if (L2TP_SKB_CB(skb)->ns != session->nr) {
-				PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-				       "%s: holding oos pkt %u len %d, "
-				       "waiting for %u, reorder_q_len=%d\n",
-				       session->name, L2TP_SKB_CB(skb)->ns,
-				       L2TP_SKB_CB(skb)->length, session->nr,
-				       skb_queue_len(&session->reorder_q));
+				l2tp_dbg(session, L2TP_MSG_SEQ,
+					 "%s: holding oos pkt %u len %d, waiting for %u, reorder_q_len=%d\n",
+					 session->name, L2TP_SKB_CB(skb)->ns,
+					 L2TP_SKB_CB(skb)->length, session->nr,
+					 skb_queue_len(&session->reorder_q));
 				goto out;
 			}
 		}
@@ -595,9 +595,10 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 	/* Parse and check optional cookie */
 	if (session->peer_cookie_len > 0) {
 		if (memcmp(ptr, &session->peer_cookie[0], session->peer_cookie_len)) {
-			PRINTK(tunnel->debug, L2TP_MSG_DATA, KERN_INFO,
-			       "%s: cookie mismatch (%u/%u). Discarding.\n",
-			       tunnel->name, tunnel->tunnel_id, session->session_id);
+			l2tp_info(tunnel, L2TP_MSG_DATA,
+				  "%s: cookie mismatch (%u/%u). Discarding.\n",
+				  tunnel->name, tunnel->tunnel_id,
+				  session->session_id);
 			u64_stats_update_begin(&sstats->syncp);
 			sstats->rx_cookie_discards++;
 			u64_stats_update_end(&sstats->syncp);
@@ -626,9 +627,9 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 			L2TP_SKB_CB(skb)->ns = ns;
 			L2TP_SKB_CB(skb)->has_seq = 1;
 
-			PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-			       "%s: recv data ns=%u, nr=%u, session nr=%u\n",
-			       session->name, ns, nr, session->nr);
+			l2tp_dbg(session, L2TP_MSG_SEQ,
+				 "%s: recv data ns=%u, nr=%u, session nr=%u\n",
+				 session->name, ns, nr, session->nr);
 		}
 	} else if (session->l2specific_type == L2TP_L2SPECTYPE_DEFAULT) {
 		u32 l2h = ntohl(*(__be32 *) ptr);
@@ -640,9 +641,9 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 			L2TP_SKB_CB(skb)->ns = ns;
 			L2TP_SKB_CB(skb)->has_seq = 1;
 
-			PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-			       "%s: recv data ns=%u, session nr=%u\n",
-			       session->name, ns, session->nr);
+			l2tp_dbg(session, L2TP_MSG_SEQ,
+				 "%s: recv data ns=%u, session nr=%u\n",
+				 session->name, ns, session->nr);
 		}
 	}
 
@@ -655,9 +656,9 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 		 * configure it so.
 		 */
 		if ((!session->lns_mode) && (!session->send_seq)) {
-			PRINTK(session->debug, L2TP_MSG_SEQ, KERN_INFO,
-			       "%s: requested to enable seq numbers by LNS\n",
-			       session->name);
+			l2tp_info(session, L2TP_MSG_SEQ,
+				  "%s: requested to enable seq numbers by LNS\n",
+				  session->name);
 			session->send_seq = -1;
 			l2tp_session_set_header_len(session, tunnel->version);
 		}
@@ -666,9 +667,9 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 		 * If user has configured mandatory sequence numbers, discard.
 		 */
 		if (session->recv_seq) {
-			PRINTK(session->debug, L2TP_MSG_SEQ, KERN_WARNING,
-			       "%s: recv data has no seq numbers when required. "
-			       "Discarding\n", session->name);
+			l2tp_warn(session, L2TP_MSG_SEQ,
+				  "%s: recv data has no seq numbers when required. Discarding.\n",
+				  session->name);
 			u64_stats_update_begin(&sstats->syncp);
 			sstats->rx_seq_discards++;
 			u64_stats_update_end(&sstats->syncp);
@@ -681,15 +682,15 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 		 * LAC is broken. Discard the frame.
 		 */
 		if ((!session->lns_mode) && (session->send_seq)) {
-			PRINTK(session->debug, L2TP_MSG_SEQ, KERN_INFO,
-			       "%s: requested to disable seq numbers by LNS\n",
-			       session->name);
+			l2tp_info(session, L2TP_MSG_SEQ,
+				  "%s: requested to disable seq numbers by LNS\n",
+				  session->name);
 			session->send_seq = 0;
 			l2tp_session_set_header_len(session, tunnel->version);
 		} else if (session->send_seq) {
-			PRINTK(session->debug, L2TP_MSG_SEQ, KERN_WARNING,
-			       "%s: recv data has no seq numbers when required. "
-			       "Discarding\n", session->name);
+			l2tp_warn(session, L2TP_MSG_SEQ,
+				  "%s: recv data has no seq numbers when required. Discarding.\n",
+				  session->name);
 			u64_stats_update_begin(&sstats->syncp);
 			sstats->rx_seq_discards++;
 			u64_stats_update_end(&sstats->syncp);
@@ -749,12 +750,11 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
 				u64_stats_update_begin(&sstats->syncp);
 				sstats->rx_seq_discards++;
 				u64_stats_update_end(&sstats->syncp);
-				PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-				       "%s: oos pkt %u len %d discarded, "
-				       "waiting for %u, reorder_q_len=%d\n",
-				       session->name, L2TP_SKB_CB(skb)->ns,
-				       L2TP_SKB_CB(skb)->length, session->nr,
-				       skb_queue_len(&session->reorder_q));
+				l2tp_dbg(session, L2TP_MSG_SEQ,
+					 "%s: oos pkt %u len %d discarded, waiting for %u, reorder_q_len=%d\n",
+					 session->name, L2TP_SKB_CB(skb)->ns,
+					 L2TP_SKB_CB(skb)->length, session->nr,
+					 skb_queue_len(&session->reorder_q));
 				goto discard;
 			}
 			skb_queue_tail(&session->reorder_q, skb);
@@ -800,7 +800,6 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
 	unsigned char *ptr, *optr;
 	u16 hdrflags;
 	u32 tunnel_id, session_id;
-	int offset;
 	u16 version;
 	int length;
 	struct l2tp_stats *tstats;
@@ -813,8 +812,9 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
 
 	/* Short packet? */
 	if (!pskb_may_pull(skb, L2TP_HDR_SIZE_SEQ)) {
-		PRINTK(tunnel->debug, L2TP_MSG_DATA, KERN_INFO,
-		       "%s: recv short packet (len=%d)\n", tunnel->name, skb->len);
+		l2tp_info(tunnel, L2TP_MSG_DATA,
+			  "%s: recv short packet (len=%d)\n",
+			  tunnel->name, skb->len);
 		goto error;
 	}
 
@@ -824,14 +824,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
 		if (!pskb_may_pull(skb, length))
 			goto error;
 
-		printk(KERN_DEBUG "%s: recv: ", tunnel->name);
-
-		offset = 0;
-		do {
-			printk(" %02X", skb->data[offset]);
-		} while (++offset < length);
-
-		printk("\n");
+		pr_debug("%s: recv\n", tunnel->name);
+		print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, skb->data, length);
 	}
 
 	/* Point to L2TP header */
@@ -843,9 +837,9 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
 	/* Check protocol version */
 	version = hdrflags & L2TP_HDR_VER_MASK;
 	if (version != tunnel->version) {
-		PRINTK(tunnel->debug, L2TP_MSG_DATA, KERN_INFO,
-		       "%s: recv protocol version mismatch: got %d expected %d\n",
-		       tunnel->name, version, tunnel->version);
+		l2tp_info(tunnel, L2TP_MSG_DATA,
+			  "%s: recv protocol version mismatch: got %d expected %d\n",
+			  tunnel->name, version, tunnel->version);
 		goto error;
 	}
 
@@ -854,8 +848,9 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
 
 	/* If type is control packet, it is handled by userspace. */
 	if (hdrflags & L2TP_HDRFLAG_T) {
-		PRINTK(tunnel->debug, L2TP_MSG_DATA, KERN_DEBUG,
-		       "%s: recv control packet, len=%d\n", tunnel->name, length);
+		l2tp_dbg(tunnel, L2TP_MSG_DATA,
+			 "%s: recv control packet, len=%d\n",
+			 tunnel->name, length);
 		goto error;
 	}
 
@@ -883,9 +878,9 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
 	session = l2tp_session_find(tunnel->l2tp_net, tunnel, session_id);
 	if (!session || !session->recv_skb) {
 		/* Not found? Pass to userspace to deal with */
-		PRINTK(tunnel->debug, L2TP_MSG_DATA, KERN_INFO,
-		       "%s: no session found (%u/%u). Passing up.\n",
-		       tunnel->name, tunnel_id, session_id);
+		l2tp_info(tunnel, L2TP_MSG_DATA,
+			  "%s: no session found (%u/%u). Passing up.\n",
+			  tunnel->name, tunnel_id, session_id);
 		goto error;
 	}
 
@@ -925,8 +920,8 @@ int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	if (tunnel == NULL)
 		goto pass_up;
 
-	PRINTK(tunnel->debug, L2TP_MSG_DATA, KERN_DEBUG,
-	       "%s: received %d bytes\n", tunnel->name, skb->len);
+	l2tp_dbg(tunnel, L2TP_MSG_DATA, "%s: received %d bytes\n",
+		 tunnel->name, skb->len);
 
 	if (l2tp_udp_recv_core(tunnel, skb, tunnel->recv_payload_hook))
 		goto pass_up_put;
@@ -968,8 +963,8 @@ static int l2tp_build_l2tpv2_header(struct l2tp_session *session, void *buf)
 		*bufp++ = 0;
 		session->ns++;
 		session->ns &= 0xffff;
-		PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-		       "%s: updated ns to %u\n", session->name, session->ns);
+		l2tp_dbg(session, L2TP_MSG_SEQ, "%s: updated ns to %u\n",
+			 session->name, session->ns);
 	}
 
 	return bufp - optr;
@@ -1005,8 +1000,9 @@ static int l2tp_build_l2tpv3_header(struct l2tp_session *session, void *buf)
 				l2h = 0x40000000 | session->ns;
 				session->ns++;
 				session->ns &= 0xffffff;
-				PRINTK(session->debug, L2TP_MSG_SEQ, KERN_DEBUG,
-				       "%s: updated ns to %u\n", session->name, session->ns);
+				l2tp_dbg(session, L2TP_MSG_SEQ,
+					 "%s: updated ns to %u\n",
+					 session->name, session->ns);
 			}
 
 			*((__be32 *) bufp) = htonl(l2h);
@@ -1029,27 +1025,19 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb,
 
 	/* Debug */
 	if (session->send_seq)
-		PRINTK(session->debug, L2TP_MSG_DATA, KERN_DEBUG,
-		       "%s: send %Zd bytes, ns=%u\n", session->name,
-		       data_len, session->ns - 1);
+		l2tp_dbg(session, L2TP_MSG_DATA, "%s: send %Zd bytes, ns=%u\n",
+			 session->name, data_len, session->ns - 1);
 	else
-		PRINTK(session->debug, L2TP_MSG_DATA, KERN_DEBUG,
-		       "%s: send %Zd bytes\n", session->name, data_len);
+		l2tp_dbg(session, L2TP_MSG_DATA, "%s: send %Zd bytes\n",
+			 session->name, data_len);
 
 	if (session->debug & L2TP_MSG_DATA) {
-		int i;
 		int uhlen = (tunnel->encap == L2TP_ENCAPTYPE_UDP) ? sizeof(struct udphdr) : 0;
 		unsigned char *datap = skb->data + uhlen;
 
-		printk(KERN_DEBUG "%s: xmit:", session->name);
-		for (i = 0; i < (len - uhlen); i++) {
-			printk(" %02X", *datap++);
-			if (i == 31) {
-				printk(" ...");
-				break;
-			}
-		}
-		printk("\n");
+		pr_debug("%s: xmit\n", session->name);
+		print_hex_dump_bytes("", DUMP_PREFIX_OFFSET,
+				     datap, min_t(size_t, 32, len - uhlen));
 	}
 
 	/* Queue the packet to IP for output */
@@ -1248,8 +1236,7 @@ static void l2tp_tunnel_destruct(struct sock *sk)
 	if (tunnel == NULL)
 		goto end;
 
-	PRINTK(tunnel->debug, L2TP_MSG_CONTROL, KERN_INFO,
-	       "%s: closing...\n", tunnel->name);
+	l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: closing...\n", tunnel->name);
 
 	/* Close all sessions */
 	l2tp_tunnel_closeall(tunnel);
@@ -1291,8 +1278,8 @@ static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
 
 	BUG_ON(tunnel == NULL);
 
-	PRINTK(tunnel->debug, L2TP_MSG_CONTROL, KERN_INFO,
-	       "%s: closing all sessions...\n", tunnel->name);
+	l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: closing all sessions...\n",
+		  tunnel->name);
 
 	write_lock_bh(&tunnel->hlist_lock);
 	for (hash = 0; hash < L2TP_HASH_SIZE; hash++) {
@@ -1300,8 +1287,8 @@ again:
 		hlist_for_each_safe(walk, tmp, &tunnel->session_hlist[hash]) {
 			session = hlist_entry(walk, struct l2tp_session, hlist);
 
-			PRINTK(session->debug, L2TP_MSG_CONTROL, KERN_INFO,
-			       "%s: closing session\n", session->name);
+			l2tp_info(session, L2TP_MSG_CONTROL,
+				  "%s: closing session\n", session->name);
 
 			hlist_del_init(&session->hlist);
 
@@ -1354,8 +1341,7 @@ static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel)
 	BUG_ON(atomic_read(&tunnel->ref_count) != 0);
 	BUG_ON(tunnel->sock != NULL);
 
-	PRINTK(tunnel->debug, L2TP_MSG_CONTROL, KERN_INFO,
-	       "%s: free...\n", tunnel->name);
+	l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: free...\n", tunnel->name);
 
 	/* Remove from tunnel list */
 	spin_lock_bh(&pn->l2tp_tunnel_list_lock);
@@ -1536,7 +1522,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
 		err = -EBADF;
 		sock = sockfd_lookup(fd, &err);
 		if (!sock) {
-			printk(KERN_ERR "tunl %hu: sockfd_lookup(fd=%d) returned %d\n",
+			pr_err("tunl %hu: sockfd_lookup(fd=%d) returned %d\n",
 			       tunnel_id, fd, err);
 			goto err;
 		}
@@ -1552,7 +1538,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
 	case L2TP_ENCAPTYPE_UDP:
 		err = -EPROTONOSUPPORT;
 		if (sk->sk_protocol != IPPROTO_UDP) {
-			printk(KERN_ERR "tunl %hu: fd %d wrong protocol, got %d, expected %d\n",
+			pr_err("tunl %hu: fd %d wrong protocol, got %d, expected %d\n",
 			       tunnel_id, fd, sk->sk_protocol, IPPROTO_UDP);
 			goto err;
 		}
@@ -1560,7 +1546,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
 	case L2TP_ENCAPTYPE_IP:
 		err = -EPROTONOSUPPORT;
 		if (sk->sk_protocol != IPPROTO_L2TP) {
-			printk(KERN_ERR "tunl %hu: fd %d wrong protocol, got %d, expected %d\n",
+			pr_err("tunl %hu: fd %d wrong protocol, got %d, expected %d\n",
 			       tunnel_id, fd, sk->sk_protocol, IPPROTO_L2TP);
 			goto err;
 		}
@@ -1868,7 +1854,7 @@ static int __init l2tp_init(void)
 	if (rc)
 		goto out;
 
-	printk(KERN_INFO "L2TP core driver, %s\n", L2TP_DRV_VERSION);
+	pr_info("L2TP core driver, %s\n", L2TP_DRV_VERSION);
 
 out:
 	return rc;
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 9002634..a38ec6c 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -261,17 +261,36 @@ static inline void l2tp_session_dec_refcount_1(struct l2tp_session *session)
 }
 
 #ifdef L2TP_REFCNT_DEBUG
-#define l2tp_session_inc_refcount(_s) do { \
-		printk(KERN_DEBUG "l2tp_session_inc_refcount: %s:%d %s: cnt=%d\n", __func__, __LINE__, (_s)->name, atomic_read(&_s->ref_count)); \
-		l2tp_session_inc_refcount_1(_s);				\
-	} while (0)
-#define l2tp_session_dec_refcount(_s) do { \
-		printk(KERN_DEBUG "l2tp_session_dec_refcount: %s:%d %s: cnt=%d\n", __func__, __LINE__, (_s)->name, atomic_read(&_s->ref_count)); \
-		l2tp_session_dec_refcount_1(_s);				\
-	} while (0)
+#define l2tp_session_inc_refcount(_s)					\
+do {									\
+	pr_debug("l2tp_session_inc_refcount: %s:%d %s: cnt=%d\n",	\
+		 __func__, __LINE__, (_s)->name,			\
+		 atomic_read(&_s->ref_count));				\
+	l2tp_session_inc_refcount_1(_s);				\
+} while (0)
+#define l2tp_session_dec_refcount(_s)					\
+do {									\
+	pr_debug("l2tp_session_dec_refcount: %s:%d %s: cnt=%d\n",	\
+		 __func__, __LINE__, (_s)->name,			\
+		 atomic_read(&_s->ref_count));				\
+	l2tp_session_dec_refcount_1(_s);				\
+} while (0)
 #else
 #define l2tp_session_inc_refcount(s) l2tp_session_inc_refcount_1(s)
 #define l2tp_session_dec_refcount(s) l2tp_session_dec_refcount_1(s)
 #endif
 
+#define l2tp_printk(ptr, type, func, fmt, ...)				\
+do {									\
+	if (((ptr)->debug) & (type))					\
+		func(fmt, ##__VA_ARGS__);				\
+} while (0)
+
+#define l2tp_warn(ptr, type, fmt, ...)					\
+	l2tp_printk(ptr, type, pr_warn, fmt, ##__VA_ARGS__)
+#define l2tp_info(ptr, type, fmt, ...)					\
+	l2tp_printk(ptr, type, pr_info, fmt, ##__VA_ARGS__)
+#define l2tp_dbg(ptr, type, fmt, ...)					\
+	l2tp_printk(ptr, type, pr_debug, fmt, ##__VA_ARGS__)
+
 #endif /* _L2TP_CORE_H_ */
diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
index c0d57ba..c3813bc 100644
--- a/net/l2tp/l2tp_debugfs.c
+++ b/net/l2tp/l2tp_debugfs.c
@@ -9,6 +9,8 @@
  *	2 of the License, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/socket.h>
@@ -325,11 +327,11 @@ static int __init l2tp_debugfs_init(void)
 	if (tunnels == NULL)
 		rc = -EIO;
 
-	printk(KERN_INFO "L2TP debugfs support\n");
+	pr_info("L2TP debugfs support\n");
 
 out:
 	if (rc)
-		printk(KERN_WARNING "l2tp debugfs: unable to init\n");
+		pr_warn("unable to init\n");
 
 	return rc;
 }
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 63fe5f3..443591d 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -9,6 +9,8 @@
  *	2 of the License, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/skbuff.h>
 #include <linux/socket.h>
@@ -115,21 +117,14 @@ static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb,
 
 	if (session->debug & L2TP_MSG_DATA) {
 		unsigned int length;
-		int offset;
 		u8 *ptr = skb->data;
 
 		length = min(32u, skb->len);
 		if (!pskb_may_pull(skb, length))
 			goto error;
 
-		printk(KERN_DEBUG "%s: eth recv: ", session->name);
-
-		offset = 0;
-		do {
-			printk(" %02X", ptr[offset]);
-		} while (++offset < length);
-
-		printk("\n");
+		pr_debug("%s: eth recv\n", session->name);
+		print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, ptr, length);
 	}
 
 	if (!pskb_may_pull(skb, sizeof(ETH_HLEN)))
@@ -308,7 +303,7 @@ static int __init l2tp_eth_init(void)
 	if (err)
 		goto out_unreg;
 
-	printk(KERN_INFO "L2TP ethernet pseudowire support (L2TPv3)\n");
+	pr_info("L2TP ethernet pseudowire support (L2TPv3)\n");
 
 	return 0;
 
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index c89a32f..889f5d1 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -9,6 +9,8 @@
  *	2 of the License, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/icmp.h>
 #include <linux/module.h>
 #include <linux/skbuff.h>
@@ -120,7 +122,6 @@ static int l2tp_ip_recv(struct sk_buff *skb)
 	struct l2tp_session *session;
 	struct l2tp_tunnel *tunnel = NULL;
 	int length;
-	int offset;
 
 	/* Point to L2TP header */
 	optr = ptr = skb->data;
@@ -155,14 +156,8 @@ static int l2tp_ip_recv(struct sk_buff *skb)
 		if (!pskb_may_pull(skb, length))
 			goto discard;
 
-		printk(KERN_DEBUG "%s: ip recv: ", tunnel->name);
-
-		offset = 0;
-		do {
-			printk(" %02X", ptr[offset]);
-		} while (++offset < length);
-
-		printk("\n");
+		pr_debug("%s: ip recv\n", tunnel->name);
+		print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, ptr, length);
 	}
 
 	l2tp_recv_common(session, skb, ptr, optr, 0, skb->len, tunnel->recv_payload_hook);
@@ -593,7 +588,7 @@ static int __init l2tp_ip_init(void)
 {
 	int err;
 
-	printk(KERN_INFO "L2TP IP encapsulation support (L2TPv3)\n");
+	pr_info("L2TP IP encapsulation support (L2TPv3)\n");
 
 	err = proto_register(&l2tp_ip_prot, 1);
 	if (err != 0)
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 88f0abe..0291d8d 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -9,6 +9,8 @@
  *	2 of the License, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/icmp.h>
 #include <linux/module.h>
 #include <linux/skbuff.h>
@@ -133,7 +135,6 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
 	struct l2tp_session *session;
 	struct l2tp_tunnel *tunnel = NULL;
 	int length;
-	int offset;
 
 	/* Point to L2TP header */
 	optr = ptr = skb->data;
@@ -168,14 +169,8 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
 		if (!pskb_may_pull(skb, length))
 			goto discard;
 
-		printk(KERN_DEBUG "%s: ip recv: ", tunnel->name);
-
-		offset = 0;
-		do {
-			printk(" %02X", ptr[offset]);
-		} while (++offset < length);
-
-		printk("\n");
+		pr_debug("%s: ip recv\n", tunnel->name);
+		print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, ptr, length);
 	}
 
 	l2tp_recv_common(session, skb, ptr, optr, 0, skb->len,
@@ -752,7 +747,7 @@ static int __init l2tp_ip6_init(void)
 {
 	int err;
 
-	printk(KERN_INFO "L2TP IP encapsulation support for IPv6 (L2TPv3)\n");
+	pr_info("L2TP IP encapsulation support for IPv6 (L2TPv3)\n");
 
 	err = proto_register(&l2tp_ip6_prot, 1);
 	if (err != 0)
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 24edad0..8577264 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -14,6 +14,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <net/sock.h>
 #include <net/genetlink.h>
 #include <net/udp.h>
@@ -902,7 +904,7 @@ static int l2tp_nl_init(void)
 {
 	int err;
 
-	printk(KERN_INFO "L2TP netlink interface\n");
+	pr_info("L2TP netlink interface\n");
 	err = genl_register_family_with_ops(&l2tp_nl_family, l2tp_nl_ops,
 					    ARRAY_SIZE(l2tp_nl_ops));
 
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 9f2c421..8ef6b94 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -57,6 +57,8 @@
  * http://openl2tp.sourceforge.net.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/string.h>
 #include <linux/list.h>
@@ -106,12 +108,6 @@
 /* Space for UDP, L2TP and PPP headers */
 #define PPPOL2TP_HEADER_OVERHEAD	40
 
-#define PRINTK(_mask, _type, _lvl, _fmt, args...)			\
-	do {								\
-		if ((_mask) & (_type))					\
-			printk(_lvl "PPPOL2TP: " _fmt, ##args);		\
-	} while (0)
-
 /* Number of bytes to build transmit L2TP headers.
  * Unfortunately the size is different depending on whether sequence numbers
  * are enabled.
@@ -236,9 +232,9 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
 
 	if (sk->sk_state & PPPOX_BOUND) {
 		struct pppox_sock *po;
-		PRINTK(session->debug, PPPOL2TP_MSG_DATA, KERN_DEBUG,
-		       "%s: recv %d byte data frame, passing to ppp\n",
-		       session->name, data_len);
+		l2tp_dbg(session, PPPOL2TP_MSG_DATA,
+			 "%s: recv %d byte data frame, passing to ppp\n",
+			 session->name, data_len);
 
 		/* We need to forget all info related to the L2TP packet
 		 * gathered in the skb as we are going to reuse the same
@@ -259,8 +255,8 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
 		po = pppox_sk(sk);
 		ppp_input(&po->chan, skb);
 	} else {
-		PRINTK(session->debug, PPPOL2TP_MSG_DATA, KERN_INFO,
-		       "%s: socket not bound\n", session->name);
+		l2tp_info(session, PPPOL2TP_MSG_DATA, "%s: socket not bound\n",
+			  session->name);
 
 		/* Not bound. Nothing we can do, so discard. */
 		session->stats.rx_errors++;
@@ -270,8 +266,7 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
 	return;
 
 no_sock:
-	PRINTK(session->debug, PPPOL2TP_MSG_DATA, KERN_INFO,
-	       "%s: no socket\n", session->name);
+	l2tp_info(session, PPPOL2TP_MSG_DATA, "%s: no socket\n", session->name);
 	kfree_skb(skb);
 }
 
@@ -827,8 +822,8 @@ out_no_ppp:
 	/* This is how we get the session context from the socket. */
 	sk->sk_user_data = session;
 	sk->sk_state = PPPOX_CONNECTED;
-	PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-	       "%s: created\n", session->name);
+	l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: created\n",
+		  session->name);
 
 end:
 	release_sock(sk);
@@ -881,8 +876,8 @@ static int pppol2tp_session_create(struct net *net, u32 tunnel_id, u32 session_i
 	ps = l2tp_session_priv(session);
 	ps->tunnel_sock = tunnel->sock;
 
-	PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-	       "%s: created\n", session->name);
+	l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: created\n",
+		  session->name);
 
 	error = 0;
 
@@ -1058,9 +1053,9 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 	struct l2tp_tunnel *tunnel = session->tunnel;
 	struct pppol2tp_ioc_stats stats;
 
-	PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_DEBUG,
-	       "%s: pppol2tp_session_ioctl(cmd=%#x, arg=%#lx)\n",
-	       session->name, cmd, arg);
+	l2tp_dbg(session, PPPOL2TP_MSG_CONTROL,
+		 "%s: pppol2tp_session_ioctl(cmd=%#x, arg=%#lx)\n",
+		 session->name, cmd, arg);
 
 	sk = ps->sock;
 	sock_hold(sk);
@@ -1078,8 +1073,8 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 		if (copy_to_user((void __user *) arg, &ifr, sizeof(struct ifreq)))
 			break;
 
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get mtu=%d\n", session->name, session->mtu);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get mtu=%d\n",
+			  session->name, session->mtu);
 		err = 0;
 		break;
 
@@ -1094,8 +1089,8 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 
 		session->mtu = ifr.ifr_mtu;
 
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set mtu=%d\n", session->name, session->mtu);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: set mtu=%d\n",
+			  session->name, session->mtu);
 		err = 0;
 		break;
 
@@ -1108,8 +1103,8 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 		if (put_user(session->mru, (int __user *) arg))
 			break;
 
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get mru=%d\n", session->name, session->mru);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get mru=%d\n",
+			  session->name, session->mru);
 		err = 0;
 		break;
 
@@ -1123,8 +1118,8 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 			break;
 
 		session->mru = val;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set mru=%d\n", session->name, session->mru);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: set mru=%d\n",
+			  session->name, session->mru);
 		err = 0;
 		break;
 
@@ -1133,8 +1128,8 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 		if (put_user(ps->flags, (int __user *) arg))
 			break;
 
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get flags=%d\n", session->name, ps->flags);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get flags=%d\n",
+			  session->name, ps->flags);
 		err = 0;
 		break;
 
@@ -1143,8 +1138,8 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 		if (get_user(val, (int __user *) arg))
 			break;
 		ps->flags = val;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set flags=%d\n", session->name, ps->flags);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: set flags=%d\n",
+			  session->name, ps->flags);
 		err = 0;
 		break;
 
@@ -1160,8 +1155,8 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 		if (copy_to_user((void __user *) arg, &stats,
 				 sizeof(stats)))
 			break;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get L2TP stats\n", session->name);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get L2TP stats\n",
+			  session->name);
 		err = 0;
 		break;
 
@@ -1188,9 +1183,9 @@ static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel,
 	struct sock *sk;
 	struct pppol2tp_ioc_stats stats;
 
-	PRINTK(tunnel->debug, PPPOL2TP_MSG_CONTROL, KERN_DEBUG,
-	       "%s: pppol2tp_tunnel_ioctl(cmd=%#x, arg=%#lx)\n",
-	       tunnel->name, cmd, arg);
+	l2tp_dbg(tunnel, PPPOL2TP_MSG_CONTROL,
+		 "%s: pppol2tp_tunnel_ioctl(cmd=%#x, arg=%#lx)\n",
+		 tunnel->name, cmd, arg);
 
 	sk = tunnel->sock;
 	sock_hold(sk);
@@ -1224,8 +1219,8 @@ static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel,
 			err = -EFAULT;
 			break;
 		}
-		PRINTK(tunnel->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get L2TP stats\n", tunnel->name);
+		l2tp_info(tunnel, PPPOL2TP_MSG_CONTROL, "%s: get L2TP stats\n",
+			  tunnel->name);
 		err = 0;
 		break;
 
@@ -1314,8 +1309,8 @@ static int pppol2tp_tunnel_setsockopt(struct sock *sk,
 	switch (optname) {
 	case PPPOL2TP_SO_DEBUG:
 		tunnel->debug = val;
-		PRINTK(tunnel->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set debug=%x\n", tunnel->name, tunnel->debug);
+		l2tp_info(tunnel, PPPOL2TP_MSG_CONTROL, "%s: set debug=%x\n",
+			  tunnel->name, tunnel->debug);
 		break;
 
 	default:
@@ -1342,8 +1337,9 @@ static int pppol2tp_session_setsockopt(struct sock *sk,
 			break;
 		}
 		session->recv_seq = val ? -1 : 0;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set recv_seq=%d\n", session->name, session->recv_seq);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL,
+			  "%s: set recv_seq=%d\n",
+			  session->name, session->recv_seq);
 		break;
 
 	case PPPOL2TP_SO_SENDSEQ:
@@ -1358,8 +1354,9 @@ static int pppol2tp_session_setsockopt(struct sock *sk,
 			po->chan.hdrlen = val ? PPPOL2TP_L2TP_HDR_SIZE_SEQ :
 				PPPOL2TP_L2TP_HDR_SIZE_NOSEQ;
 		}
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set send_seq=%d\n", session->name, session->send_seq);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL,
+			  "%s: set send_seq=%d\n",
+			  session->name, session->send_seq);
 		break;
 
 	case PPPOL2TP_SO_LNSMODE:
@@ -1368,20 +1365,22 @@ static int pppol2tp_session_setsockopt(struct sock *sk,
 			break;
 		}
 		session->lns_mode = val ? -1 : 0;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set lns_mode=%d\n", session->name, session->lns_mode);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL,
+			  "%s: set lns_mode=%d\n",
+			  session->name, session->lns_mode);
 		break;
 
 	case PPPOL2TP_SO_DEBUG:
 		session->debug = val;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set debug=%x\n", session->name, session->debug);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: set debug=%x\n",
+			  session->name, session->debug);
 		break;
 
 	case PPPOL2TP_SO_REORDERTO:
 		session->reorder_timeout = msecs_to_jiffies(val);
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: set reorder_timeout=%d\n", session->name, session->reorder_timeout);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL,
+			  "%s: set reorder_timeout=%d\n",
+			  session->name, session->reorder_timeout);
 		break;
 
 	default:
@@ -1460,8 +1459,8 @@ static int pppol2tp_tunnel_getsockopt(struct sock *sk,
 	switch (optname) {
 	case PPPOL2TP_SO_DEBUG:
 		*val = tunnel->debug;
-		PRINTK(tunnel->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get debug=%x\n", tunnel->name, tunnel->debug);
+		l2tp_info(tunnel, PPPOL2TP_MSG_CONTROL, "%s: get debug=%x\n",
+			  tunnel->name, tunnel->debug);
 		break;
 
 	default:
@@ -1483,32 +1482,32 @@ static int pppol2tp_session_getsockopt(struct sock *sk,
 	switch (optname) {
 	case PPPOL2TP_SO_RECVSEQ:
 		*val = session->recv_seq;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get recv_seq=%d\n", session->name, *val);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL,
+			  "%s: get recv_seq=%d\n", session->name, *val);
 		break;
 
 	case PPPOL2TP_SO_SENDSEQ:
 		*val = session->send_seq;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get send_seq=%d\n", session->name, *val);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL,
+			  "%s: get send_seq=%d\n", session->name, *val);
 		break;
 
 	case PPPOL2TP_SO_LNSMODE:
 		*val = session->lns_mode;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get lns_mode=%d\n", session->name, *val);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL,
+			  "%s: get lns_mode=%d\n", session->name, *val);
 		break;
 
 	case PPPOL2TP_SO_DEBUG:
 		*val = session->debug;
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get debug=%d\n", session->name, *val);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL, "%s: get debug=%d\n",
+			  session->name, *val);
 		break;
 
 	case PPPOL2TP_SO_REORDERTO:
 		*val = (int) jiffies_to_msecs(session->reorder_timeout);
-		PRINTK(session->debug, PPPOL2TP_MSG_CONTROL, KERN_INFO,
-		       "%s: get reorder_timeout=%d\n", session->name, *val);
+		l2tp_info(session, PPPOL2TP_MSG_CONTROL,
+			  "%s: get reorder_timeout=%d\n", session->name, *val);
 		break;
 
 	default:
@@ -1871,8 +1870,7 @@ static int __init pppol2tp_init(void)
 		goto out_unregister_pppox;
 #endif
 
-	printk(KERN_INFO "PPPoL2TP kernel driver, %s\n",
-	       PPPOL2TP_DRV_VERSION);
+	pr_info("PPPoL2TP kernel driver, %s\n", PPPOL2TP_DRV_VERSION);
 
 out:
 	return err;

^ permalink raw reply related

* Re: [PATCH 0/4] netfilter fixes for 3.4-rc7
From: Jozsef Kadlecsik @ 2012-05-16 19:48 UTC (permalink / raw)
  To: David Miller; +Cc: pablo, netfilter-devel, netdev
In-Reply-To: <20120516.153913.686915759183202821.davem@davemloft.net>

On Wed, 16 May 2012, David Miller wrote:

> From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Date: Wed, 16 May 2012 21:34:55 +0200 (CEST)
> 
> > On Wed, 16 May 2012, David Miller wrote:
> > 
> >> > Could at least the patch with the subject
> >> > 
> >> >    netfilter: ipset: fix hash size checking in kernel
> >> > 
> >> >    The hash size must fit both into u32 (jhash) and the max value of
> >> >    size_t. The missing checking could lead to kernel crash, bug reported
> >> >    by Seblu.
> >> > 
> >> > be submitted into 3.4-rc7? Any non most-recent ipset package compiled with 
> >> > gcc-4.7 or above can trigger the bug.
> >> 
> >> And only root can trigger it if he gives bogus parameters right?
> > 
> > Yes, root can trigger it only, however it's a two sided bug: ipset < 6.12 
> > compiled with gcc >= 4.7 is broken and sends bogus data to the kernel, 
> > even when normal input parameters are supplied. And unpatched kernel can 
> > crash when acting on the bogus data. :-(
> 
> Ok, that's a more convincing argument.
> 
> I've applied the hash size validation patch to 'net'.

Thank you indeed - I received multiple reports of the crash, so the 
conditions are not uncommon.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

^ permalink raw reply

* Re: [PATCH 0/4] netfilter fixes for 3.4-rc7
From: David Miller @ 2012-05-16 19:39 UTC (permalink / raw)
  To: kadlec; +Cc: pablo, netfilter-devel, netdev
In-Reply-To: <alpine.DEB.2.00.1205162130090.2742@blackhole.kfki.hu>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Date: Wed, 16 May 2012 21:34:55 +0200 (CEST)

> On Wed, 16 May 2012, David Miller wrote:
> 
>> > Could at least the patch with the subject
>> > 
>> >    netfilter: ipset: fix hash size checking in kernel
>> > 
>> >    The hash size must fit both into u32 (jhash) and the max value of
>> >    size_t. The missing checking could lead to kernel crash, bug reported
>> >    by Seblu.
>> > 
>> > be submitted into 3.4-rc7? Any non most-recent ipset package compiled with 
>> > gcc-4.7 or above can trigger the bug.
>> 
>> And only root can trigger it if he gives bogus parameters right?
> 
> Yes, root can trigger it only, however it's a two sided bug: ipset < 6.12 
> compiled with gcc >= 4.7 is broken and sends bogus data to the kernel, 
> even when normal input parameters are supplied. And unpatched kernel can 
> crash when acting on the bogus data. :-(

Ok, that's a more convincing argument.

I've applied the hash size validation patch to 'net'.

^ permalink raw reply

* Re: [PATCH net-next v4] be2net: Fix to allow get/set of debug levels in the firmware.
From: David Miller @ 2012-05-16 19:37 UTC (permalink / raw)
  To: somnath.kotur; +Cc: netdev, bhutchings, suresh.reddy
In-Reply-To: <23cb36ad-2827-44bd-a91a-c6d8b01db70e@exht1.ad.emulex.com>

From: Somnath Kotur <somnath.kotur@emulex.com>
Date: Mon, 14 May 2012 21:59:28 +0530

> Fixed missing paranthesis warning
> Incorporated review comments by Ben Hutchings.
> 
> Signed-off-by: Suresh Reddy <suresh.reddy@emulex.com>
> Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>

Ben H., please review.

Thank you.

^ permalink raw reply

* Re: PROBLEM: Fragmentation issue with 1521 bytes ip packets
From: Omar Alhassane @ 2012-05-16 19:36 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1337094259.8512.1100.camel@edumazet-glaptop>

Thank you for explaining.

Omar

On Tue, May 15, 2012 at 11:04 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> On Tue, 2012-05-15 at 10:00 -0400, Omar Alhassane wrote:
> > Hello Folks,
> >
> > I think i may have found a problem with the linux networking stack.
> > Below is a description of the problem.
> >
> > [1.] One line summary of the problem:
> > No response to pings of certain sizes.
> >
> > [2.] Full description of the problem/report:
> > Using hping3, when i ping a linux machine with 1521 bytes ip packets i
> > get only one response.
> > But when i use 1482 bytes, everything works fine. I've tried this with
> > both tcp and udp. The MTU of my interface is 1500.
> > [3.] Keywords (i.e., modules, networking, kernel):
> > ip, udp, tcp, networking, fragmentation
> > [4.] Kernel version (from /proc/version):
> > 3.3.1
> > [5.] Output of Oops.. message (if applicable) with symbolic information
> > [6.] A small shell script or example program which triggers the
> > problem (if possible)
> > The following commands works only if the target has tcp port 22 open
> >
> > hping3 -d 1481 -S -P 22 10.0.30.225 (only one response)
> > hping3 -d 1482 -S -P 22 10.0.30.225 (works fine)
> >
> > Can somebody confirm if this is a problem?
>
> hping3 bug : All the fragments it sends have the same ID field.
>
> First 2 frags are reassembled by remote. Remote sends a SYNACK.
>
>
> Following frags are 'ignored' because they have same ID than previous
> packet.
>
>
>
>
>

^ permalink raw reply

* [PATCH v3] xfrm: take iphdr size into account for esp payload size calculation
From: Benjamin Poirier @ 2012-05-16 19:35 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, linux-kernel,
	Steffen Klassert
In-Reply-To: <20120514.183952.1459413342269893446.davem@davemloft.net>

Corrects the function that determines the esp payload size.
The calculations done in esp4_get_mtu lead to overlength frames in transport
mode for certain mtu values and suboptimal frames for others.

According to what is done, mainly in esp_output(), net_header_len aka
sizeof(struct iphdr) must be taken into account before doing the alignment
calculation.

Signed-off-by: Benjamin Poirier <bpoirier@suse.de>

---
Changes since v2:
* rename l3_adj to net_adj
* fix indentation

Changes since v1:
* introduce l3_adj to preserve the same returned value as before for tunnel
  mode

For example, with md5 AH and 3des ESP (transport mode):
mtu = 1499 leads to FRAGFAILS
mtu = 1500 the addition of padding in the esp header could be avoided

Tested with
* transport mode E
* transport mode EA
* transport mode E + ah
* tunnel mode E

Not tested with BEET, but it should be the same as transport mode
	draft-nikander-esp-beet-mode-03.txt Section 5.2:
	"The wire packet format is identical to the ESP transport mode"
---
 net/ipv4/esp4.c |   24 +++++++++---------------
 1 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 89a47b3..cb982a6 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -459,28 +459,22 @@ static u32 esp4_get_mtu(struct xfrm_state *x, int mtu)
 	struct esp_data *esp = x->data;
 	u32 blksize = ALIGN(crypto_aead_blocksize(esp->aead), 4);
 	u32 align = max_t(u32, blksize, esp->padlen);
-	u32 rem;
-
-	mtu -= x->props.header_len + crypto_aead_authsize(esp->aead);
-	rem = mtu & (align - 1);
-	mtu &= ~(align - 1);
+	unsigned int net_adj;
 
 	switch (x->props.mode) {
-	case XFRM_MODE_TUNNEL:
-		break;
-	default:
 	case XFRM_MODE_TRANSPORT:
-		/* The worst case */
-		mtu -= blksize - 4;
-		mtu += min_t(u32, blksize - 4, rem);
-		break;
 	case XFRM_MODE_BEET:
-		/* The worst case. */
-		mtu += min_t(u32, IPV4_BEET_PHMAXLEN, rem);
+		net_adj = sizeof(struct iphdr);
 		break;
+	case XFRM_MODE_TUNNEL:
+		net_adj = 0;
+		break;
+	default:
+		BUG();
 	}
 
-	return mtu - 2;
+	return ((mtu - x->props.header_len - crypto_aead_authsize(esp->aead) -
+		 net_adj) & ~(align - 1)) + (net_adj - 2);
 }
 
 static void esp4_err(struct sk_buff *skb, u32 info)
-- 
1.7.7

^ permalink raw reply related

* Re: [PATCH 0/4] netfilter fixes for 3.4-rc7
From: Jozsef Kadlecsik @ 2012-05-16 19:34 UTC (permalink / raw)
  To: David Miller; +Cc: pablo, netfilter-devel, netdev
In-Reply-To: <20120516.151836.786389543745557157.davem@davemloft.net>

On Wed, 16 May 2012, David Miller wrote:

> > Could at least the patch with the subject
> > 
> >    netfilter: ipset: fix hash size checking in kernel
> > 
> >    The hash size must fit both into u32 (jhash) and the max value of
> >    size_t. The missing checking could lead to kernel crash, bug reported
> >    by Seblu.
> > 
> > be submitted into 3.4-rc7? Any non most-recent ipset package compiled with 
> > gcc-4.7 or above can trigger the bug.
> 
> And only root can trigger it if he gives bogus parameters right?

Yes, root can trigger it only, however it's a two sided bug: ipset < 6.12 
compiled with gcc >= 4.7 is broken and sends bogus data to the kernel, 
even when normal input parameters are supplied. And unpatched kernel can 
crash when acting on the bogus data. :-(
 
> If that's the case, the exposure is to privileged users committing an
> operator error, so I don't see it as so important.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

^ permalink raw reply

* Re: [PATCH net-next v5 0/13] basic ieee802.15.4 mac support
From: David Miller @ 2012-05-16 19:34 UTC (permalink / raw)
  To: alex.bluesman.smirnov; +Cc: netdev, dbaryshkov
In-Reply-To: <1337151031-5178-1-git-send-email-alex.bluesman.smirnov@gmail.com>

From: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
Date: Wed, 16 May 2012 10:50:18 +0400

> This is the 5-th version of patch series for IEEE 802.15.4 Medium Access
> Control layer support.
> 
> Changes since the last post:
>  - All the comments were polished to the same style
>  - Some new extra comments were added

All applied, thanks.

^ permalink raw reply

* Re: [patch 0/2] s390: qeth patches for net-next
From: David Miller @ 2012-05-16 19:33 UTC (permalink / raw)
  To: frank.blaschka; +Cc: netdev, linux-s390
In-Reply-To: <20120516112824.602200177@de.ibm.com>

From: frank.blaschka@de.ibm.com
Date: Wed, 16 May 2012 13:28:24 +0200

> here are 2 qeth patches for net-next
> 
> shortlog:
> 
> Ursula Braun (1)
> qeth: recognize vlan devices in layer3 mode
> 
> Frank Blaschka (1)
> qeth: remove token ring part 2

All applied, thanks Frank.

^ permalink raw reply

* Re: [PATCH net-next] fq_codel: should use qdisc backlog as threshold
From: David Miller @ 2012-05-16 19:30 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, nichols, dave.taht, van, codel, bloat
In-Reply-To: <1337179149.8512.1208.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 16 May 2012 16:39:09 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> codel_should_drop() logic allows a packet being not dropped if queue
> size is under max packet size.
> 
> In fq_codel, we have two possible backlogs : The qdisc global one, and
> the flow local one.
> 
> The meaningful one for codel_should_drop() should be the global backlog,
> not the per flow one, so that thin flows can have a non zero drop/mark
> probability.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] net: sock_flag() cleanup
From: David Miller @ 2012-05-16 19:30 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1337183827.8512.1215.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 16 May 2012 17:57:07 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> - sock_flag() accepts a const pointer
> 
> - sock_flag() returns a boolean
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2 0/8] mISDN: Fixes and enhancements for the data channels
From: David Miller @ 2012-05-16 19:28 UTC (permalink / raw)
  To: kkeil; +Cc: netdev
In-Reply-To: <1337161868-19399-1-git-send-email-kkeil@linux-pingi.de>

From: Karsten Keil <kkeil@linux-pingi.de>
Date: Wed, 16 May 2012 11:51:00 +0200

> v2: All fixes from Dave Millers review
>    - comment format
>    - use bool type for boolean parameter
>    - use (var == CONSTANT)
> 
> This series improve the stability of streaming raw voice data when the
> system is under high load. With the fixes in place you can send and
> receive multiple faximilies (using SPANDSP) in parallel
> while compiling a kernel without a getting a transfer aborted.
> 
> for net-next

All applied to net-next.

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] 6lowpan: code updates
From: David Miller @ 2012-05-16 19:28 UTC (permalink / raw)
  To: alex.bluesman.smirnov; +Cc: netdev
In-Reply-To: <1337153248-5779-1-git-send-email-alex.bluesman.smirnov@gmail.com>

From: Alexander Smirnov <alex.bluesman.smirnov@gmail.com>
Date: Wed, 16 May 2012 11:27:26 +0400

>   6lowpan: rework data fetching from skb

This is not what we told you to do.

We told you that IF you were going to emit a warning message
for the pskb_may_pull() failure condition, you should use
WARN_ON_ONCE() so that it doesn't potentially flood the
logs.

But you must always, in every case, handle the error in some
reasonable way, not just when WARN_ON_ONCE() does that initial
one-and-only trigger.

^ permalink raw reply

* [PATCH] bonding: only send arp monitor packets if no other traffic
From: Chris Friesen @ 2012-05-16 18:48 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: netdev

In order to minimize network traffic, when using load balancing modes
only send out arp monitor packets if it's been more than delta_in_ticks
jiffies since we either received or transmitted packets.  The rationale
behind this is that if there is a lot of other traffic going on we don't
need the arp monitor packets to determine whether or not the link is
working.

This makes the most difference if you have a lot of hosts all arping
the same target at high frequency.

Signed-off-by: Chris Friesen <chris.friesen@genand.com>
---
 drivers/net/bonding/bond_main.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index bc13b3d..4c8459a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2885,8 +2885,12 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
 		 * do - all replies will be rx'ed on same link causing slaves
 		 * to be unstable during low/no traffic periods
 		 */
-		if (IS_UP(slave->dev))
-			bond_arp_send_all(bond, slave);
+		if (IS_UP(slave->dev)) {
+			if (time_after_eq(jiffies, dev_trans_start(slave->dev) + delta_in_ticks) ||
+			    time_after_eq(jiffies, slave->dev->last_rx + delta_in_ticks)) {
+				bond_arp_send_all(bond, slave);
+			}
+		}
 	}
 
 	if (do_failover) {

-- 

Chris Friesen
Software Designer
3500 Carling Avenue
Ottawa, Ontario K2H 8E9
www.genband.com

^ permalink raw reply related

* Re: [PATCH 0/4] netfilter fixes for 3.4-rc7
From: David Miller @ 2012-05-16 19:18 UTC (permalink / raw)
  To: kadlec; +Cc: pablo, netfilter-devel, netdev
In-Reply-To: <alpine.DEB.2.00.1205162037110.2742@blackhole.kfki.hu>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Date: Wed, 16 May 2012 20:41:51 +0200 (CEST)

> Could at least the patch with the subject
> 
>    netfilter: ipset: fix hash size checking in kernel
> 
>    The hash size must fit both into u32 (jhash) and the max value of
>    size_t. The missing checking could lead to kernel crash, bug reported
>    by Seblu.
> 
> be submitted into 3.4-rc7? Any non most-recent ipset package compiled with 
> gcc-4.7 or above can trigger the bug.

And only root can trigger it if he gives bogus parameters right?

If that's the case, the exposure is to privileged users committing an
operator error, so I don't see it as so important.

^ permalink raw reply

* Re: [PATCH] bonding: only send arp monitor packets if no other traffic
From: Jay Vosburgh @ 2012-05-16 19:08 UTC (permalink / raw)
  To: Chris Friesen; +Cc: netdev
In-Reply-To: <4FB3F67C.6000401@genband.com>

Chris Friesen <chris.friesen@genband.com> wrote:

>In order to minimize network traffic, when using load balancing modes
>only send out arp monitor packets if it's been more than delta_in_ticks
>jiffies since we either received or transmitted packets.  The rationale
>behind this is that if there is a lot of other traffic going on we don't
>need the arp monitor packets to determine whether or not the link is
>working.
>
>This makes the most difference if you have a lot of hosts all arping
>the same target at high frequency.

	This logic would not work for the active-backup case (it would
break arp_validate, for one thing), but might be ok for the loadbalance
(balance-xor, balance-rr) case.

	This might adversely affect cases where the switch ports are not
configured into a port channel; in that case, the ARP broadcasts would
be sent to all slaves, but with this patch, will no longer be.  That's
technically not a correct configuration, but seems to be in use
nevertheless.

	I didn't think that the ARP monitor was particularly popular for
the loadbalance case, since it is not as reliable.  It depends upon the
switch to insure that some traffic comes in to each slave, and low
traffic periods can result in false detection of link failures.  Even
with the ARPs being sent out, if those are not evenly balanced by the
switch, false failure detections can occur.

	-J


>Signed-off-by: Chris Friesen <chris.friesen@genand.com>
>---
> drivers/net/bonding/bond_main.c |    8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index bc13b3d..4c8459a 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2885,8 +2885,12 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
> 		 * do - all replies will be rx'ed on same link causing slaves
> 		 * to be unstable during low/no traffic periods
> 		 */
>-		if (IS_UP(slave->dev))
>-			bond_arp_send_all(bond, slave);
>+		if (IS_UP(slave->dev)) {
>+			if (time_after_eq(jiffies, dev_trans_start(slave->dev) + delta_in_ticks) ||
>+			    time_after_eq(jiffies, slave->dev->last_rx + delta_in_ticks)) {
>+				bond_arp_send_all(bond, slave);
>+			}
>+		}
> 	}
>
> 	if (do_failover) {
>
>-- 
>
>Chris Friesen
>Software Designer
>3500 Carling Avenue
>Ottawa, Ontario K2H 8E9
>www.genband.com

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [V2 PATCH 9/9] vhost: zerocopy: poll vq in zerocopy callback
From: Shirley Ma @ 2012-05-16 19:08 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jason Wang, eric.dumazet, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120516183629.GJ10769@redhat.com>

On Wed, 2012-05-16 at 21:36 +0300, Michael S. Tsirkin wrote:
> On Wed, May 16, 2012 at 10:32:05AM -0700, Shirley Ma wrote:
> > On Wed, 2012-05-16 at 18:14 +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 16, 2012 at 08:10:27AM -0700, Shirley Ma wrote:
> > > > On Wed, 2012-05-16 at 10:58 +0800, Jason Wang wrote:
> > > > > >>   drivers/vhost/vhost.c |    1 +
> > > > > >>   1 files changed, 1 insertions(+), 0 deletions(-)
> > > > > >>
> > > > > >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > > >> index 947f00d..7b75fdf 100644
> > > > > >> --- a/drivers/vhost/vhost.c
> > > > > >> +++ b/drivers/vhost/vhost.c
> > > > > >> @@ -1604,6 +1604,7 @@ void vhost_zerocopy_callback(void
> *arg)
> > > > > >>          struct vhost_ubuf_ref *ubufs = ubuf->arg;
> > > > > >>          struct vhost_virtqueue *vq = ubufs->vq;
> > > > > >>
> > > > > >> +       vhost_poll_queue(&vq->poll);
> > > > > >>          /* set len = 1 to mark this desc buffers done DMA
> */
> > > > > >>          vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
> > > > > >>          kref_put(&ubufs->kref,
> vhost_zerocopy_done_signal);
> > > > > > Doing so, we might have redundant vhost_poll_queue(). Do you
> > > know in
> > > > > > which scenario there might be missing of adding and
> signaling
> > > during
> > > > > > zerocopy?
> > > > > 
> > > > > Yes, as we only do signaling and adding during tx work, if
> there's
> > > no
> > > > > tx 
> > > > > work when the skb were sent, we may lose the opportunity to
> let
> > > guest 
> > > > > know about the completion. It's easy to be reproduced with
> netperf
> > > > > test. 
> > > > 
> > > > The reason which host signals guest is to free guest tx buffers,
> if
> > > > there is no tx work, then it's not necessary to signal the guest
> > > unless
> > > > guest runs out of memory. The pending buffers will be released
> > > > virtio_net device gone.
> > > > 
> > > > What's the behavior of netperf test when you hit this situation?
> > > > 
> > > > Thanks
> > > > Shirley
> > > 
> > > IIRC guest networking seems to be lost. 
> > 
> > It seems vhost_enable_notify is missing in somewhere else?
> > 
> > Thanks
> > Shirley
> 
> Donnu. I see virtio sending packets but they do not get
> to tun on host. debugging. 

I looked at the code, if (zerocopy) check is needed for below code:

+	if (zerocopy) {
                        num_pends = likely(vq->upend_idx >= vq->done_idx) ?
                                    (vq->upend_idx - vq->done_idx) :
                                    (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
                        if (unlikely(num_pends > VHOST_MAX_PEND)) {
                                tx_poll_start(net, sock);
				vhost_poll_queue
                                set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
                                break;
                        }
+	}
                        if (unlikely(vhost_enable_notify(&net->dev, vq))) {
                                vhost_disable_notify(&net->dev, vq);
                                continue;
                        }
                        break;


Second, whether it's possible the problem comes from tx_poll_start()? In
some situation, vhost_poll_wakeup() is not being called?

Thanks
Shirley

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox