* Re: big picture UDP/IP performance question re 2.6.18 -> 2.6.32
From: starlight @ 2011-10-05 15:12 UTC (permalink / raw)
To: Peter Zijlstra, Christoph Lameter
Cc: Eric Dumazet, linux-kernel, netdev, Willy Tarreau, Ingo Molnar
In-Reply-To: <1317820942.6766.26.camel@twins>
At 03:22 PM 10/5/2011 +0200, Peter Zijlstra wrote:
>On Tue, 2011-10-04 at 14:16 -0500, Christoph Lameter wrote:
>I suppose all this testing and feedback we receive
>from you really helps us keep the performance
>levels you want.. Oh wait, that's 0.
Well I have to admit that I've looked into the
future (i.e. tested kernel.org releases) only on
occasion. To the extent that performance wasn't
looking great I harbored an apparently over
optimistic hope that things would get tuned/fixed
in the process somewhere. In particular I was
thinking that RH might do something impressive
with RHEL 6 where Novell failed with SLES 11.
However, if anything the downstream is fiddling
less and less with the kernel as it becomes ever
more complex.
>Clearly none of the tests being ran on a regular
>basis, by for instance the Intel regression team,
>covers your needs. Start by fixing that.
I could create a test case. It's quite a lot more
than trivial, but not ridiculous either. That UDP
is broken on everything past 2.6.27 may goad me
into the effort. Have had a fairly excellent
experience WRT the hugepage kernel corruption bugs
I reported 18 months or so ago. Not only did the
kernel developers fix them, the fixes made it
downstream into RHEL fairly quickly. So that does
encourage me.
^ permalink raw reply
* Re: big picture UDP/IP performance question re 2.6.18 -> 2.6.32
From: Christoph Lameter @ 2011-10-05 14:26 UTC (permalink / raw)
To: Peter Zijlstra
Cc: starlight, Eric Dumazet, linux-kernel, netdev, Willy Tarreau,
Ingo Molnar
In-Reply-To: <1317820942.6766.26.camel@twins>
On Wed, 5 Oct 2011, Peter Zijlstra wrote:
> Clearly none of the tests being ran on a regular basis, by for instance
> the Intel regression team, covers your needs. Start by fixing that.
The most commonly run, the aged old tests contained in the AIM9 suite,
have consistently shown these regressions over long years and they have
been brought up repeatedly in numerous discussions. This is pervasive
thoughout the OS hotpaths. Just look at how the page fault latencies
change over time. Take a modern machine and then run successively older
kernel versions on it. You will see performance getting better and
latencies becoming smaller.
> Also, for latency, we've got ftrace and a latencytracer, provide traces
> that illustrate your fail.
We would need a backport of both to a kernel version that works with
reasonable latencies so that we can figure out what caused these
regressions for this particular case. Disabling network and kernel
features usually gives you better performance but there are a lot of
things in the hot paths these days that can not be disabled.
^ permalink raw reply
* [PATCH] route: fix ICMP redirect validation
From: Flavio Leitner @ 2011-10-05 14:20 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Flavio Leitner
The commit f39925dbde7788cfb96419c0f092b086aa325c0f
(ipv4: Cache learned redirect information in inetpeer.)
removed some ICMP packet validations which are required by
RFC 1122, section 3.2.2.2:
...
A Redirect message SHOULD be silently discarded if the new
gateway address it specifies is not on the same connected
(sub-) net through which the Redirect arrived [INTRO:2,
Appendix A], or if the source of the Redirect is not the
current first-hop gateway for the specified destination (see
Section 3.3.1).
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
net/ipv4/route.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 26c77e1..1c11f28 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1308,7 +1308,12 @@ static void rt_del(unsigned hash, struct rtable *rt)
void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
__be32 saddr, struct net_device *dev)
{
+ int s, i;
struct in_device *in_dev = __in_dev_get_rcu(dev);
+ struct rtable *rth;
+ struct rtable __rcu **rthp;
+ __be32 skeys[2] = { saddr, 0 };
+ int ikeys[2] = { dev->ifindex, 0 };
struct inet_peer *peer;
struct net *net;
@@ -1321,6 +1326,9 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
ipv4_is_zeronet(new_gw))
goto reject_redirect;
+ if (!rt_caching(net))
+ goto reject_redirect;;
+
if (!IN_DEV_SHARED_MEDIA(in_dev)) {
if (!inet_addr_onlink(in_dev, new_gw, old_gw))
goto reject_redirect;
@@ -1331,13 +1339,42 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
goto reject_redirect;
}
- peer = inet_getpeer_v4(daddr, 1);
- if (peer) {
- peer->redirect_learned.a4 = new_gw;
+ for (s = 0; s < 2; s++) {
+ for (i = 0; i < 2; i++) {
+ unsigned int hash = rt_hash(daddr, skeys[s],
+ ikeys[i], rt_genid(net));
- inet_putpeer(peer);
+ rthp=&rt_hash_table[hash].chain;
- atomic_inc(&__rt_peer_genid);
+ while ((rth = rcu_dereference(*rthp)) != NULL) {
+
+ if (rth->rt_key_dst != daddr ||
+ rth->rt_key_src != skeys[s] ||
+ rth->rt_oif != ikeys[i] ||
+ rt_is_input_route(rth) ||
+ rt_is_expired(rth) ||
+ !net_eq(dev_net(rth->dst.dev), net)) {
+ rthp = &rth->dst.rt_next;
+ continue;
+ }
+
+ if (rth->rt_dst != daddr ||
+ rth->rt_src != saddr ||
+ rth->rt_gateway != old_gw ||
+ rth->dst.dev != dev ||
+ rth->dst.error)
+ break;
+
+ peer = inet_getpeer_v4(daddr, 1);
+ if (peer) {
+ peer->redirect_learned.a4 = new_gw;
+ inet_putpeer(peer);
+ atomic_inc(&__rt_peer_genid);
+ }
+
+ break;
+ }
+ }
}
return;
--
1.7.6
^ permalink raw reply related
* Re: [PATCH] net-proc: expose the tos values in /proc/net/[tcp|udp]
From: MuraliRaja Muniraju @ 2011-10-05 14:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <1317807270.2473.35.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Hi Eric,
I am trying one more approach of adding a new attribute to the
inet_diag extension to expose the tos values. Hence with this approach
tcp tos problem would be solved. When udp would also be supported via
netlink and will be using inet_diag, it would be just be publishing
the tos values in the new extension added. Can you let me know your
thoughts on the following patch.
--- a/include/linux/inet_diag.h
+++ b/include/linux/inet_diag.h
@@ -97,9 +97,10 @@ enum {
INET_DIAG_INFO,
INET_DIAG_VEGASINFO,
INET_DIAG_CONG,
+ INET_DIAG_TOS,
};
-#define INET_DIAG_MAX INET_DIAG_CONG
+#define INET_DIAG_MAX INET_DIAG_TOS
/* INET_DIAG_MEM */
@@ -120,6 +121,13 @@ struct tcpvegas_info {
__u32 tcpv_minrtt;
};
+/* INET_DIAG_TOS */
+
+struct inet_diag_tos {
+ __u8 idiag_tos;
+ __u8 idiag_reserved[3];
+};
+
#ifdef __KERNEL__
struct sock;
struct inet_hashinfo;
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index e5fa2dd..abdd606 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -82,6 +82,7 @@ static int inet_csk_diag_fill(struct sock *sk,
struct nlmsghdr *nlh;
void *info = NULL;
struct inet_diag_meminfo *minfo = NULL;
+ struct inet_diag_tos *tos = NULL;
unsigned char *b = skb_tail_pointer(skb);
const struct inet_diag_handler *handler;
@@ -108,6 +109,9 @@ static int inet_csk_diag_fill(struct sock *sk,
icsk->icsk_ca_ops->name);
}
+ if (ext & (1 << (INET_DIAG_TOS - 1)))
+ tos = INET_DIAG_PUT(skb, INET_DIAG_TOS, sizeof(*tos));
+
r->idiag_family = sk->sk_family;
r->idiag_state = sk->sk_state;
r->idiag_timer = 0;
@@ -169,6 +173,9 @@ static int inet_csk_diag_fill(struct sock *sk,
icsk->icsk_ca_ops && icsk->icsk_ca_ops->get_info)
icsk->icsk_ca_ops->get_info(sk, ext, skb);
+ if (tos)
+ tos->idiag_tos = inet->tos;
+
nlh->nlmsg_len = skb_tail_pointer(skb) - b;
return skb->len;
Thanks,
Murali
On Wed, Oct 5, 2011 at 2:34 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 04 octobre 2011 à 10:38 -0700, MuraliRaja Muniraju a écrit :
>> I shall make the changes by exposing the tos values via the
>> netlink as suggested. I had a doubt is I have to modify the
>> inet_diag_sockid or export it via a new option. I am considering
>> that adding into inet_diag_sockid is better because the latter is a
>> bit of a over kill for a single value to be exposed and it also seems
>> to be logical fit.
>> Can you let me know your thoughts on this.
>
> I believe you could add one "u32 tos" to struct tcp_info
>
> [ It wont solve the UDP case, since "ss -u" still dumps /proc/net/udp
> and /proc/net/udp6 ]
>
> Following patch should handle tcp/dccp ipv4/ipv6
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 7f59ee9..eec6f3b 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -167,6 +167,7 @@ struct tcp_info {
> __u32 tcpi_rcv_space;
>
> __u32 tcpi_total_retrans;
> + __u32 tos;
> };
>
> /* for TCP_MD5SIG socket option */
> diff --git a/net/dccp/diag.c b/net/dccp/diag.c
> index b21f261..70d9498 100644
> --- a/net/dccp/diag.c
> +++ b/net/dccp/diag.c
> @@ -37,6 +37,7 @@ static void dccp_get_info(struct sock *sk, struct tcp_info *info)
>
> if (dp->dccps_hc_tx_ccid != NULL)
> ccid_hc_tx_get_info(dp->dccps_hc_tx_ccid, sk, info);
> + info->tos = inet_sk(sk)->tos;
> }
>
> static void dccp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 4c0da24..b24c3d8 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2493,6 +2493,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
> info->tcpi_rcv_space = tp->rcvq_space.space;
>
> info->tcpi_total_retrans = tp->total_retrans;
> + info->tos = inet_sk(sk)->tos;
> }
> EXPORT_SYMBOL_GPL(tcp_get_info);
>
>
>
>
--
Thanks,
Murali
^ permalink raw reply related
* Re: xfrm warning by mip6d
From: Eric Dumazet @ 2011-10-05 13:55 UTC (permalink / raw)
To: András Takács; +Cc: netdev
In-Reply-To: <34287764-C788-4718-BDD8-8B4DC98FE9F4@wakoond.hu>
Le mercredi 05 octobre 2011 à 15:32 +0200, András Takács a écrit :
> Thanks for the help. I added this two skb_dst_force calls to my kernel source, but unfortunately it didn't solved the problem.
>
> I know it's just a warning message, but I would be happy, if I could fix it.
>
> Does anybody have any other idea?
Is a more recent kernel issuing the same error ?
^ permalink raw reply
* RE: netfilter: Use proper rwlock init function
From: Hans Schillström @ 2011-10-05 13:54 UTC (permalink / raw)
To: Thomas Gleixner, netdev@vger.kernel.org, Julian Anastasov,
horms@verge.net.au
Cc: David Miller
In-Reply-To: <alpine.LFD.2.02.1110051522410.18778@ionos>
Hello Simon,
you can drop my "fix lockdep warning" patch
since Thomas patch is identical to what I prepared to send...
>Replace the open coded initialization with the init function.
>
>Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Ack-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
>---
> net/netfilter/ipvs/ip_vs_ctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>Index: linux-2.6/net/netfilter/ipvs/ip_vs_ctl.c
>===================================================================
>--- linux-2.6.orig/net/netfilter/ipvs/ip_vs_ctl.c
>+++ linux-2.6/net/netfilter/ipvs/ip_vs_ctl.c
>@@ -3679,7 +3679,7 @@ int __net_init __ip_vs_control_init(stru
> int idx;
> struct netns_ipvs *ipvs = net_ipvs(net);
>
>- ipvs->rs_lock = __RW_LOCK_UNLOCKED(ipvs->rs_lock);
>+ rwlock_init(&ipvs->rs_lock);
>
> /* Initialize rs_table */
> for (idx = 0; idx < IP_VS_RTAB_SIZE; idx++)
Thanks
Hans
^ permalink raw reply
* Re: [PATCH v2 2/2] virtio-net: Prevent NULL dereference
From: Sasha Levin @ 2011-10-05 13:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <20111003184037.GA22427@redhat.com>
On Mon, 2011-10-03 at 20:40 +0200, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2011 at 05:40:55PM +0300, Sasha Levin wrote:
> > This patch prevents a NULL dereference when the user has passed a length
> > longer than an actual buffer to virtio-net.
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> > drivers/net/virtio_net.c | 12 +++++++++++-
> > 1 files changed, 11 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index bde0dec..4a53d2a 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -208,12 +208,22 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > return NULL;
> > }
> >
> > - while (len) {
> > + while (len && page) {
> > set_skb_frag(skb, page, offset, &len);
> > page = (struct page *)page->private;
> > offset = 0;
> > }
> >
> > + /*
> > + * This is the case where we ran out of pages in our linked list, but
> > + * supposedly have more data to read.
>
> Again, let's clarify that this only happens with broken devices.
I think that the code within the if() makes it clear that it isn't the
regular path.
--
Sasha.
^ permalink raw reply
* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
From: David Howells @ 2011-10-05 13:32 UTC (permalink / raw)
To: Paul Moore; +Cc: dhowells, selinux, netdev
In-Reply-To: <2230709.7n5noARWFd@sifl>
Paul Moore <paul@paul-moore.com> wrote:
> We should probably do the same for the security/selinux/netif.c as it uses
> the same logic; David is this something you want to tackle?
netif.c doesn't use any rcu_dereference*() function directly, though it does
use list_for_each_entry_rcu(). However, I'm not sure that's a problem. What
is it you're referring to?
David
^ permalink raw reply
* Re: xfrm warning by mip6d
From: András Takács @ 2011-10-05 13:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1317810803.2473.37.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Thanks for the help. I added this two skb_dst_force calls to my kernel source, but unfortunately it didn't solved the problem.
I know it's just a warning message, but I would be happy, if I could fix it.
Does anybody have any other idea?
Thank you very much!
Regards,
András Takács
http://www.wakoond.hu
On 2011.10.05., at 12:33, Eric Dumazet wrote:
> Le mercredi 05 octobre 2011 à 11:22 +0200, András Takács a écrit :
>
> Issue solved by commit 3bc07321ccc236f693ce1b6a8786f0a2e38bb87e
>
> Thanks
>
>
> commit 3bc07321ccc236f693ce1b6a8786f0a2e38bb87e
> Author: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Tue Mar 15 21:08:28 2011 +0000
>
> xfrm: Force a dst refcount before entering the xfrm type handlers
>
> Crypto requests might return asynchronous. In this case we leave
> the rcu protected region, so force a refcount on the skb's
> destination entry before we enter the xfrm type input/output
> handlers.
>
> This fixes a crash when a route is deleted whilst sending IPsec
> data that is transformed by an asynchronous algorithm.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index 872065c..341cd11 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -190,6 +190,8 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
> XFRM_SKB_CB(skb)->seq.input.low = seq;
> XFRM_SKB_CB(skb)->seq.input.hi = seq_hi;
>
> + skb_dst_force(skb);
> +
> nexthdr = x->type->input(x, skb);
>
> if (nexthdr == -EINPROGRESS)
> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> index 1aba03f..8f3f0ee 100644
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -78,6 +78,8 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
>
> spin_unlock_bh(&x->lock);
>
> + skb_dst_force(skb);
> +
> err = x->type->output(x, skb);
> if (err == -EINPROGRESS)
> goto out_exit;
>
>
>
^ permalink raw reply
* netfilter: Use proper rwlock init function
From: Thomas Gleixner @ 2011-10-05 13:24 UTC (permalink / raw)
To: netdev; +Cc: David Miller
Replace the open coded initialization with the init function.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
net/netfilter/ipvs/ip_vs_ctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6/net/netfilter/ipvs/ip_vs_ctl.c
===================================================================
--- linux-2.6.orig/net/netfilter/ipvs/ip_vs_ctl.c
+++ linux-2.6/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3679,7 +3679,7 @@ int __net_init __ip_vs_control_init(stru
int idx;
struct netns_ipvs *ipvs = net_ipvs(net);
- ipvs->rs_lock = __RW_LOCK_UNLOCKED(ipvs->rs_lock);
+ rwlock_init(&ipvs->rs_lock);
/* Initialize rs_table */
for (idx = 0; idx < IP_VS_RTAB_SIZE; idx++)
^ permalink raw reply
* Re: big picture UDP/IP performance question re 2.6.18 -> 2.6.32
From: Peter Zijlstra @ 2011-10-05 13:22 UTC (permalink / raw)
To: Christoph Lameter
Cc: starlight, Eric Dumazet, linux-kernel, netdev, Willy Tarreau,
Ingo Molnar
In-Reply-To: <alpine.DEB.2.00.1110041358500.12199@router.home>
On Tue, 2011-10-04 at 14:16 -0500, Christoph Lameter wrote:
>
> We had similar experiences. Basically latency constantly gets screwed up
> by the new fancy features being added to the scheduler and network
> subsystem (most notorious is the new "fair" scheduler, 2.6.23 made a big
> step down). The kernel has a fairly constant regression in terms of
> latency release after release. Only the new and more efficient processors
> periodically provide some compensation (and some isolated patches to
> actually improve things get in but these are usually watered down one or
> two releases after those improvements have been made).
I suppose all this testing and feedback we receive from you really helps
us keep the performance levels you want.. Oh wait, that's 0.
Clearly none of the tests being ran on a regular basis, by for instance
the Intel regression team, covers your needs. Start by fixing that.
Also, for latency, we've got ftrace and a latencytracer, provide traces
that illustrate your fail.
^ permalink raw reply
* Re: [PATCH v2 1/2] virtio-net: Verify page list size before fitting into skb
From: Sasha Levin @ 2011-10-05 13:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Rusty Russell, virtualization, netdev, kvm
In-Reply-To: <20111003190416.GC22427@redhat.com>
On Mon, 2011-10-03 at 21:04 +0200, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2011 at 05:40:54PM +0300, Sasha Levin wrote:
> > This patch verifies that the length of a buffer stored in a linked list
> > of pages is small enough to fit into a skb.
> >
> > If the size is larger than a max size of a skb, it means that we shouldn't
> > go ahead building skbs anyway since we won't be able to send the buffer as
> > the user requested.
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> > drivers/net/virtio_net.c | 13 +++++++++++++
> > 1 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 0c7321c..bde0dec 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -195,6 +195,19 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > len -= copy;
> > offset += copy;
> >
> > + /*
> > + * Verify that we can indeed put this data into a skb.
> > + * This is here to handle cases when the device erroneously
> > + * tries to receive more than is possible. This is usually
> > + * the case of a broken device.
> > + */
> > + if (unlikely(len > MAX_SKB_FRAGS * PAGE_SIZE)) {
> > + if (net_ratelimit())
> > + pr_debug("%s: too much data\n", skb->dev->name);
> > + dev_kfree_skb(skb);
> > + return NULL;
> > + }
> > +
>
> BTW, receive_mergeable does
> pr_debug("%s: packet too long\n", skb->dev->name);
> skb->dev->stats.rx_length_errors++;
>
> which makes sense.
Do you think we should increase rx_length_errors here as well?
--
Sasha.
^ permalink raw reply
* Re: big picture UDP/IP performance question re 2.6.18 -> 2.6.32
From: starlight @ 2011-10-05 11:50 UTC (permalink / raw)
To: Eric Dumazet
Cc: Joe Perches, Christoph Lameter, Serge Belyshev, Con Kolivas,
linux-kernel, netdev, Willy Tarreau, Peter Zijlstra,
Stephen Hemminger
In-Reply-To: <1317804832.2473.25.camel@edumazet-HP-Compaq-6005-Pr o-SFF-PC>
At 10:53 AM 10/5/2011 +0200, Eric Dumazet wrote:
>
>Note :
>
>Your results are from a combination of a user
>application and kernel default strategies.
>
>On other combinations, results can be completely different.
>
>A wakeup strategy is somewhat tricky :
>
>- Should we affine or not.
>- Should we queue the wakeup on a remote CPU,
> to keep scheduler data hot in a single cpu cache.
>- Should we use RPS/RFS to queue the packet to
> another CPU before even handling it in our stack,
> to keep network data hot in a single cpu
> cache. (check Documentation/networking/scaling.txt)
>
>At least, with recent kernels, we have many
>available choices to tune a workload.
I would argue that results speak louder than
features. A 300% deterioration in latency,
600% deterioration in sigma latency and
a 50-100% increase in apparent system overhead
is not impressive.
Our application is designed to run optimally
as a scalable real-time network transaction
processor and provides for a variety of
different thread-pool and queuing approaches.
Performance is worse for every one of them
in newer kernels. The ones that scale the
best fared worst.
It seems to me that any scheduler-intensive
application will suffer a similar fate.
^ permalink raw reply
* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert() [ver #3]
From: Eric Dumazet @ 2011-10-05 11:44 UTC (permalink / raw)
To: David Howells; +Cc: selinux, netdev
In-Reply-To: <20111005111919.30551.77529.stgit@warthog.procyon.org.uk>
Le mercredi 05 octobre 2011 à 12:19 +0100, David Howells a écrit :
> Fix the following bug in sel_netport_insert() where rcu_dereference() should
> be rcu_dereference_protected() as sel_netport_lock is held.
>
> ===================================================
> [ INFO: suspicious rcu_dereference_check() usage. ]
> ---------------------------------------------------
> security/selinux/netport.c:127 invoked rcu_dereference_check() without protection!
>
> other info that might help us debug this:
>
>
> rcu_scheduler_active = 1, debug_locks = 0
> 1 lock held by ossec-rootcheck/3323:
> #0: (sel_netport_lock){+.....}, at: [<ffffffff8117d775>] sel_netport_sid+0xbb/0x226
>
> stack backtrace:
> Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
> Call Trace:
> [<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
> [<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
> [<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
> [<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
> [<ffffffff810a5388>] ? might_fault+0x4e/0x9e
> [<ffffffff810a53d1>] ? might_fault+0x97/0x9e
> [<ffffffff81171cf4>] security_socket_bind+0x11/0x13
> [<ffffffff812ba967>] sys_bind+0x56/0x95
> [<ffffffff81380dac>] ? sysret_check+0x27/0x62
> [<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
> [<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
> [<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> security/selinux/netport.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> index 0b62bd1..7b9eb1f 100644
> --- a/security/selinux/netport.c
> +++ b/security/selinux/netport.c
> @@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
> if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
> struct sel_netport *tail;
> tail = list_entry(
> - rcu_dereference(sel_netport_hash[idx].list.prev),
> + rcu_dereference_protected(
> + sel_netport_hash[idx].list.prev,
> + lockdep_is_held(&sel_netport_lock)),
> struct sel_netport, list);
> list_del_rcu(&tail->list);
> kfree_rcu(tail, rcu);
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert() [ver #2]
From: David Howells @ 2011-10-05 11:19 UTC (permalink / raw)
Cc: dhowells, selinux, netdev
In-Reply-To: <20111005110843.30324.19133.stgit@warthog.procyon.org.uk>
David Howells <dhowells@redhat.com> wrote:
> + spin_is_locked(&sel_netport_lock)),
Oops... I forgot to commit the change.
David
^ permalink raw reply
* [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert() [ver #3]
From: David Howells @ 2011-10-05 11:19 UTC (permalink / raw)
To: selinux; +Cc: netdev, dhowells
Fix the following bug in sel_netport_insert() where rcu_dereference() should
be rcu_dereference_protected() as sel_netport_lock is held.
===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/selinux/netport.c:127 invoked rcu_dereference_check() without protection!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 0
1 lock held by ossec-rootcheck/3323:
#0: (sel_netport_lock){+.....}, at: [<ffffffff8117d775>] sel_netport_sid+0xbb/0x226
stack backtrace:
Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
Call Trace:
[<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
[<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
[<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
[<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
[<ffffffff810a5388>] ? might_fault+0x4e/0x9e
[<ffffffff810a53d1>] ? might_fault+0x97/0x9e
[<ffffffff81171cf4>] security_socket_bind+0x11/0x13
[<ffffffff812ba967>] sys_bind+0x56/0x95
[<ffffffff81380dac>] ? sysret_check+0x27/0x62
[<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
[<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
[<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b
Signed-off-by: David Howells <dhowells@redhat.com>
---
security/selinux/netport.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/security/selinux/netport.c b/security/selinux/netport.c
index 0b62bd1..7b9eb1f 100644
--- a/security/selinux/netport.c
+++ b/security/selinux/netport.c
@@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
struct sel_netport *tail;
tail = list_entry(
- rcu_dereference(sel_netport_hash[idx].list.prev),
+ rcu_dereference_protected(
+ sel_netport_hash[idx].list.prev,
+ lockdep_is_held(&sel_netport_lock)),
struct sel_netport, list);
list_del_rcu(&tail->list);
kfree_rcu(tail, rcu);
^ permalink raw reply related
* [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert() [ver #2]
From: David Howells @ 2011-10-05 11:08 UTC (permalink / raw)
To: selinux; +Cc: netdev, dhowells
Fix the following bug in sel_netport_insert() where rcu_dereference() should
be rcu_dereference_protected() as sel_netport_lock is held.
===================================================
[ INFO: suspicious rcu_dereference_check() usage. ]
---------------------------------------------------
security/selinux/netport.c:127 invoked rcu_dereference_check() without protection!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 0
1 lock held by ossec-rootcheck/3323:
#0: (sel_netport_lock){+.....}, at: [<ffffffff8117d775>] sel_netport_sid+0xbb/0x226
stack backtrace:
Pid: 3323, comm: ossec-rootcheck Not tainted 3.1.0-rc8-fsdevel+ #1095
Call Trace:
[<ffffffff8105cfb7>] lockdep_rcu_dereference+0xa7/0xb0
[<ffffffff8117d871>] sel_netport_sid+0x1b7/0x226
[<ffffffff8117d6ba>] ? sel_netport_avc_callback+0xbc/0xbc
[<ffffffff8117556c>] selinux_socket_bind+0x115/0x230
[<ffffffff810a5388>] ? might_fault+0x4e/0x9e
[<ffffffff810a53d1>] ? might_fault+0x97/0x9e
[<ffffffff81171cf4>] security_socket_bind+0x11/0x13
[<ffffffff812ba967>] sys_bind+0x56/0x95
[<ffffffff81380dac>] ? sysret_check+0x27/0x62
[<ffffffff8105b767>] ? trace_hardirqs_on_caller+0x11e/0x155
[<ffffffff81076fcd>] ? audit_syscall_entry+0x17b/0x1ae
[<ffffffff811b5eae>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[<ffffffff81380d7b>] system_call_fastpath+0x16/0x1b
Signed-off-by: David Howells <dhowells@redhat.com>
---
security/selinux/netport.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/security/selinux/netport.c b/security/selinux/netport.c
index 0b62bd1..39e2138 100644
--- a/security/selinux/netport.c
+++ b/security/selinux/netport.c
@@ -123,7 +123,9 @@ static void sel_netport_insert(struct sel_netport *port)
if (sel_netport_hash[idx].size == SEL_NETPORT_HASH_BKT_LIMIT) {
struct sel_netport *tail;
tail = list_entry(
- rcu_dereference(sel_netport_hash[idx].list.prev),
+ rcu_dereference_protected(
+ sel_netport_hash[idx].list.prev,
+ spin_is_locked(&sel_netport_lock)),
struct sel_netport, list);
list_del_rcu(&tail->list);
kfree_rcu(tail, rcu);
^ permalink raw reply related
* Re: [PATCH] SELinux: Fix RCU deref check warning in sel_netport_insert()
From: David Howells @ 2011-10-05 11:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: dhowells, Paul Moore, selinux, netdev
In-Reply-To: <1317702176.2784.4.camel@edumazet-laptop>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Usual way is to use :
> rcu_dereference_protected(
> sel_netport_hash[idx].list.prev,
> lockdep_is_held(&sel_netport_lock)),
Good point.
David
^ permalink raw reply
* Re: [RFC] [PATCH] virtio: Dont add "config" to list for !per_vq_vector
From: Michael S. Tsirkin @ 2011-10-05 10:48 UTC (permalink / raw)
To: Krishna Kumar; +Cc: rusty, netdev, linux-kernel, virtualization
In-Reply-To: <20111005053859.5825.1089.sendpatchset@krkumar2.in.ibm.com>
On Wed, Oct 05, 2011 at 11:08:59AM +0530, Krishna Kumar wrote:
> For the MSI but non-per_vq_vector case, the config/change vq
> also gets added to the list of vqs that need to process the
> MSI interrupt. This is not needed as config has it's own
> handler (vp_config_changed). In any case, vring_interrupt()
> finds nothing needs to be done on this vq.
>
> I tested this patch by testing the "Fallback:" and "Finally
> fall back" cases in vp_find_vqs(). Please review.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
(note: this is not a bugfix so not 3.1 material).
> ---
> drivers/virtio/virtio_pci.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff -ruNp org/drivers/virtio/virtio_pci.c new/drivers/virtio/virtio_pci.c
> --- org/drivers/virtio/virtio_pci.c 2011-10-03 09:10:11.000000000 +0530
> +++ new/drivers/virtio/virtio_pci.c 2011-10-04 19:16:34.000000000 +0530
> @@ -415,9 +415,13 @@ static struct virtqueue *setup_vq(struct
> }
> }
>
> - spin_lock_irqsave(&vp_dev->lock, flags);
> - list_add(&info->node, &vp_dev->virtqueues);
> - spin_unlock_irqrestore(&vp_dev->lock, flags);
> + if (callback) {
> + spin_lock_irqsave(&vp_dev->lock, flags);
> + list_add(&info->node, &vp_dev->virtqueues);
> + spin_unlock_irqrestore(&vp_dev->lock, flags);
> + } else {
> + INIT_LIST_HEAD(&info->node);
> + }
>
> return vq;
Some further enhancement suggestions for this shared case:
- we don't really need a lock, do we? and how about replacing vq list
with an array?
- vring_interrupt calls
if (!more_used(vq))
outside any lock.
This looks scary - don't we need a read barrier somewhere?
--
MST
^ permalink raw reply
* Re: xfrm warning by mip6d
From: Eric Dumazet @ 2011-10-05 10:33 UTC (permalink / raw)
To: András Takács; +Cc: netdev
In-Reply-To: <4C76ADDE-B534-4C40-BBDA-DCADA5DD347D@wakoond.hu>
Le mercredi 05 octobre 2011 à 11:22 +0200, András Takács a écrit :
> Dear All,
>
> We're running mip6d mobile IPv6 daemon on 2.6.35.13 kernel. While it is sending periodic Binding Update messages via raw IPv6 socket the following warning message appears:
>
> [ 946.883714] ------------[ cut here ]------------
> [ 946.883714] WARNING: at trunk/kernel/linux-2.6.35.13/include/linux/skbuff.h:447 skb_dst.clone.22+0x42/0x44()
> [ 946.883714] Hardware name: Bochs
> [ 946.890048] Modules linked in: xfrm_user
> [ 946.890179] Pid: 523, comm: mip6d Tainted: G W 2.6.35.13-itssv6 #1
> [ 946.890270] Call Trace:
> [ 946.890354] [<c0130e75>] warn_slowpath_common+0x65/0x7a
> [ 946.890453] [<c03e2b11>] ? skb_dst.clone.22+0x42/0x44
> [ 946.890553] [<c0130e99>] warn_slowpath_null+0xf/0x13
> [ 946.890651] [<c03e2b11>] skb_dst.clone.22+0x42/0x44
> [ 946.890748] [<c03e2e04>] ip6_finish_output2+0x15/0x23c
> [ 946.890847] [<c0130e99>] ? warn_slowpath_null+0xf/0x13
> [ 946.890947] [<c03e438f>] ip6_finish_output+0x8b3/0x8bf
> [ 946.891048] [<c0130c9c>] ? print_oops_end_marker+0x1e/0x23
> [ 946.891151] [<c0130e82>] ? warn_slowpath_common+0x72/0x7a
> [ 946.891248] [<c03e2b11>] ? skb_dst.clone.22+0x42/0x44
> [ 946.891346] [<c0130e99>] ? warn_slowpath_null+0xf/0x13
> [ 946.891445] [<c03e4443>] ip6_output+0xa8/0xb0
> [ 946.891539] [<c03dd551>] xfrm_output_resume+0x99/0x349
> [ 946.891639] [<c03dd80e>] xfrm_output2+0xd/0xf
> [ 946.891732] [<c03dd8a6>] xfrm_output+0x96/0xa0
> [ 946.891828] [<c0404fbf>] xfrm6_output_finish+0x13/0x15
> [ 946.891928] [<c04050fc>] xfrm6_output+0x4a/0x52
> [ 946.892020] [<c03e2b28>] dst_output+0x15/0x18
> [ 946.892111] [<c03e33b0>] ip6_local_out+0x17/0x1a
> [ 946.892206] [<c03e502c>] ip6_push_pending_frames+0x27c/0x31f
> [ 946.892314] [<c03f62f7>] rawv6_sendmsg+0xa79/0xb0b
> [ 946.892411] [<c014e7ad>] ? __lock_acquire+0x54f/0xbdc
> [ 946.892525] [<c03c932d>] inet_sendmsg+0x3d/0x46
> [ 946.892623] [<c0383b2f>] __sock_sendmsg+0x45/0x4e
> [ 946.892722] [<c0383f8f>] sock_sendmsg+0x92/0xa6
> [ 946.892822] [<c014eec2>] ? lock_release_non_nested+0x88/0x1f7
> [ 946.892927] [<c0177f81>] ? might_fault+0x46/0x80
> [ 946.893023] [<c0177f81>] ? might_fault+0x46/0x80
> [ 946.893121] [<c024a690>] ? _copy_from_user+0x39/0x4d
> [ 946.893221] [<c0385656>] sys_sendmsg+0x17d/0x1e1
> [ 946.893331] [<c014eec2>] ? lock_release_non_nested+0x88/0x1f7
> [ 946.893436] [<c0177f81>] ? might_fault+0x46/0x80
> [ 946.893531] [<c0177f81>] ? might_fault+0x46/0x80
> [ 946.893628] [<c0385a28>] sys_socketcall+0x14b/0x189
> [ 946.893725] [<c024a130>] ? trace_hardirqs_on_thunk+0xc/0x10
> [ 946.893827] [<c0119b17>] sysenter_do_call+0x12/0x36
> [ 946.893915] ---[ end trace a2273a766b3b014c ]---
>
> I tried to find the issue, and printed out the three condition values of WARN_ON macro:
>
> [ 61.686487] debug skb->_skb_refdst: 00000000CED5EA01 SKB_DST_NOREF 0000000000000001 ==> 0000000000000001
> [ 61.686649] rcu_read_lock_held() 0
> [ 61.686730] rcu_read_lock_bh_held() 0
>
> I think, that the issue is somewhere in the xfrm module, which includes the Home Address Destination Option header, before ip6_output.
>
>
> Did anyone have the same issue before? Where do you thing the missing rcu lock is?
>
Issue solved by commit 3bc07321ccc236f693ce1b6a8786f0a2e38bb87e
Thanks
commit 3bc07321ccc236f693ce1b6a8786f0a2e38bb87e
Author: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue Mar 15 21:08:28 2011 +0000
xfrm: Force a dst refcount before entering the xfrm type handlers
Crypto requests might return asynchronous. In this case we leave
the rcu protected region, so force a refcount on the skb's
destination entry before we enter the xfrm type input/output
handlers.
This fixes a crash when a route is deleted whilst sending IPsec
data that is transformed by an asynchronous algorithm.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 872065c..341cd11 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -190,6 +190,8 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
XFRM_SKB_CB(skb)->seq.input.low = seq;
XFRM_SKB_CB(skb)->seq.input.hi = seq_hi;
+ skb_dst_force(skb);
+
nexthdr = x->type->input(x, skb);
if (nexthdr == -EINPROGRESS)
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 1aba03f..8f3f0ee 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -78,6 +78,8 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
spin_unlock_bh(&x->lock);
+ skb_dst_force(skb);
+
err = x->type->output(x, skb);
if (err == -EINPROGRESS)
goto out_exit;
^ permalink raw reply related
* [PATCH 8/8] chelsio: convert to SKB paged frag API.
From: Ian Campbell @ 2011-10-05 10:28 UTC (permalink / raw)
To: netdev; +Cc: Ian Campbell, Divy Le Ray, Dimitris Michailidis, Casey Leedom
In-Reply-To: <1317810511.21903.204.camel@zakaz.uk.xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Divy Le Ray <divy@chelsio.com>
Cc: Dimitris Michailidis <dm@chelsio.com>
Cc: Casey Leedom <leedom@chelsio.com>
Cc: netdev@vger.kernel.org
---
drivers/net/ethernet/chelsio/cxgb/sge.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.c b/drivers/net/ethernet/chelsio/cxgb/sge.c
index e9a03ff..7cde425 100644
--- a/drivers/net/ethernet/chelsio/cxgb/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb/sge.c
@@ -1277,9 +1277,8 @@ static inline void write_tx_descs(struct adapter *adapter, struct sk_buff *skb,
ce = q->centries;
}
- mapping = pci_map_page(adapter->pdev, frag->page,
- frag->page_offset, frag->size,
- PCI_DMA_TODEVICE);
+ mapping = skb_frag_dma_map(&adapter->pdev->dev, frag, 0,
+ frag->size, PCI_DMA_TODEVICE);
desc_mapping = mapping;
desc_len = frag->size;
--
1.7.2.5
^ permalink raw reply related
* [PATCH 6/8] myri10ge: convert to SKB paged frag API.
From: Ian Campbell @ 2011-10-05 10:28 UTC (permalink / raw)
To: netdev; +Cc: Ian Campbell, Jon Mason, Andrew Gallatin
In-Reply-To: <1317810511.21903.204.camel@zakaz.uk.xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jon Mason <mason@myri.com>
Cc: Andrew Gallatin <gallatin@myri.com>
Cc: netdev@vger.kernel.org
---
drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 81c1700..8bf6034 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -1342,7 +1342,7 @@ myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
/* Fill skb_frag_struct(s) with data from our receive */
for (i = 0, remainder = len; remainder > 0; i++) {
myri10ge_unmap_rx_page(pdev, &rx->info[idx], bytes);
- rx_frags[i].page = rx->info[idx].page;
+ __skb_frag_set_page(&rx_frags[i], rx->info[idx].page);
rx_frags[i].page_offset = rx->info[idx].page_offset;
if (remainder < MYRI10GE_ALLOC_SIZE)
rx_frags[i].size = remainder;
@@ -1375,7 +1375,7 @@ myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
ss->stats.rx_dropped++;
do {
i--;
- put_page(rx_frags[i].page);
+ __skb_frag_unref(&rx_frags[i]);
} while (i != 0);
return 0;
}
@@ -1383,7 +1383,7 @@ myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
/* Attach the pages to the skb, and trim off any padding */
myri10ge_rx_skb_build(skb, va, rx_frags, len, hlen);
if (skb_shinfo(skb)->frags[0].size <= 0) {
- put_page(skb_shinfo(skb)->frags[0].page);
+ skb_frag_unref(skb, 0);
skb_shinfo(skb)->nr_frags = 0;
}
skb->protocol = eth_type_trans(skb, dev);
@@ -2284,7 +2284,7 @@ myri10ge_get_frag_header(struct skb_frag_struct *frag, void **mac_hdr,
struct ethhdr *eh;
struct vlan_ethhdr *veh;
struct iphdr *iph;
- u8 *va = page_address(frag->page) + frag->page_offset;
+ u8 *va = skb_frag_address(frag);
unsigned long ll_hlen;
/* passed opaque through lro_receive_frags() */
__wsum csum = (__force __wsum) (unsigned long)priv;
@@ -2927,8 +2927,8 @@ again:
frag = &skb_shinfo(skb)->frags[frag_idx];
frag_idx++;
len = frag->size;
- bus = pci_map_page(mgp->pdev, frag->page, frag->page_offset,
- len, PCI_DMA_TODEVICE);
+ bus = skb_frag_dma_map(&mgp->pdev->dev, frag, 0, len,
+ PCI_DMA_TODEVICE);
dma_unmap_addr_set(&tx->info[idx], bus, bus);
dma_unmap_len_set(&tx->info[idx], len, len);
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH 4/8] et131x: convert to SKB paged frag API.
From: Ian Campbell @ 2011-10-05 10:28 UTC (permalink / raw)
To: netdev; +Cc: Ian Campbell, Greg Kroah-Hartman, Mark Einon, devel
In-Reply-To: <1317810511.21903.204.camel@zakaz.uk.xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Mark Einon <mark.einon@gmail.com>
Cc: devel@driverdev.osuosl.org
Cc: netdev@vger.kernel.org
---
drivers/staging/et131x/et1310_tx.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/et131x/et1310_tx.c b/drivers/staging/et131x/et1310_tx.c
index 8fb3051..03e7a4e 100644
--- a/drivers/staging/et131x/et1310_tx.c
+++ b/drivers/staging/et131x/et1310_tx.c
@@ -519,12 +519,12 @@ static int nic_send_packet(struct et131x_adapter *etdev, struct tcb *tcb)
* returned by pci_map_page() is always 32-bit
* addressable (as defined by the pci/dma subsystem)
*/
- desc[frag++].addr_lo =
- pci_map_page(etdev->pdev,
- frags[i - 1].page,
- frags[i - 1].page_offset,
- frags[i - 1].size,
- PCI_DMA_TODEVICE);
+ desc[frag++].addr_lo = skb_frag_dma_map(
+ &etdev->pdev->dev,
+ &frags[i - 1],
+ 0,
+ frags[i - 1].size,
+ PCI_DMA_TODEVICE);
}
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH 7/8] cxgb3: convert to SKB paged frag API.
From: Ian Campbell @ 2011-10-05 10:28 UTC (permalink / raw)
To: netdev; +Cc: Ian Campbell, Divy Le Ray
In-Reply-To: <1317810511.21903.204.camel@zakaz.uk.xensource.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Divy Le Ray <divy@chelsio.com>
Cc: netdev@vger.kernel.org
---
drivers/net/ethernet/chelsio/cxgb3/sge.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb3/sge.c b/drivers/net/ethernet/chelsio/cxgb3/sge.c
index d6fa177..a0baaa0 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/sge.c
@@ -979,8 +979,8 @@ static inline unsigned int make_sgl(const struct sk_buff *skb,
for (i = 0; i < nfrags; i++) {
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
- mapping = pci_map_page(pdev, frag->page, frag->page_offset,
- frag->size, PCI_DMA_TODEVICE);
+ mapping = skb_frag_dma_map(&pdev->dev, frag, 0, frag->size,
+ PCI_DMA_TODEVICE);
sgp->len[j] = cpu_to_be32(frag->size);
sgp->addr[j] = cpu_to_be64(mapping);
j ^= 1;
@@ -2116,7 +2116,7 @@ static void lro_add_page(struct adapter *adap, struct sge_qset *qs,
len -= offset;
rx_frag += nr_frags;
- rx_frag->page = sd->pg_chunk.page;
+ __skb_frag_set_page(rx_frag, sd->pg_chunk.page);
rx_frag->page_offset = sd->pg_chunk.offset + offset;
rx_frag->size = len;
--
1.7.2.5
^ permalink raw reply related
* [PATCH 2/8] xen: netback: convert to SKB paged frag API.
From: Ian Campbell @ 2011-10-05 10:28 UTC (permalink / raw)
To: netdev; +Cc: Ian Campbell, xen-devel
In-Reply-To: <1317810511.21903.204.camel@zakaz.uk.xensource.com>
netback currently uses frag->page to store a temporary index reference while
processing incoming requests. Since frag->page is to become opaque switch
instead to using page_offset. Add a wrapper to tidy this up and propagate the
fact that the indexes are only u16 through the code (this was already true in
practice but unsigned long and in were inconsistently used as variable and
parameter types)
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: xen-devel@lists.xensource.com
Cc: netdev@vger.kernel.org
---
drivers/net/xen-netback/netback.c | 54 ++++++++++++++++++++++--------------
1 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index fd00f25..8d70b44 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -60,6 +60,9 @@ struct netbk_rx_meta {
#define MAX_PENDING_REQS 256
+/* Discriminate from any valid pending_idx value. */
+#define INVALID_PENDING_IDX 0xFFFF
+
#define MAX_BUFFER_OFFSET PAGE_SIZE
/* extra field used in struct page */
@@ -155,13 +158,13 @@ static struct xen_netif_rx_response *make_rx_response(struct xenvif *vif,
u16 flags);
static inline unsigned long idx_to_pfn(struct xen_netbk *netbk,
- unsigned int idx)
+ u16 idx)
{
return page_to_pfn(netbk->mmap_pages[idx]);
}
static inline unsigned long idx_to_kaddr(struct xen_netbk *netbk,
- unsigned int idx)
+ u16 idx)
{
return (unsigned long)pfn_to_kaddr(idx_to_pfn(netbk, idx));
}
@@ -215,6 +218,16 @@ static int get_page_ext(struct page *pg,
sizeof(struct iphdr) + MAX_IPOPTLEN + \
sizeof(struct tcphdr) + MAX_TCP_OPTION_SPACE)
+static u16 frag_get_pending_idx(skb_frag_t *frag)
+{
+ return (u16)frag->page_offset;
+}
+
+static void frag_set_pending_idx(skb_frag_t *frag, u16 pending_idx)
+{
+ frag->page_offset = pending_idx;
+}
+
static inline pending_ring_idx_t pending_index(unsigned i)
{
return i & (MAX_PENDING_REQS-1);
@@ -512,7 +525,7 @@ static int netbk_gop_skb(struct sk_buff *skb,
for (i = 0; i < nr_frags; i++) {
netbk_gop_frag_copy(vif, skb, npo,
- skb_shinfo(skb)->frags[i].page,
+ skb_frag_page(&skb_shinfo(skb)->frags[i]),
skb_shinfo(skb)->frags[i].size,
skb_shinfo(skb)->frags[i].page_offset,
&head);
@@ -890,7 +903,7 @@ static int netbk_count_requests(struct xenvif *vif,
static struct page *xen_netbk_alloc_page(struct xen_netbk *netbk,
struct sk_buff *skb,
- unsigned long pending_idx)
+ u16 pending_idx)
{
struct page *page;
page = alloc_page(GFP_KERNEL|__GFP_COLD);
@@ -909,11 +922,11 @@ static struct gnttab_copy *xen_netbk_get_requests(struct xen_netbk *netbk,
{
struct skb_shared_info *shinfo = skb_shinfo(skb);
skb_frag_t *frags = shinfo->frags;
- unsigned long pending_idx = *((u16 *)skb->data);
+ u16 pending_idx = *((u16 *)skb->data);
int i, start;
/* Skip first skb fragment if it is on same page as header fragment. */
- start = ((unsigned long)shinfo->frags[0].page == pending_idx);
+ start = (frag_get_pending_idx(&shinfo->frags[0]) == pending_idx);
for (i = start; i < shinfo->nr_frags; i++, txp++) {
struct page *page;
@@ -945,7 +958,7 @@ static struct gnttab_copy *xen_netbk_get_requests(struct xen_netbk *netbk,
memcpy(&pending_tx_info[pending_idx].req, txp, sizeof(*txp));
xenvif_get(vif);
pending_tx_info[pending_idx].vif = vif;
- frags[i].page = (void *)pending_idx;
+ frag_set_pending_idx(&frags[i], pending_idx);
}
return gop;
@@ -956,7 +969,7 @@ static int xen_netbk_tx_check_gop(struct xen_netbk *netbk,
struct gnttab_copy **gopp)
{
struct gnttab_copy *gop = *gopp;
- int pending_idx = *((u16 *)skb->data);
+ u16 pending_idx = *((u16 *)skb->data);
struct pending_tx_info *pending_tx_info = netbk->pending_tx_info;
struct xenvif *vif = pending_tx_info[pending_idx].vif;
struct xen_netif_tx_request *txp;
@@ -976,13 +989,13 @@ static int xen_netbk_tx_check_gop(struct xen_netbk *netbk,
}
/* Skip first skb fragment if it is on same page as header fragment. */
- start = ((unsigned long)shinfo->frags[0].page == pending_idx);
+ start = (frag_get_pending_idx(&shinfo->frags[0]) == pending_idx);
for (i = start; i < nr_frags; i++) {
int j, newerr;
pending_ring_idx_t index;
- pending_idx = (unsigned long)shinfo->frags[i].page;
+ pending_idx = frag_get_pending_idx(&shinfo->frags[i]);
/* Check error status: if okay then remember grant handle. */
newerr = (++gop)->status;
@@ -1008,7 +1021,7 @@ static int xen_netbk_tx_check_gop(struct xen_netbk *netbk,
pending_idx = *((u16 *)skb->data);
xen_netbk_idx_release(netbk, pending_idx);
for (j = start; j < i; j++) {
- pending_idx = (unsigned long)shinfo->frags[i].page;
+ pending_idx = frag_get_pending_idx(&shinfo->frags[i]);
xen_netbk_idx_release(netbk, pending_idx);
}
@@ -1029,15 +1042,14 @@ static void xen_netbk_fill_frags(struct xen_netbk *netbk, struct sk_buff *skb)
for (i = 0; i < nr_frags; i++) {
skb_frag_t *frag = shinfo->frags + i;
struct xen_netif_tx_request *txp;
- unsigned long pending_idx;
+ struct page *page;
+ u16 pending_idx;
- pending_idx = (unsigned long)frag->page;
+ pending_idx = frag_get_pending_idx(frag);
txp = &netbk->pending_tx_info[pending_idx].req;
- frag->page = virt_to_page(idx_to_kaddr(netbk, pending_idx));
- frag->size = txp->size;
- frag->page_offset = txp->offset;
-
+ page = virt_to_page(idx_to_kaddr(netbk, pending_idx));
+ __skb_fill_page_desc(skb, i, page, txp->offset, txp->size);
skb->len += txp->size;
skb->data_len += txp->size;
skb->truesize += txp->size;
@@ -1349,11 +1361,11 @@ static unsigned xen_netbk_tx_build_gops(struct xen_netbk *netbk)
skb_shinfo(skb)->nr_frags = ret;
if (data_len < txreq.size) {
skb_shinfo(skb)->nr_frags++;
- skb_shinfo(skb)->frags[0].page =
- (void *)(unsigned long)pending_idx;
+ frag_set_pending_idx(&skb_shinfo(skb)->frags[0],
+ pending_idx);
} else {
- /* Discriminate from any valid pending_idx value. */
- skb_shinfo(skb)->frags[0].page = (void *)~0UL;
+ frag_set_pending_idx(&skb_shinfo(skb)->frags[0],
+ INVALID_PENDING_IDX);
}
__skb_queue_tail(&netbk->tx_queue, skb);
--
1.7.2.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox