pktgen performance hit due to memset.

pktgen performance hit due to memset.

[Ben Greear]

Some time back, someone added some memset() calls to pktgen to
keep from leaking memory contents to the network.

At least in our modified version of pktgen, this caused about 25%
performance degradation when sending 1514 byte pkts (multi-pkt == 0)
on a pair of 10G ports.  It was easy enough to comment these memset
calls out of course.

I don't mind if this patch stays in,
but thought I'd post my findings in case anyone else wonders why
their pktgen slowed down...

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: pktgen performance hit due to memset.

[David Miller]

From: Ben Greear <greearb@candelatech.com>
Date: Fri, 23 Jul 2010 16:14:19 -0700

> Some time back, someone added some memset() calls to pktgen to
> keep from leaking memory contents to the network.
> 
> At least in our modified version of pktgen, this caused about 25%
> performance degradation when sending 1514 byte pkts (multi-pkt == 0)
> on a pair of 10G ports.  It was easy enough to comment these memset
> calls out of course.
> 
> I don't mind if this patch stays in,
> but thought I'd post my findings in case anyone else wonders why
> their pktgen slowed down...

Thanks for the data point Ben.

[PATCH net-next-2.6] pktgen: Optionally leak kernel memory

[Eric Dumazet]

Le vendredi 23 juillet 2010 à 16:14 -0700, Ben Greear a écrit :
> Some time back, someone added some memset() calls to pktgen to
> keep from leaking memory contents to the network.
> 

Well, someone might be me ;)

> At least in our modified version of pktgen, this caused about 25%
> performance degradation when sending 1514 byte pkts (multi-pkt == 0)
> on a pair of 10G ports.  It was easy enough to comment these memset
> calls out of course.
> 
> I don't mind if this patch stays in,
> but thought I'd post my findings in case anyone else wonders why
> their pktgen slowed down...
> 

Thanks Ben

Here is a patch adding a new pktgen flag, so that admins can choose
speed if they want to, if they dont use clone_skb to reduce skb setup
costs.

Oc course, admins could change pktgen code themselves, but as you said,
better document it so that admins are aware of this possible speed
increase.

[PATCH net-next-2.6] pktgen: Optionally leak kernel memory

Commit 66ed1e5ec1d979 (pktgen: Dont leak kernel memory)
closed a security hole, by making sure data sent to network was cleared,
instead of using previous content of pages.

As Ben Greear noticed, this can slow down pktgen as much as 25%.

Add a new pktgen flag, UNSAFE, to ask pktgen to not clear data and use
previous content of memory.

Also add documentation for UNSAFE and NODE_ALLOC flags

Reported-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 Documentation/networking/pktgen.txt |   24 ++++++++++++++++++++++-
 net/core/pktgen.c                   |   27 +++++++++++++++++++++-----
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt
index 75e4fd7..88e2e6f 100644
--- a/Documentation/networking/pktgen.txt
+++ b/Documentation/networking/pktgen.txt
@@ -108,7 +108,10 @@ Examples:
                               MPLS_RND, VID_RND, SVID_RND
                               QUEUE_MAP_RND # queue map random
                               QUEUE_MAP_CPU # queue map mirrors smp_processor_id()
-
+                              NODE_ALLOC # NUMA aware skb allocations
+                              UNSAFE # Dont clear packets payload
+                                      (Might be 25% faster, but can leak sensitive
+                                       data to network. Use at your own risk !)
 
  pgset "udp_src_min 9"   set UDP source port min, If < udp_src_max, then
                          cycle through the port range.
@@ -178,6 +181,18 @@ Note when adding devices to a specific CPU there good idea to also assign
 as this reduces cache bouncing when freeing skb's.
 
 
+Very fast mode
+==============
+One knob to get very fast pktgen is the UNSAFE flag :
+
+flag UNSAFE
+
+This ask to pktgen to not clear content of packets before sending them.
+Note this is a security problem, and should be used only if really needed.
+If packets are cloned (clone_skb 1000), clearing data cost is amortized so
+this UNSAFE mode is less interesting.
+
+
 Current commands and configuration options
 ==========================================
 
@@ -225,6 +240,13 @@ flag
   UDPDST_RND
   MACSRC_RND
   MACDST_RND
+  MPLS_RND
+  VID_RND
+  SVID_RND
+  FLOW_SEQ
+  IPSEC
+  NODE_ALLOC
+  UNSAFE
 
 dst_min
 dst_max
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 24a19de..01990cb 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -172,7 +172,7 @@
 #include <asm/dma.h>
 #include <asm/div64.h>		/* do_div */
 
-#define VERSION	"2.74"
+#define VERSION	"2.75"
 #define IP_NAME_SZ 32
 #define MAX_MPLS_LABELS 16 /* This is the max label stack depth */
 #define MPLS_STACK_BOTTOM htonl(0x00000100)
@@ -196,6 +196,7 @@
 #define F_QUEUE_MAP_RND (1<<13)	/* queue map Random */
 #define F_QUEUE_MAP_CPU (1<<14)	/* queue map mirrors smp_processor_id() */
 #define F_NODE          (1<<15)	/* Node memory alloc*/
