* [PATCH 1/2] net: Allow no-cache copy from user on transmit
@ 2011-03-09 5:05 Tom Herbert
2011-03-09 6:12 ` Stephen Hemminger
2011-03-09 13:04 ` Michał Mirosław
0 siblings, 2 replies; 6+ messages in thread
From: Tom Herbert @ 2011-03-09 5:05 UTC (permalink / raw)
To: davem, netdev
This patch uses __copy_from_user_nocache (from skb_copy_to_page)
on transmit to bypass data cache for a performance improvement.
This functionality is configurable per device using ethtool, the
device must also be doing TX csum offload to enable. It seems
reasonable to set this when the device does not copy or
otherwise touch the data.
This patch was tested using 200 instances of netperf TCP_RR with
1400 byte request and one byte reply. Platform is 16 core AMD.
No-cache copy disabled:
672703 tps, 97.13% client utilization
50/90/99% latency 244.31 484.205 1028.41
No-cache copy enabled:
702113 tps, 96.16% client utilization,
50/90/99% latency 238.56 467.56 956.955
This seems to provide a nice little performance improvement and is
consistent in the tests I ran. Presumably, this would provide
the greatest benfits in the presence of an application workload
stressing the cache and a lot of transmit data happening. I don't
yet see a downside to using this.
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/linux/ethtool.h | 4 ++++
include/linux/netdevice.h | 4 +++-
include/net/sock.h | 5 +++++
net/core/ethtool.c | 11 ++++++++++-
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index aac3e2e..2cf3cc9 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -833,6 +833,10 @@ struct ethtool_ops {
#define ETHTOOL_GFEATURES 0x0000003a /* Get device offload settings */
#define ETHTOOL_SFEATURES 0x0000003b /* Change device offload settings */
+#define ETHTOOL_GNCCOPY 0x0000003c /* Get no-cache-copy enable
+ * (ethtool_value). */
+#define ETHTOOL_SNCCOPY 0x0000003d /* Set no-cache-copy enable
+ * (ethtool_value). */
/* compatibility with older code */
#define SPARC_ETH_GSET ETHTOOL_GSET
#define SPARC_ETH_SSET ETHTOOL_SSET
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 71563e7..1b72f29 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -978,6 +978,7 @@ struct net_device {
#define NETIF_F_NTUPLE (1 << 27) /* N-tuple filters supported */
#define NETIF_F_RXHASH (1 << 28) /* Receive hashing offload */
#define NETIF_F_RXCSUM (1 << 29) /* Receive checksumming offload */
+#define NETIF_F_NOCACHE_COPY (1 << 30) /* Use no-cache copyfromuser */
/* Segmentation offload features */
#define NETIF_F_GSO_SHIFT 16
@@ -1020,7 +1021,8 @@ struct net_device {
NETIF_F_FRAGLIST)
/* changeable features with no special hardware requirements */
-#define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO)
+#define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO | \
+ NETIF_F_NOCACHE_COPY)
/* Interface index. Unique device identifier */
int ifindex;
diff --git a/include/net/sock.h b/include/net/sock.h
index da0534d..55b1385 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1401,6 +1401,11 @@ static inline int skb_copy_to_page(struct sock *sk, char __user *from,
if (err)
return err;
skb->csum = csum_block_add(skb->csum, csum, skb->len);
+ } else if (sk->sk_route_caps & NETIF_F_NOCACHE_COPY) {
+ if (!access_ok(VERIFY_READ, from, copy)
+ || __copy_from_user_nocache(page_address(page) + off,
+ from, copy))
+ return -EFAULT;
} else if (copy_from_user(page_address(page) + off, from, copy))
return -EFAULT;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index c1a71bb..a8b2638 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -328,6 +328,7 @@ static const char netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
/* NETIF_F_NETNS_LOCAL */ "netns-local",
/* NETIF_F_GRO */ "rx-gro",
/* NETIF_F_LRO */ "rx-lro",
+ /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy",
/* NETIF_F_TSO */ "tx-tcp-segmentation",
/* NETIF_F_UFO */ "tx-udp-fragmentation",
@@ -336,7 +337,6 @@ static const char netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
/* NETIF_F_TSO6 */ "tx-tcp6-segmentation",
/* NETIF_F_FSO */ "tx-fcoe-segmentation",
"",
- "",
/* NETIF_F_FCOE_CRC */ "tx-checksum-fcoe-crc",
/* NETIF_F_SCTP_CSUM */ "tx-checksum-sctp",
@@ -400,6 +400,9 @@ static u32 ethtool_get_feature_mask(u32 eth_cmd)
case ETHTOOL_GGRO:
case ETHTOOL_SGRO:
return NETIF_F_GRO;
+ case ETHTOOL_GNCCOPY:
+ case ETHTOOL_SNCCOPY:
+ return NETIF_F_NOCACHE_COPY;
default:
BUG();
}
@@ -1487,6 +1490,9 @@ static int __ethtool_set_tx_csum(struct net_device *dev, u32 data)
return err;
}
+ if (!data)
+ dev->features &= ~NETIF_F_NOCACHE_COPY;
+
return dev->ethtool_ops->set_tx_csum(dev, data);
}
@@ -1768,6 +1774,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_GUFO:
case ETHTOOL_GGSO:
case ETHTOOL_GGRO:
+ case ETHTOOL_GNCCOPY:
case ETHTOOL_GFLAGS:
case ETHTOOL_GPFLAGS:
case ETHTOOL_GRXFH:
@@ -1924,6 +1931,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_GUFO:
case ETHTOOL_GGSO:
case ETHTOOL_GGRO:
+ case ETHTOOL_GNCCOPY:
rc = ethtool_get_one_feature(dev, useraddr, ethcmd);
break;
case ETHTOOL_STXCSUM:
@@ -1933,6 +1941,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
case ETHTOOL_SUFO:
case ETHTOOL_SGSO:
case ETHTOOL_SGRO:
+ case ETHTOOL_SNCCOPY:
rc = ethtool_set_one_feature(dev, useraddr, ethcmd);
break;
default:
--
1.7.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] net: Allow no-cache copy from user on transmit
2011-03-09 5:05 [PATCH 1/2] net: Allow no-cache copy from user on transmit Tom Herbert
@ 2011-03-09 6:12 ` Stephen Hemminger
2011-03-09 13:04 ` Michał Mirosław
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2011-03-09 6:12 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev
On Tue, 8 Mar 2011 21:05:06 -0800 (PST)
Tom Herbert <therbert@google.com> wrote:
> This patch uses __copy_from_user_nocache (from skb_copy_to_page)
> on transmit to bypass data cache for a performance improvement.
> This functionality is configurable per device using ethtool, the
> device must also be doing TX csum offload to enable. It seems
> reasonable to set this when the device does not copy or
> otherwise touch the data.
>
> This patch was tested using 200 instances of netperf TCP_RR with
> 1400 byte request and one byte reply. Platform is 16 core AMD.
>
> No-cache copy disabled:
> 672703 tps, 97.13% client utilization
> 50/90/99% latency 244.31 484.205 1028.41
>
> No-cache copy enabled:
> 702113 tps, 96.16% client utilization,
> 50/90/99% latency 238.56 467.56 956.955
>
> This seems to provide a nice little performance improvement and is
> consistent in the tests I ran. Presumably, this would provide
> the greatest benfits in the presence of an application workload
> stressing the cache and a lot of transmit data happening. I don't
> yet see a downside to using this.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
Since this is not an ethernet only optimization or device
specific, I would prefer it be a property of device not
something in ethtool.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net: Allow no-cache copy from user on transmit
2011-03-09 5:05 [PATCH 1/2] net: Allow no-cache copy from user on transmit Tom Herbert
2011-03-09 6:12 ` Stephen Hemminger
@ 2011-03-09 13:04 ` Michał Mirosław
2011-03-09 19:18 ` Tom Herbert
1 sibling, 1 reply; 6+ messages in thread
From: Michał Mirosław @ 2011-03-09 13:04 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev
2011/3/9 Tom Herbert <therbert@google.com>:
> This patch uses __copy_from_user_nocache (from skb_copy_to_page)
> on transmit to bypass data cache for a performance improvement.
> This functionality is configurable per device using ethtool, the
> device must also be doing TX csum offload to enable. It seems
> reasonable to set this when the device does not copy or
> otherwise touch the data.
>
> This patch was tested using 200 instances of netperf TCP_RR with
> 1400 byte request and one byte reply. Platform is 16 core AMD.
>
> No-cache copy disabled:
> 672703 tps, 97.13% client utilization
> 50/90/99% latency 244.31 484.205 1028.41
>
> No-cache copy enabled:
> 702113 tps, 96.16% client utilization,
> 50/90/99% latency 238.56 467.56 956.955
>
> This seems to provide a nice little performance improvement and is
> consistent in the tests I ran. Presumably, this would provide
> the greatest benfits in the presence of an application workload
> stressing the cache and a lot of transmit data happening. I don't
> yet see a downside to using this.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
> include/linux/ethtool.h | 4 ++++
> include/linux/netdevice.h | 4 +++-
> include/net/sock.h | 5 +++++
> net/core/ethtool.c | 11 ++++++++++-
> 4 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index aac3e2e..2cf3cc9 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -833,6 +833,10 @@ struct ethtool_ops {
> #define ETHTOOL_GFEATURES 0x0000003a /* Get device offload settings */
> #define ETHTOOL_SFEATURES 0x0000003b /* Change device offload settings */
>
> +#define ETHTOOL_GNCCOPY 0x0000003c /* Get no-cache-copy enable
> + * (ethtool_value). */
> +#define ETHTOOL_SNCCOPY 0x0000003d /* Set no-cache-copy enable
> + * (ethtool_value). */
Please don't add new get/set ops for netdev features - G/SFEATURES
handle them all.
Proper way is to modify NETIF_F_ETHTOOL_BITS, netdev_fix_features()
and, if needed for now,
legacy get/set ops (they will go away, eventually).
> /* compatibility with older code */
> #define SPARC_ETH_GSET ETHTOOL_GSET
> #define SPARC_ETH_SSET ETHTOOL_SSET
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 71563e7..1b72f29 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -978,6 +978,7 @@ struct net_device {
> #define NETIF_F_NTUPLE (1 << 27) /* N-tuple filters supported */
> #define NETIF_F_RXHASH (1 << 28) /* Receive hashing offload */
> #define NETIF_F_RXCSUM (1 << 29) /* Receive checksumming offload */
> +#define NETIF_F_NOCACHE_COPY (1 << 30) /* Use no-cache copyfromuser */
>
> /* Segmentation offload features */
> #define NETIF_F_GSO_SHIFT 16
> @@ -1020,7 +1021,8 @@ struct net_device {
> NETIF_F_FRAGLIST)
>
> /* changeable features with no special hardware requirements */
> -#define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO)
> +#define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO | \
> + NETIF_F_NOCACHE_COPY)
>
> /* Interface index. Unique device identifier */
> int ifindex;
> diff --git a/include/net/sock.h b/include/net/sock.h
> index da0534d..55b1385 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1401,6 +1401,11 @@ static inline int skb_copy_to_page(struct sock *sk, char __user *from,
> if (err)
> return err;
> skb->csum = csum_block_add(skb->csum, csum, skb->len);
> + } else if (sk->sk_route_caps & NETIF_F_NOCACHE_COPY) {
> + if (!access_ok(VERIFY_READ, from, copy)
> + || __copy_from_user_nocache(page_address(page) + off,
> + from, copy))
> + return -EFAULT;
> } else if (copy_from_user(page_address(page) + off, from, copy))
> return -EFAULT;
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index c1a71bb..a8b2638 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -328,6 +328,7 @@ static const char netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
> /* NETIF_F_NETNS_LOCAL */ "netns-local",
> /* NETIF_F_GRO */ "rx-gro",
> /* NETIF_F_LRO */ "rx-lro",
> + /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy",
>
> /* NETIF_F_TSO */ "tx-tcp-segmentation",
> /* NETIF_F_UFO */ "tx-udp-fragmentation",
You can't put the strings in the middle of this array. Positions in
the array are dependent on flag bit position.
[...]
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] net: Allow no-cache copy from user on transmit
2011-03-09 13:04 ` Michał Mirosław
@ 2011-03-09 19:18 ` Tom Herbert
2011-03-09 19:38 ` Ben Hutchings
2011-03-09 19:51 ` Michał Mirosław
0 siblings, 2 replies; 6+ messages in thread
From: Tom Herbert @ 2011-03-09 19:18 UTC (permalink / raw)
To: Michał Mirosław; +Cc: davem, netdev
> Please don't add new get/set ops for netdev features - G/SFEATURES
> handle them all.
>
> Proper way is to modify NETIF_F_ETHTOOL_BITS, netdev_fix_features()
> and, if needed for now,
> legacy get/set ops (they will go away, eventually).
>
Don't see this in
git://git.kernel.org/pub/scm/network/ethtool/ethtool. Has the patch
been applied?
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net: Allow no-cache copy from user on transmit
2011-03-09 19:18 ` Tom Herbert
@ 2011-03-09 19:38 ` Ben Hutchings
2011-03-09 19:51 ` Michał Mirosław
1 sibling, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2011-03-09 19:38 UTC (permalink / raw)
To: Tom Herbert; +Cc: Michał Mirosław, davem, netdev
On Wed, 2011-03-09 at 11:18 -0800, Tom Herbert wrote:
> > Please don't add new get/set ops for netdev features - G/SFEATURES
> > handle them all.
> >
> > Proper way is to modify NETIF_F_ETHTOOL_BITS, netdev_fix_features()
> > and, if needed for now,
> > legacy get/set ops (they will go away, eventually).
> >
>
> Don't see this in
> git://git.kernel.org/pub/scm/network/ethtool/ethtool. Has the patch
> been applied?
I haven't changed ethtool yet. I put some patches out for discussion
and Michał found some problems for them; I haven't had the time to
revise them yet.
Ben.
--
Ben Hutchings, Senior Software 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 [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net: Allow no-cache copy from user on transmit
2011-03-09 19:18 ` Tom Herbert
2011-03-09 19:38 ` Ben Hutchings
@ 2011-03-09 19:51 ` Michał Mirosław
1 sibling, 0 replies; 6+ messages in thread
From: Michał Mirosław @ 2011-03-09 19:51 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, Ben Hutchings
W dniu 9 marca 2011 20:18 użytkownik Tom Herbert <therbert@google.com> napisał:
>> Please don't add new get/set ops for netdev features - G/SFEATURES
>> handle them all.
>>
>> Proper way is to modify NETIF_F_ETHTOOL_BITS, netdev_fix_features()
>> and, if needed for now,
>> legacy get/set ops (they will go away, eventually).
> Don't see this in
> git://git.kernel.org/pub/scm/network/ethtool/ethtool. Has the patch
> been applied?
This is recent work that just went into net-next. For userspace, if
you want to test it, there is my proof-of-concept patch (posted to
netdev some time ago) or Ben's ethtool cleanup patches (also in netdev
archive).
http://marc.info/?l=linux-netdev&m=129674290420375&w=3
http://marc.info/?l=linux-netdev&m=129830729917334&w=3
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-09 19:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-09 5:05 [PATCH 1/2] net: Allow no-cache copy from user on transmit Tom Herbert
2011-03-09 6:12 ` Stephen Hemminger
2011-03-09 13:04 ` Michał Mirosław
2011-03-09 19:18 ` Tom Herbert
2011-03-09 19:38 ` Ben Hutchings
2011-03-09 19:51 ` Michał Mirosław
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox