* Re: [PATCH] net: skb_set_dev do not unconditionally drop ref to dst
From: Frank Blaschka @ 2012-05-02 5:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev, arnd, linux-s390
In-Reply-To: <1335765093.2296.4.camel@edumazet-glaptop>
On Mon, Apr 30, 2012 at 07:51:33AM +0200, Eric Dumazet wrote:
> On Mon, 2012-04-30 at 07:38 +0200, Frank Blaschka wrote:
> > From: Frank Blaschka <frank.blaschka@de.ibm.com>
> >
> > commit 8a83a00b0735190384a348156837918271034144 unconditionally
> > drops dst reference when skb->dev is set. This causes a regression
> > with VLAN and the qeth_l3 network driver. qeth_l3 can not get gw
> > information from the skb coming from the vlan driver. It is only
> > valid to drop the dst in case of different name spaces.
> >
> > Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
> > ---
> > net/core/dev.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -1881,8 +1881,8 @@ EXPORT_SYMBOL(netif_device_attach);
> > #ifdef CONFIG_NET_NS
> > void skb_set_dev(struct sk_buff *skb, struct net_device *dev)
> > {
> > - skb_dst_drop(skb);
> > if (skb->dev && !net_eq(dev_net(skb->dev), dev_net(dev))) {
> > + skb_dst_drop(skb);
> > secpath_reset(skb);
> > nf_reset(skb);
> > skb_init_secmark(skb);
> >
>
> You forgot CC Arnd Bergmann <arnd@arndb.de> ?
>
> But we do want to do the skb_dst_drop() in dev_forward_skb()
>
> Your patch breaks dev_forward_skb() then.
If NET_NS is not defined is this broken too?
>
> But apparently this forward path was alredy broken in Arnd patch...
>
Ok, until nobody comes up with an other idea I will post a patch to
change back the vlan driver (use skb->dev = dev instead of skb_set_dev)
next week.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [V2 PATCH 0/9] vhost/macvtap zeropcopy fixes
From: Michael S. Tsirkin @ 2012-05-02 5:50 UTC (permalink / raw)
To: Jason Wang; +Cc: eric.dumazet, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
On Wed, May 02, 2012 at 11:41:21AM +0800, Jason Wang wrote:
> This is an updated since the last series of vhost/macvtap zerocopy fixes which
> fixes the the possible transmission stall, host kernel stack overflow and other
> misc fixes.
>
> Changes from V1:
> - Addressing comments from Eric and Michael.
> - Adding more fixes into the seires.
Thanks for fixing this.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Dave, can you merge this for 3.4 please?
Thanks!
> ---
>
> Jason Wang (9):
> macvtap: zerocopy: fix offset calculation when building skb
> macvtap: zerocopy: fix truesize underestimation
> macvtap: zerocopy: put page when fail to get all requested user pages
> macvtap: zerocopy: set SKBTX_DEV_ZEROCOPY only when skb is built successfully
> macvtap: zerocopy: validate vectors before building skb
> vhost_net: zerocopy: fix possible NULL pointer dereference of vq->bufs
> vhost_net: re-poll only on EAGAIN or ENOBUFS
> vhost_net: zerocopy: adding and signalling immediately when fully copied
> vhost: zerocopy: poll vq in zerocopy callback
>
>
> drivers/net/macvtap.c | 57 ++++++++++++++++++++++++++++++++++---------------
> drivers/vhost/net.c | 7 ++++--
> drivers/vhost/vhost.c | 1 +
> 3 files changed, 46 insertions(+), 19 deletions(-)
>
> --
> Jason Wang
^ permalink raw reply
* Re: [PATCH] RPS: Sparse connection optimizations
From: Deng-Cheng Zhu @ 2012-05-02 4:16 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, therbert, netdev
In-Reply-To: <1335931968.22133.47.camel@edumazet-glaptop>
On 05/02/2012 12:12 PM, Eric Dumazet wrote:
> On Wed, 2012-05-02 at 12:08 +0800, Deng-Cheng Zhu wrote:
>
>> Unfortunately, on the Malta platform, NIC irqs are not suitable for SMP
>> IRQ affinity -- they are based on XT-PIC. However, I did do some RPS
>> tests where different CPU masks were assigned to the 2 NICs. And the
>> throughput *WAS* better than that of assigning the same mask to NICs.
>> But the problem addressed in this patch *STILL* exists -- hash indexing
>> causes imbalance across CPUs in the case of sparse connections.
>
> You mean your two NIC irqs are handled by CPU0 and this cant be
> changed ?
>
> That really is bad.
>
>
>
Yes, that's right. It's not IO-APIC enabled. When trying to write values
to /proc/irq/$irqnum/smp_affinity:
echo: write error: Input/output error
Deng-Cheng
^ permalink raw reply
* Re: [PATCH] RPS: Sparse connection optimizations
From: Eric Dumazet @ 2012-05-02 4:12 UTC (permalink / raw)
To: Deng-Cheng Zhu; +Cc: davem, therbert, netdev
In-Reply-To: <4FA0B337.1070401@mips.com>
On Wed, 2012-05-02 at 12:08 +0800, Deng-Cheng Zhu wrote:
> Unfortunately, on the Malta platform, NIC irqs are not suitable for SMP
> IRQ affinity -- they are based on XT-PIC. However, I did do some RPS
> tests where different CPU masks were assigned to the 2 NICs. And the
> throughput *WAS* better than that of assigning the same mask to NICs.
> But the problem addressed in this patch *STILL* exists -- hash indexing
> causes imbalance across CPUs in the case of sparse connections.
You mean your two NIC irqs are handled by CPU0 and this cant be
changed ?
That really is bad.
^ permalink raw reply
* Re: [PATCH] RPS: Sparse connection optimizations
From: Deng-Cheng Zhu @ 2012-05-02 4:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, therbert, netdev
In-Reply-To: <1335614156.2900.66.camel@edumazet-glaptop>
On 04/28/2012 07:55 PM, Eric Dumazet wrote:
> On Sat, 2012-04-28 at 18:10 +0800, Deng-Cheng Zhu wrote:
>> From: Deng-Cheng Zhu<dczhu@mips.com>
>>
>> Currently, choosing target CPU to process the incoming packet is based on
>> skb->rxhash. In the case of sparse connections, this could lead to
>> relatively low and inconsistent bandwidth while doing network throughput
>> tests -- CPU selection in the RPS map is imbalanced. Even with the same
>> hash value, 2 packets could come from different devices. Besides, on
>> architectures like MIPS with multi-threaded cores, siblings of CPU0 should
>> not be selected when others are not saturated.
>
> What CPU0 is doing so special you have to mention it in this changelog ?
Serve the NIC hardware irq and do the 1st part of softirq in RPS.
Changes relating to this will be made in v2. Thanks.
>>
>> This patch introduces a feature that allows some flows to select their CPUs
>> by looping the RPS CPU maps. Some tests were performed on the MIPS Malta
>> 1004K platform (2 cores, each with 2 VPEs) at 25Mhz with 2 Intel Pro/1000
>> NICs. The Malta board works as a router between 2 PCs. Using iperf, here
>> are results:
>
>
> RPS on a router ? Thats not very good, unless you perform a crazy amount
> of work in iptables rules maybe ?
>
> One packet comes, its better to handle it right now and send it right
> now on the same cpu. No IPI cost, no cache line misses...
Theoretically, you are right. But RPS works early -- when the hot CPU
gets really hot, RPS takes effect even with forwarding workload. Earlier
experiments on the mentioned Malta platform proved it as well
(typically 45% ~ more than doubled, amazingly).
>
> RPS is something more suitable to TCP handling in local host because
> stack has big memory footprint and latencies, not for forwarding
> workload.
See above.
> I suspect you can reach more throughput using appropriate tunings
> (correct interrupt affinities). This sounds like a bad config from the
> very beginning.
Unfortunately, on the Malta platform, NIC irqs are not suitable for SMP
IRQ affinity -- they are based on XT-PIC. However, I did do some RPS
tests where different CPU masks were assigned to the 2 NICs. And the
throughput *WAS* better than that of assigning the same mask to NICs.
But the problem addressed in this patch *STILL* exists -- hash indexing
causes imbalance across CPUs in the case of sparse connections.
>
>> +};
>> +
>> +static struct cpu_flow flow[CONFIG_NR_RPS_MAP_LOOPS][NR_CPUS];
>
> Thats absolutely not allowed to add a [NR_CPUS] array anywhere in linux
> kernel in 2012.
Good advice. Will be different in V2.
>
>> +/*
>> + * We've got CONFIG_SMP to do RPS, so only arch define is needed here to access
>> + * sibling specific information.
>> + */
>> +#if defined(CONFIG_MIPS)
>
> Thats not allowed to add a CONFIG_somearch in net/core/dev.c
Good advice too.
Thanks!
Deng-Cheng
^ permalink raw reply
* Re: pull request: wireless 2012-05-01
From: David Miller @ 2012-05-02 4:05 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20120501183157.GA2987-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Tue, 1 May 2012 14:31:58 -0400
> Part of these are Bluetooth -- Gustavo says:
>
> "A few more fixes to 3.4, there is three new device id supported, a missing
> break and a fix to retain key during the connection."
>
> Along with those, we have some 802.11 fixes. Felix gives us a fix
> for sending EAP frames for AP VLAN interfaces. Franky gives us
> two brcmfmac fixes, one for missed event completions and another
> for an improperly initialized function pointer. Grazvydas Ignotas
> gives us a couple of wl1251 fixes for some improperly freed memory.
> Jonathan Bither gives us an ath5k fix for an AHB resource leak.
> Seth Forshee gives us a b43 fix to avoid a crash when b43 fails to
> properly initialize. Finally, Wey-yi gives us a fixup for a previously
> merged fix to use the correct firmware for certain iwlwifi devices.
>
> The brcmfmac fixes are bigger than I would like to see. But since
> they are isolated to that driver and fix real problems, I felt they
> were still acceptable.
>
> Please let me know if there are problems!
Pulled, thanks John.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/4 v2 net-next] net: make GRO aware of skb->head_frag
From: Eric Dumazet @ 2012-05-02 3:54 UTC (permalink / raw)
To: Alexander Duyck
Cc: Alexander Duyck, David Miller, netdev, Neal Cardwell, Tom Herbert,
Jeff Kirsher, Michael Chan, Matt Carlson, Herbert Xu,
Ben Hutchings, Ilpo Järvinen, Maciej Żenczykowski
In-Reply-To: <1335926862.22133.42.camel@edumazet-glaptop>
On Wed, 2012-05-02 at 04:47 +0200, Eric Dumazet wrote:
> Thanks Alex, I'll take a look.
>
> It seems my tcpdump is different than yours.
>
>
I'll test following patch in a couple of hours :
Thanks !
net/ipv4/tcp_input.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 96a631d..910a794 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4467,7 +4467,8 @@ static bool tcp_try_coalesce(struct sock *sk,
struct sk_buff *from,
bool *fragstolen)
{
- int delta, len = from->len;
+ int i, delta, len = from->len;
+ bool fastpath;
*fragstolen = false;
if (tcp_hdr(from)->fin)
@@ -4484,6 +4485,7 @@ merge:
if (skb_has_frag_list(to) || skb_has_frag_list(from))
return false;
+ fastpath = !from->cloned || atomic_read(&skb_shinfo(from)->dataref) == 1;
if (skb_headlen(from) == 0 &&
(skb_shinfo(to)->nr_frags +
skb_shinfo(from)->nr_frags <= MAX_SKB_FRAGS)) {
@@ -4497,7 +4499,13 @@ copyfrags:
skb_shinfo(from)->frags,
skb_shinfo(from)->nr_frags * sizeof(skb_frag_t));
skb_shinfo(to)->nr_frags += skb_shinfo(from)->nr_frags;
- skb_shinfo(from)->nr_frags = 0;
+
+ if (fastpath)
+ skb_shinfo(from)->nr_frags = 0;
+ else
+ for (i = 0; i < skb_shinfo(from)->nr_frags; i++)
+ skb_frag_ref(from, i);
+
to->truesize += delta;
atomic_add(delta, &sk->sk_rmem_alloc);
sk_mem_charge(sk, delta);
@@ -4515,7 +4523,9 @@ copyfrags:
offset = from->data - (unsigned char *)page_address(page);
skb_fill_page_desc(to, skb_shinfo(to)->nr_frags,
page, offset, skb_headlen(from));
- *fragstolen = true;
+ *fragstolen = fastpath;
+ if (!fastpath)
+ get_page(page);
delta = len; /* we dont know real truesize... */
goto copyfrags;
}
^ permalink raw reply related
* [V2 PATCH 7/9] vhost_net: re-poll only on EAGAIN or ENOBUFS
From: Jason Wang @ 2012-05-02 3:42 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
Currently, we restart tx polling unconditionally when sendmsg()
fails. This would cause unnecessary wakeups of vhost wokers and waste
cpu utlization when evil userspace(guest driver) is able to hit EFAULT or
EINVAL.
The polling is only needed when the socket send buffer were exceeded or not
enough memory. So fix this by restarting polling only when sendmsg() returns
EAGAIN/ENOBUFS.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index ffdc0d8..62828aa 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -257,7 +257,8 @@ static void handle_tx(struct vhost_net *net)
UIO_MAXIOV;
}
vhost_discard_vq_desc(vq, 1);
- tx_poll_start(net, sock);
+ if (err == -EAGAIN || err == -ENOBUFS)
+ tx_poll_start(net, sock);
break;
}
if (err != len)
^ permalink raw reply related
* [V2 PATCH 5/9] macvtap: zerocopy: validate vectors before building skb
From: Jason Wang @ 2012-05-02 3:42 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
There're several reasons that the vectors need to be validated:
- Return error when caller provides vectors whose num is greater than UIO_MAXIOV.
- Linearize part of skb when userspace provides vectors grater than MAX_SKB_FRAGS.
- Return error when userspace provides vectors whose total length may exceed
- MAX_SKB_FRAGS * PAGE_SIZE.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvtap.c | 25 +++++++++++++++++++++----
1 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a4ff694..163559c 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -529,9 +529,10 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
}
base = (unsigned long)from->iov_base + offset;
size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ if (i + size > MAX_SKB_FRAGS)
+ return -EMSGSIZE;
num_pages = get_user_pages_fast(base, size, 0, &page[i]);
- if ((num_pages != size) ||
- (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags)) {
+ if (num_pages != size) {
for (i = 0; i < num_pages; i++)
put_page(page[i]);
return -EFAULT;
@@ -651,7 +652,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
int err;
struct virtio_net_hdr vnet_hdr = { 0 };
int vnet_hdr_len = 0;
- int copylen;
+ int copylen = 0;
bool zerocopy = false;
if (q->flags & IFF_VNET_HDR) {
@@ -680,15 +681,31 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
if (unlikely(len < ETH_HLEN))
goto err;
+ err = -EMSGSIZE;
+ if (unlikely(count > UIO_MAXIOV))
+ goto err;
+
if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY))
zerocopy = true;
if (zerocopy) {
+ /* Userspace may produce vectors with count greater than
+ * MAX_SKB_FRAGS, so we need to linearize parts of the skb
+ * to let the rest of data to be fit in the frags.
+ */
+ if (count > MAX_SKB_FRAGS) {
+ copylen = iov_length(iv, count - MAX_SKB_FRAGS);
+ if (copylen < vnet_hdr_len)
+ copylen = 0;
+ else
+ copylen -= vnet_hdr_len;
+ }
/* There are 256 bytes to be copied in skb, so there is enough
* room for skb expand head in case it is used.
* The rest buffer is mapped from userspace.
*/
- copylen = vnet_hdr.hdr_len;
+ if (copylen < vnet_hdr.hdr_len)
+ copylen = vnet_hdr.hdr_len;
if (!copylen)
copylen = GOODCOPY_LEN;
} else
^ permalink raw reply related
* [V2 PATCH 0/9] vhost/macvtap zeropcopy fixes
From: Jason Wang @ 2012-05-02 3:41 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
This is an updated since the last series of vhost/macvtap zerocopy fixes which
fixes the the possible transmission stall, host kernel stack overflow and other
misc fixes.
Changes from V1:
- Addressing comments from Eric and Michael.
- Adding more fixes into the seires.
---
Jason Wang (9):
macvtap: zerocopy: fix offset calculation when building skb
macvtap: zerocopy: fix truesize underestimation
macvtap: zerocopy: put page when fail to get all requested user pages
macvtap: zerocopy: set SKBTX_DEV_ZEROCOPY only when skb is built successfully
macvtap: zerocopy: validate vectors before building skb
vhost_net: zerocopy: fix possible NULL pointer dereference of vq->bufs
vhost_net: re-poll only on EAGAIN or ENOBUFS
vhost_net: zerocopy: adding and signalling immediately when fully copied
vhost: zerocopy: poll vq in zerocopy callback
drivers/net/macvtap.c | 57 ++++++++++++++++++++++++++++++++++---------------
drivers/vhost/net.c | 7 ++++--
drivers/vhost/vhost.c | 1 +
3 files changed, 46 insertions(+), 19 deletions(-)
--
Jason Wang
^ permalink raw reply
* [V2 PATCH 9/9] vhost: zerocopy: poll vq in zerocopy callback
From: Jason Wang @ 2012-05-02 3:42 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
We add used and signal guest in worker thread but did not poll the virtqueue
during the zero copy callback. This may lead the missing of adding and
signalling during zerocopy. Solve this by polling the virtqueue and let it
wakeup the worker during callback.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
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);
^ permalink raw reply related
* [V2 PATCH 8/9] vhost_net: zerocopy: adding and signalling immediately when fully copied
From: Jason Wang @ 2012-05-02 3:42 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
When a packet were fully copied in zerocopy, we don't wait for the DMA done to
mark the done flag, so after the packet were passed to lower device, we need to
add used and signal guest immediately.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 62828aa..1dc2aeb 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -266,6 +266,8 @@ static void handle_tx(struct vhost_net *net)
" len %d != %zd\n", err, len);
if (!zcopy)
vhost_add_used_and_signal(&net->dev, vq, head, 0);
+ else
+ vhost_zerocopy_signal_used(vq);
total_len += len;
if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
vhost_poll_queue(&vq->poll);
^ permalink raw reply related
* [V2 PATCH 6/9] vhost_net: zerocopy: fix possible NULL pointer dereference of vq->bufs
From: Jason Wang @ 2012-05-02 3:42 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
When we want to disable vhost_net backend while there's a tx work, a possible
NULL pointer defernece may happen we we try to deference the vq->bufs after
vhost_net_set_backend() assign a NULL to it.
As suggested by Michael, fix this by checking the vq->bufs instead of
vhost_sock_zcopy().
---
drivers/vhost/net.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f0da2c3..ffdc0d8 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -166,7 +166,7 @@ static void handle_tx(struct vhost_net *net)
if (wmem < sock->sk->sk_sndbuf / 2)
tx_poll_stop(net);
hdr_size = vq->vhost_hlen;
- zcopy = vhost_sock_zcopy(sock);
+ zcopy = vq->ubufs;
for (;;) {
/* Release DMAs done buffers first */
^ permalink raw reply related
* [V2 PATCH 4/9] macvtap: zerocopy: set SKBTX_DEV_ZEROCOPY only when skb is built successfully
From: Jason Wang @ 2012-05-02 3:42 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
Current the SKBTX_DEV_ZEROCOPY is set unconditionally after
zerocopy_sg_from_iovec(), this would lead NULL pointer when macvtap
fails to build zerocopy skb because destructor_arg was not
initialized. Solve this by set this flag after the skb were built
successfully.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvtap.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 9ab182a..a4ff694 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -699,10 +699,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
if (!skb)
goto err;
- if (zerocopy) {
+ if (zerocopy)
err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
- skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
- } else
+ else
err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
len);
if (err)
@@ -721,8 +720,10 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
rcu_read_lock_bh();
vlan = rcu_dereference_bh(q->vlan);
/* copy skb_ubuf_info for callback when skb has no error */
- if (zerocopy)
+ if (zerocopy) {
skb_shinfo(skb)->destructor_arg = m->msg_control;
+ skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+ }
if (vlan)
macvlan_start_xmit(skb, vlan->dev);
else
^ permalink raw reply related
* [V2 PATCH 3/9] macvtap: zerocopy: put page when fail to get all requested user pages
From: Jason Wang @ 2012-05-02 3:41 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
When get_user_pages_fast() fails to get all requested pages, we could not use
kfree_skb() to free it as it has not been put in the skb fragments. So we need
to call put_page() instead.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvtap.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 7cb2684..9ab182a 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -531,9 +531,11 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
num_pages = get_user_pages_fast(base, size, 0, &page[i]);
if ((num_pages != size) ||
- (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags))
- /* put_page is in skb free */
+ (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags)) {
+ for (i = 0; i < num_pages; i++)
+ put_page(page[i]);
return -EFAULT;
+ }
truesize = size * PAGE_SIZE;
skb->data_len += len;
skb->len += len;
^ permalink raw reply related
* [V2 PATCH 2/9] macvtap: zerocopy: fix truesize underestimation
From: Jason Wang @ 2012-05-02 3:41 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
As the skb fragment were pinned/built from user pages, we should
account the page instead of length for truesize.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvtap.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index bd4a70d..7cb2684 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -519,6 +519,7 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
struct page *page[MAX_SKB_FRAGS];
int num_pages;
unsigned long base;
+ unsigned long truesize;
len = from->iov_len - offset;
if (!len) {
@@ -533,10 +534,11 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
(num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags))
/* put_page is in skb free */
return -EFAULT;
+ truesize = size * PAGE_SIZE;
skb->data_len += len;
skb->len += len;
- skb->truesize += len;
- atomic_add(len, &skb->sk->sk_wmem_alloc);
+ skb->truesize += truesize;
+ atomic_add(truesize, &skb->sk->sk_wmem_alloc);
while (len) {
int off = base & ~PAGE_MASK;
int size = min_t(int, len, PAGE_SIZE - off);
^ permalink raw reply related
* [V2 PATCH 1/9] macvtap: zerocopy: fix offset calculation when building skb
From: Jason Wang @ 2012-05-02 3:41 UTC (permalink / raw)
To: eric.dumazet, mst, netdev, linux-kernel, ebiederm, davem
In-Reply-To: <20120502033901.11782.13157.stgit@amd-6168-8-1.englab.nay.redhat.com>
This patch fixes the offset calculation when building skb:
- offset1 were used as skb data offset not vector offset
- reset offset to zero only when we advance to next vector
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvtap.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 0427c65..bd4a70d 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -505,10 +505,11 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
if (copy > size) {
++from;
--count;
- }
+ offset = 0;
+ } else
+ offset += size;
copy -= size;
offset1 += size;
- offset = 0;
}
if (len == offset1)
@@ -519,13 +520,13 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
int num_pages;
unsigned long base;
- len = from->iov_len - offset1;
+ len = from->iov_len - offset;
if (!len) {
- offset1 = 0;
+ offset = 0;
++from;
continue;
}
- base = (unsigned long)from->iov_base + offset1;
+ base = (unsigned long)from->iov_base + offset;
size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
num_pages = get_user_pages_fast(base, size, 0, &page[i]);
if ((num_pages != size) ||
@@ -546,7 +547,7 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
len -= size;
i++;
}
- offset1 = 0;
+ offset = 0;
++from;
}
return 0;
^ permalink raw reply related
* Re: [PATCH 3/4 v2 net-next] net: make GRO aware of skb->head_frag
From: Eric Dumazet @ 2012-05-02 2:47 UTC (permalink / raw)
To: Alexander Duyck
Cc: Alexander Duyck, David Miller, netdev, Neal Cardwell, Tom Herbert,
Jeff Kirsher, Michael Chan, Matt Carlson, Herbert Xu,
Ben Hutchings, Ilpo Järvinen, Maciej Żenczykowski
In-Reply-To: <4FA06D7A.6090800@intel.com>
On Tue, 2012-05-01 at 16:10 -0700, Alexander Duyck wrote:
> On 05/01/2012 03:58 PM, Alexander Duyck wrote:
> > Eric,
> >
> > I think I have found a bug, although it is not specific to this patch
> > but it is related. It looks like the TCP coalesce code is causing
> > tcpdump to fail when using frags. Based on the comments in the patch I
> > am assuming you have an ixgbe adapter to test with as that is what I
> > reproduced this on. To reproduce the issue all you need to do is run
> > "tcpdump -i ethX > /dev/null" on one console, and on a second console
> > run a netperf TCP_MAERTS test to some other server. Tcpdump will exit
> > out with a message about bad address like this:
> >
> > tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
> > listening on eth1, link-type EN10MB (Ethernet), capture size 96 bytes
> > tcpdump: pcap_loop: recvfrom: Bad address
> > 682 packets captured
> > 2357 packets received by filter
> > 1543 packets dropped by kernel
> >
> >
> > A bisect of the issue tracked it down to:
> >
> > 1402d366019fedaa2b024f2bac06b7cc9a8782e1 is first bad commit
> > commit 1402d366019fedaa2b024f2bac06b7cc9a8782e1
> > Author: Eric Dumazet <edumazet@google.com>
> > Date: Mon Apr 23 07:11:42 2012 +0000
> >
> > tcp: introduce tcp_try_coalesce
> >
> > commit c8628155ece3 (tcp: reduce out_of_order memory use) took care of
> > coalescing tcp segments provided by legacy devices (linear skbs)
> >
> > We extend this idea to fragged skbs, as their truesize can be heavy.
> >
> > ixgbe for example uses 256+1024+PAGE_SIZE/2 = 3328 bytes per segment.
> >
> > Use this coalescing strategy for receive queue too.
> >
> > This contributes to reduce number of tcp collapses, at minimal cost, and
> > reduces memory overhead and packets drops.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Neal Cardwell <ncardwell@google.com>
> > Cc: Tom Herbert <therbert@google.com>
> > Cc: Maciej Żenczykowski <maze@google.com>
> > Cc: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> > Acked-by: Neal Cardwell <ncardwell@google.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > :040000 040000 8ca3e0b4e6c6a8f375fd800069d24203880623f3 2576d34c5c9cfc717a11e2ebe054143956716b93 M net
> >
> > I suspect we are dealing with either a shared or cloned skb in this
> > case, though I haven't verified which it is yet.
> >
> > Thanks,
> >
> > Alex
> >
> One additional note. It looks like LRO and GRO need to be disabled to
> trigger the bug. If either of them are enabled it doesn't seem to
> occur. Likely due to the fact that they are doing the coalescing before
> it gets up to the tcp_try_coalesce call.
>
Thanks Alex, I'll take a look.
It seems my tcpdump is different than yours.
^ permalink raw reply
* Re: [PATCH 3/4 v2 net-next] net: make GRO aware of skb->head_frag
From: Eric Dumazet @ 2012-05-02 2:45 UTC (permalink / raw)
To: Alexander Duyck
Cc: Alexander Duyck, David Miller, netdev, Neal Cardwell, Tom Herbert,
Jeff Kirsher, Michael Chan, Matt Carlson, Herbert Xu,
Ben Hutchings, Ilpo Järvinen, Maciej Żenczykowski
In-Reply-To: <4FA03D69.6060907@intel.com>
On Tue, 2012-05-01 at 12:45 -0700, Alexander Duyck wrote:
> I wasn't worried about the kfree vs put_page, I was worried about the
> coalesce case. However, it looks like you are correct and I am not
> seeing any issues so everything seems to be working fine.
>
> I have a hacked together ixgbe up and running now with the new build_skb
> logic and RSC/LRO disabled. It looks like it is giving me a 5%
> performance boost for small packet routing, but I am using more CPU for
> netperf TCP receive tests and I was wondering if you had seen anything
> similar on the tg3 driver?
>
Hmm, tg3 perf for small packets is not changed, because tg3 uses
copybreak anyway.
#define TG3_RX_COPY_THRESHOLD 256
For big packets, I have no changes in netperf because GRO cannot really
provide big enough skbs at 1Gb speed.
The changes are noticeable when BDP (Bandwidth*Delay Product) is large
and some packet losses happen : You notice lower latencies in TCP stack,
because of less collapses (big copies)
^ permalink raw reply
* Re: [PATCH] mwl8k: Add 0x2a02 PCI device-id (Marvell 88W8361)
From: Lennert Buytenhek @ 2012-05-02 2:38 UTC (permalink / raw)
To: Pat Erley
Cc: sedat.dilek-Re5JQEeQqe8AvxtiuMwx3w, Sedat Dilek, John W. Linville,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, lautriv, Jim Cromie,
Ben Hutchings, Hauke Mehrtens
In-Reply-To: <4FA089F9.1020602-Jx9fsTfDDR3YtjvyW6yDsg@public.gmane.org>
On Tue, May 01, 2012 at 09:12:25PM -0400, Pat Erley wrote:
> >--- a/drivers/net/wireless/mwl8k.c
> >+++ b/drivers/net/wireless/mwl8k.c
> >@@ -5225,6 +5225,7 @@ static void mwl8k_finalize_join_worker(struct work_struct *work)
> > }
> >
> > enum {
> >+ MWL8361P = 0,
> > MWL8363 = 0,
> > MWL8687,
> > MWL8366,
>
> I'm not an expert, but isn't this giving both 8361P and 8363 the
> same values in the enum?
Yeah, this is broken.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 15/17] ipvs: optimize the use of flags in ip_vs_bind_dest
From: Simon Horman @ 2012-05-02 1:24 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
Simon Horman
In-Reply-To: <1335921901-21284-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
cp->flags is marked volatile but ip_vs_bind_dest
can safely modify the flags, so save some CPU cycles by
using temp variable.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_conn.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 6a43c93..7f21b91 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -548,6 +548,7 @@ static inline void
ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
{
unsigned int conn_flags;
+ __u32 flags;
/* if dest is NULL, then return directly */
if (!dest)
@@ -559,17 +560,19 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
conn_flags = atomic_read(&dest->conn_flags);
if (cp->protocol != IPPROTO_UDP)
conn_flags &= ~IP_VS_CONN_F_ONE_PACKET;
+ flags = cp->flags;
/* Bind with the destination and its corresponding transmitter */
- if (cp->flags & IP_VS_CONN_F_SYNC) {
+ if (flags & IP_VS_CONN_F_SYNC) {
/* if the connection is not template and is created
* by sync, preserve the activity flag.
*/
- if (!(cp->flags & IP_VS_CONN_F_TEMPLATE))
+ if (!(flags & IP_VS_CONN_F_TEMPLATE))
conn_flags &= ~IP_VS_CONN_F_INACTIVE;
/* connections inherit forwarding method from dest */
- cp->flags &= ~(IP_VS_CONN_F_FWD_MASK | IP_VS_CONN_F_NOOUTPUT);
+ flags &= ~(IP_VS_CONN_F_FWD_MASK | IP_VS_CONN_F_NOOUTPUT);
}
- cp->flags |= conn_flags;
+ flags |= conn_flags;
+ cp->flags = flags;
cp->dest = dest;
IP_VS_DBG_BUF(7, "Bind-dest %s c:%s:%d v:%s:%d "
@@ -584,11 +587,11 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
atomic_read(&dest->refcnt));
/* Update the connection counters */
- if (!(cp->flags & IP_VS_CONN_F_TEMPLATE)) {
+ if (!(flags & IP_VS_CONN_F_TEMPLATE)) {
/* It is a normal connection, so modify the counters
* according to the flags, later the protocol can
* update them on state change */
- if (!(cp->flags & IP_VS_CONN_F_INACTIVE))
+ if (!(flags & IP_VS_CONN_F_INACTIVE))
atomic_inc(&dest->activeconns);
else
atomic_inc(&dest->inactconns);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 05/17] ipvs: LBLCR scheduler does not need GFP_ATOMIC allocation on init
From: Simon Horman @ 2012-05-02 1:24 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
Hans Schillstrom, Simon Horman
In-Reply-To: <1335921901-21284-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
Schedulers are initialized and bound to services only
on commands.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_lblcr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index 7498756..00906ea 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -511,7 +511,7 @@ static int ip_vs_lblcr_init_svc(struct ip_vs_service *svc)
/*
* Allocate the ip_vs_lblcr_table for this service
*/
- tbl = kmalloc(sizeof(*tbl), GFP_ATOMIC);
+ tbl = kmalloc(sizeof(*tbl), GFP_KERNEL);
if (tbl == NULL)
return -ENOMEM;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 06/17] ipvs: SH scheduler does not need GFP_ATOMIC allocation
From: Simon Horman @ 2012-05-02 1:24 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
Hans Schillstrom, Simon Horman
In-Reply-To: <1335921901-21284-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
Schedulers are initialized and bound to services only
on commands.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_sh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/ip_vs_sh.c b/net/netfilter/ipvs/ip_vs_sh.c
index 91e97ee..0512652 100644
--- a/net/netfilter/ipvs/ip_vs_sh.c
+++ b/net/netfilter/ipvs/ip_vs_sh.c
@@ -162,7 +162,7 @@ static int ip_vs_sh_init_svc(struct ip_vs_service *svc)
/* allocate the SH table for this service */
tbl = kmalloc(sizeof(struct ip_vs_sh_bucket)*IP_VS_SH_TAB_SIZE,
- GFP_ATOMIC);
+ GFP_KERNEL);
if (tbl == NULL)
return -ENOMEM;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 11/17] ipvs: always update some of the flags bits in backup
From: Simon Horman @ 2012-05-02 1:24 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
Simon Horman
In-Reply-To: <1335921901-21284-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
As the goal is to mirror the inactconns/activeconns
counters in the backup server, make sure the cp->flags are
updated even if cp is still not bound to dest. If cp->flags
are not updated ip_vs_bind_dest will rely only on the initial
flags when updating the counters. To avoid mistakes and
complicated checks for protocol state rely only on the
IP_VS_CONN_F_INACTIVE bit when updating the counters.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Tested-by: Aleksey Chudov <aleksey.chudov@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/linux/ip_vs.h | 5 +++
net/netfilter/ipvs/ip_vs_sync.c | 65 ++++++++++++++-------------------------
2 files changed, 28 insertions(+), 42 deletions(-)
diff --git a/include/linux/ip_vs.h b/include/linux/ip_vs.h
index be0ef3d..8a2d438 100644
--- a/include/linux/ip_vs.h
+++ b/include/linux/ip_vs.h
@@ -89,6 +89,7 @@
#define IP_VS_CONN_F_TEMPLATE 0x1000 /* template, not connection */
#define IP_VS_CONN_F_ONE_PACKET 0x2000 /* forward only one packet */
+/* Initial bits allowed in backup server */
#define IP_VS_CONN_F_BACKUP_MASK (IP_VS_CONN_F_FWD_MASK | \
IP_VS_CONN_F_NOOUTPUT | \
IP_VS_CONN_F_INACTIVE | \
@@ -97,6 +98,10 @@
IP_VS_CONN_F_TEMPLATE \
)
+/* Bits allowed to update in backup server */
+#define IP_VS_CONN_F_BACKUP_UPD_MASK (IP_VS_CONN_F_INACTIVE | \
+ IP_VS_CONN_F_SEQ_MASK)
+
/* Flags that are not sent to backup server start from bit 16 */
#define IP_VS_CONN_F_NFCT (1 << 16) /* use netfilter conntrack */
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index f4e0b6c..eeed767 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -731,9 +731,30 @@ static void ip_vs_proc_conn(struct net *net, struct ip_vs_conn_param *param,
else
cp = ip_vs_ct_in_get(param);
- if (cp && param->pe_data) /* Free pe_data */
+ if (cp) {
+ /* Free pe_data */
kfree(param->pe_data);
- if (!cp) {
+
+ dest = cp->dest;
+ if ((cp->flags ^ flags) & IP_VS_CONN_F_INACTIVE &&
+ !(flags & IP_VS_CONN_F_TEMPLATE) && dest) {
+ if (flags & IP_VS_CONN_F_INACTIVE) {
+ atomic_dec(&dest->activeconns);
+ atomic_inc(&dest->inactconns);
+ } else {
+ atomic_inc(&dest->activeconns);
+ atomic_dec(&dest->inactconns);
+ }
+ }
+ flags &= IP_VS_CONN_F_BACKUP_UPD_MASK;
+ flags |= cp->flags & ~IP_VS_CONN_F_BACKUP_UPD_MASK;
+ cp->flags = flags;
+ if (!dest) {
+ dest = ip_vs_try_bind_dest(cp);
+ if (dest)
+ atomic_dec(&dest->refcnt);
+ }
+ } else {
/*
* Find the appropriate destination for the connection.
* If it is not found the connection will remain unbound
@@ -742,18 +763,6 @@ static void ip_vs_proc_conn(struct net *net, struct ip_vs_conn_param *param,
dest = ip_vs_find_dest(net, type, daddr, dport, param->vaddr,
param->vport, protocol, fwmark, flags);
- /* Set the approprite ativity flag */
- if (protocol == IPPROTO_TCP) {
- if (state != IP_VS_TCP_S_ESTABLISHED)
- flags |= IP_VS_CONN_F_INACTIVE;
- else
- flags &= ~IP_VS_CONN_F_INACTIVE;
- } else if (protocol == IPPROTO_SCTP) {
- if (state != IP_VS_SCTP_S_ESTABLISHED)
- flags |= IP_VS_CONN_F_INACTIVE;
- else
- flags &= ~IP_VS_CONN_F_INACTIVE;
- }
cp = ip_vs_conn_new(param, daddr, dport, flags, dest, fwmark);
if (dest)
atomic_dec(&dest->refcnt);
@@ -763,34 +772,6 @@ static void ip_vs_proc_conn(struct net *net, struct ip_vs_conn_param *param,
IP_VS_DBG(2, "BACKUP, add new conn. failed\n");
return;
}
- } else if (!cp->dest) {
- dest = ip_vs_try_bind_dest(cp);
- if (dest)
- atomic_dec(&dest->refcnt);
- } else if ((cp->dest) && (cp->protocol == IPPROTO_TCP) &&
- (cp->state != state)) {
- /* update active/inactive flag for the connection */
- dest = cp->dest;
- if (!(cp->flags & IP_VS_CONN_F_INACTIVE) &&
- (state != IP_VS_TCP_S_ESTABLISHED)) {
- atomic_dec(&dest->activeconns);
- atomic_inc(&dest->inactconns);
- cp->flags |= IP_VS_CONN_F_INACTIVE;
- } else if ((cp->flags & IP_VS_CONN_F_INACTIVE) &&
- (state == IP_VS_TCP_S_ESTABLISHED)) {
- atomic_inc(&dest->activeconns);
- atomic_dec(&dest->inactconns);
- cp->flags &= ~IP_VS_CONN_F_INACTIVE;
- }
- } else if ((cp->dest) && (cp->protocol == IPPROTO_SCTP) &&
- (cp->state != state)) {
- dest = cp->dest;
- if (!(cp->flags & IP_VS_CONN_F_INACTIVE) &&
- (state != IP_VS_SCTP_S_ESTABLISHED)) {
- atomic_dec(&dest->activeconns);
- atomic_inc(&dest->inactconns);
- cp->flags &= ~IP_VS_CONN_F_INACTIVE;
- }
}
if (opt)
--
1.7.9.5
^ permalink raw reply related
* [PATCH 09/17] ipvs: remove check for IP_VS_CONN_F_SYNC from ip_vs_bind_dest
From: Simon Horman @ 2012-05-02 1:24 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
Simon Horman
In-Reply-To: <1335921901-21284-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
As the IP_VS_CONN_F_INACTIVE bit is properly set
in cp->flags for all kind of connections we do not need to
add special checks for synced connections when updating
the activeconns/inactconns counters for first time. Now
logic will look just like in ip_vs_unbind_dest.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_conn.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index f562e63..7647f3b 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -585,11 +585,10 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
/* Update the connection counters */
if (!(cp->flags & IP_VS_CONN_F_TEMPLATE)) {
- /* It is a normal connection, so increase the inactive
- connection counter because it is in TCP SYNRECV
- state (inactive) or other protocol inacive state */
- if ((cp->flags & IP_VS_CONN_F_SYNC) &&
- (!(cp->flags & IP_VS_CONN_F_INACTIVE)))
+ /* It is a normal connection, so modify the counters
+ * according to the flags, later the protocol can
+ * update them on state change */
+ if (!(cp->flags & IP_VS_CONN_F_INACTIVE))
atomic_inc(&dest->activeconns);
else
atomic_inc(&dest->inactconns);
--
1.7.9.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