+#define F_UNSAFE        (1<<16)	/* Payload of packets is left uninitialized */
 
 /* Thread control flag bits */
 #define T_STOP        (1<<0)	/* Stop run */
@@ -674,6 +675,9 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
 	if (pkt_dev->flags & F_NODE)
 		seq_printf(seq, "NODE_ALLOC  ");
 
+	if (pkt_dev->flags & F_UNSAFE)
+		seq_printf(seq, "UNSAFE  ");
+
 	seq_puts(seq, "\n");
 
 	/* not really stopped, more like last-running-at */
@@ -1231,12 +1235,20 @@ static ssize_t pktgen_if_write(struct file *file,
 		else if (strcmp(f, "!NODE_ALLOC") == 0)
 			pkt_dev->flags &= ~F_NODE;
 
+		else if (strcmp(f, "UNSAFE") == 0)
+			pkt_dev->flags |= F_UNSAFE;
+
+		else if (strcmp(f, "!UNSAFE") == 0)
+			pkt_dev->flags &= ~F_UNSAFE;
+
 		else {
 			sprintf(pg_result,
 				"Flag -:%s:- unknown\nAvailable flags, (prepend ! to un-set flag):\n%s",
 				f,
 				"IPSRC_RND, IPDST_RND, UDPSRC_RND, UDPDST_RND, "
-				"MACSRC_RND, MACDST_RND, TXSIZE_RND, IPV6, MPLS_RND, VID_RND, SVID_RND, FLOW_SEQ, IPSEC, NODE_ALLOC\n");
+				"MACSRC_RND, MACDST_RND, TXSIZE_RND, IPV6, "
+				"MPLS_RND, VID_RND, SVID_RND, FLOW_SEQ, IPSEC, "
+				"NODE_ALLOC, UNSAFE\n");
 			return count;
 		}
 		sprintf(pg_result, "OK: flags=0x%x", pkt_dev->flags);
@@ -2723,7 +2735,8 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
 
 	if (pkt_dev->nfrags <= 0) {
 		pgh = (struct pktgen_hdr *)skb_put(skb, datalen);
-		memset(pgh + 1, 0, datalen - sizeof(struct pktgen_hdr));
+		if (!(pkt_dev->flags & F_UNSAFE))
+			memset(pgh + 1, 0, datalen - sizeof(struct pktgen_hdr));
 	} else {
 		int frags = pkt_dev->nfrags;
 		int i, len;
@@ -2734,13 +2747,17 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
 			frags = MAX_SKB_FRAGS;
 		if (datalen > frags * PAGE_SIZE) {
 			len = datalen - frags * PAGE_SIZE;
-			memset(skb_put(skb, len), 0, len);
+			if (!(pkt_dev->flags & F_UNSAFE))
+				memset(skb_put(skb, len), 0, len);
 			datalen = frags * PAGE_SIZE;
 		}
 
 		i = 0;
 		while (datalen > 0) {
-			struct page *page = alloc_pages(GFP_KERNEL | __GFP_ZERO, 0);
+			struct page *page = alloc_pages((pkt_dev->flags & F_UNSAFE) ?
+								GFP_KERNEL :
+								GFP_KERNEL | __GFP_ZERO,
+							0);
 			skb_shinfo(skb)->frags[i].page = page;
 			skb_shinfo(skb)->frags[i].page_offset = 0;
 			skb_shinfo(skb)->frags[i].size =

Re: [PATCH net-next-2.6] pktgen: Optionally leak kernel memory

[Ben Greear]

On 07/23/2010 10:23 PM, Eric Dumazet wrote:
> Le vendredi 23 juillet 2010 à 16:14 -0700, Ben Greear a écrit :
>> Some time back, someone added some memset() calls to pktgen to
>> keep from leaking memory contents to the network.
>>
>
> Well, someone might be me ;)
>
>> At least in our modified version of pktgen, this caused about 25%
>> performance degradation when sending 1514 byte pkts (multi-pkt == 0)
>> on a pair of 10G ports.  It was easy enough to comment these memset
>> calls out of course.
>>
>> I don't mind if this patch stays in,
>> but thought I'd post my findings in case anyone else wonders why
>> their pktgen slowed down...
>>
>
> Thanks Ben
>
> Here is a patch adding a new pktgen flag, so that admins can choose
> speed if they want to, if they dont use clone_skb to reduce skb setup
> costs.

It looks fine to me, though I have not actually tested it.

> +Very fast mode
> +==============
> +One knob to get very fast pktgen is the UNSAFE flag :
> +
> +flag UNSAFE
> +
> +This ask to pktgen to not clear content of packets before sending them.
> +Note this is a security problem, and should be used only if really needed.
> +If packets are cloned (clone_skb 1000), clearing data cost is amortized so
> +this UNSAFE mode is less interesting.

I think most users of pktgen wouldn't be too concerned about leaking
memory content to the network.  It's a root-only test tool that can easily
saturate most networks and do horrible things like overflow switch CAM tables
by randomizing source/dest macs etc.  So, this warning might could be a bit
more descriptive of how it is a security problem "arbitrary contents of memory can be
sent across the network and may be sniffed by devices on the network, potentially
revealing private information such as passwords and application data for applications
running on the machine running pktgen" instead of telling folks not to use it unless it's
really needed.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH net-next-2.6] pktgen: Optionally leak kernel memory

[Eric Dumazet]

Le samedi 24 juillet 2010 à 06:18 -0700, Ben Greear a écrit :

> I think most users of pktgen wouldn't be too concerned about leaking
> memory content to the network.  It's a root-only test tool that can easily
> saturate most networks and do horrible things like overflow switch CAM tables
> by randomizing source/dest macs etc.  So, this warning might could be a bit
> more descriptive of how it is a security problem "arbitrary contents of memory can be
> sent across the network and may be sniffed by devices on the network, potentially
> revealing private information such as passwords and application data for applications
> running on the machine running pktgen" instead of telling folks not to use it unless it's
> really needed.

Most of the horrible things you mention are not related to the memset()
thing, arent they ?


Being root means : "I am a trusted user on this machine, and as such,
must know a bit what security means".

It doesnt mean : "I am allowed to steal passwords, credit card numbers,
from gentle users. I am allowed to blow up the LAN with billions of evil
frames". Still, pktgen is there and might be used by a fool.

The "UNSAFE" label should be more than enough to warn the fool admin ;)

Note this "UNSAFE" thing is really bad. Nowhere in the kernel we are
allowed to make this sort of thing : No special mmap() flag asking
kernel to give non cleared memory pages, even to root user.

I am not sure David will accept the patch !

Anyway, as I said, if you want to saturate a 10Gb+ network with pktgen,
you probably need clone_skb ?

Re: [PATCH net-next-2.6] pktgen: Optionally leak kernel memory

[Ben Greear]

On 07/24/2010 07:13 AM, Eric Dumazet wrote:
> Le samedi 24 juillet 2010 à 06:18 -0700, Ben Greear a écrit :
>
>> I think most users of pktgen wouldn't be too concerned about leaking
>> memory content to the network.  It's a root-only test tool that can easily
>> saturate most networks and do horrible things like overflow switch CAM tables
>> by randomizing source/dest macs etc.  So, this warning might could be a bit
>> more descriptive of how it is a security problem "arbitrary contents of memory can be
>> sent across the network and may be sniffed by devices on the network, potentially
>> revealing private information such as passwords and application data for applications
>> running on the machine running pktgen" instead of telling folks not to use it unless it's
>> really needed.
>
> Most of the horrible things you mention are not related to the memset()
> thing, arent they ?
>
>
> Being root means : "I am a trusted user on this machine, and as such,
> must know a bit what security means".
>
> It doesnt mean : "I am allowed to steal passwords, credit card numbers,
> from gentle users. I am allowed to blow up the LAN with billions of evil
> frames". Still, pktgen is there and might be used by a fool.

Out of curiosity, couldn't root just use gdb, strace or similar means to
get access to user's programs?  Or add a simple module to the kernel to
dump memory pages for that matter?

It would seem to me that this UNSAFE flag is only to protect root users from
accidentally sharing their own private memory accidentally.

> The "UNSAFE" label should be more than enough to warn the fool admin ;)
>
> Note this "UNSAFE" thing is really bad. Nowhere in the kernel we are
> allowed to make this sort of thing : No special mmap() flag asking
> kernel to give non cleared memory pages, even to root user.

Ok, I don't mind either way.  I have a bunch of hacks to pktgen in
my tree already, so one more isn't a big deal.

> Anyway, as I said, if you want to saturate a 10Gb+ network with pktgen,
> you probably need clone_skb ?

I can get bi-directional 9.6Gbps or so using 1514 byte pkts and clone-skb == 0
on two ports using Intel 82599 10G NIC on core-i7 3.33Ghz (6GT/s pci-e bus).
(with memsets commented out).  This is around 40Gbps total data across the network
interfaces.

Some day I'll get a quad or 6-port 10G and see what it can do :)

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH net-next-2.6] pktgen: Optionally leak kernel memory

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 24 Jul 2010 16:13:15 +0200

> I am not sure David will accept the patch !

I don't think I can apply this, sorry :-)

Re: [PATCH net-next-2.6] pktgen: Optionally leak kernel memory

[Eric Dumazet]

Le samedi 24 juillet 2010 à 21:35 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sat, 24 Jul 2010 16:13:15 +0200
> 
> > I am not sure David will accept the patch !
> 
> I don't think I can apply this, sorry :-)

Absolutely.

It might be possible for pktgen to use a pool of prebuilt pages to avoid
the cost of clearing pages. This wont work for skb without frags, unless
we change skb_release_data() (it calls kfree(skb->head), I dont think we
can trap this one...)

One better idea would be to take an extra reference on skb before giving
it to transmit, and maintain a list of skbs to recycle once their
refcount hits 1 (our reference). We could avoid most of the skb
setup/freeing costs (no more memory allocations/freeing)

I'll take a look after my vacations, unless someone motivated enough
beats me of course :)

Thanks !