* Re: TCP: orphans broken by RFC 2525 #2.17
From: Willy Tarreau @ 2010-09-26 17:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1285520567.2530.8.camel@edumazet-laptop>
Hi Eric,
On Sun, Sep 26, 2010 at 07:02:47PM +0200, Eric Dumazet wrote:
> How could we delay the close() ? We must either send a FIN or RST.
I don't mean to delay the close(), but I'm aware that my description
was not very clear.
Here's what I would find normal :
1) upon close(), we send a FIN, whether there are incoming pending
data or not (after all, the only difference is only a timing
issue, as the data in the rx buffer might very well come just
after the FIN, as it almost always does, BTW). The connection
then becomes FIN_WAIT1 just as now.
2) mark the socket as orphaned
3) when an ACK comes from the other side, either it's below our last
seq, and we simply ignore it, just as if we were in TIME_WAIT, or
it is equal to the last seq and indicates that it's now safe to
reset ; we would then just send the RST to notify the other side
that the data it sent were not read. The connection can then either
be destroyed or put in TIME_WAIT. It's the point where the connection
normally switches from FIN_WAIT1 to FIN_WAIT2, since the FIN has been
acked. The only difference is that we don't need a FIN_WAIT2 state
for an orphan.
> I would say, fix the program, so that RST is avoided ?
Not that easy, see below.
> The program does :
>
> recv() // read the request
> send() // queue the answer
> close() // could work if world was perfect...
>
> Change it to
>
> recv()
> send()
> shutdown()
> recv() // read & flush in excess data
New data arrives now, close() below will cause an RST again.
> close()
>
> This for sure will send FIN after all queued data is sent.
> I am not sure the final rcv() is even needed, its Sunday after all ;)
Currently the real code (ie: not the poc I posted) does :
recv()
send()
shutdown()
close()
The extra CRLF almost always happens between the recv() and send(). What
I intend to do as a workaround is exactly what you described above, but
I'm well aware it's not enough. It will only reduce the rate at which this
case happens. Well, in fact, in 10 years of production at many sites, it's
the first time such an issue is reported and it could be tracked down to
these two extra bytes. But the workaround will not prevent the two extra
bytes from coming after the last recv().
Also, the issue remains when processing large POST requests. Let's suppose
the application is receiving a massive POST (eg: 10 MB) but the request is
not authenticated, so the application returns an HTTP 401 response to
require the client to authenticate. There's no way for the application to
be notified that the small response was completely read by the client and
that it's safe to close().
For these reasons, I concluded that the application can't get everything
right and needs help from the kernel (said differently, I think that the
RFC2525 fix is causing harm in addition to goods). In my opinion, this
section in the RFC was added based on a few observations of trivial cases
but was but its impact was not completely explored.
I'm willing to experiment, but I'm not much familiar with the code itself
and sometimes I'm not sure about what I'm doing, probably that some help
would be welcome. What I'd like to do is to implement the step 3 above,
which is to only send the RST upon receipt of an ACK on an orphan that
would switch a normal socket from FIN_WAIT1 to FIN_WAIT2.
Also, I'm not sure about what other OSes are doing. For instance, I tried
on Solaris and did not observe the issue at all, though I think that
Solaris simply does not implement the RFC2525 recommendation.
Have a nice sunday evening ;-)
Willy
^ permalink raw reply
* Re: TCP: orphans broken by RFC 2525 #2.17
From: Eric Dumazet @ 2010-09-26 17:02 UTC (permalink / raw)
To: Willy Tarreau; +Cc: netdev
In-Reply-To: <20100926131717.GA13046@1wt.eu>
Le dimanche 26 septembre 2010 à 15:17 +0200, Willy Tarreau a écrit :
> Hi,
>
> one haproxy user was reporting occasionally truncated responses to
> HTTP POST requests exclusively. After he took many captures, we
> could verify that the strace dumps were showing all data to be
> emitted, but network captures showed that an RST was emitted before
> the end of the data.
>
> Looking more closely, I noticed that in traces showing the issue,
> the client was sending an additional CRLF after the data in a
> separate packet (permitted eventhough not recommended).
>
> I could thus finally understand what happens and I'm now able to
> reproduce it very easily using the attached program. What happens
> is that haproxy sends the last data to the client, followed by a
> shutdown()+close(). This is mimmicked by the attached program,
> which is connected to by a simple netcat from another machine
> sending two distinct chunks :
>
> server:$ ./abort-data
> client:$ (echo "req1";usleep 200000; echo "req2") | nc6 server 8000
> block1
> ("block2" is missing here)
> client:$
>
> It gives the following capture, with client=10.8.3.4 and server=10.8.3.1 :
>
> reading from file abort-linux.cap, link-type EN10MB (Ethernet)
> 10:47:07.057793 IP (tos 0x0, ttl 64, id 57159, offset 0, flags [DF], proto TCP (6), length 60)
> 10.8.3.4.39925 > 10.8.3.1.8000: Flags [S], cksum 0xdad9 (correct), seq 2570439277, win 5840, options [mss 1460,sackOK,TS val 138417450 ecr 0,nop,wscale 6], length 0
> 10:47:07.058015 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto TCP (6), length 60)
> 10.8.3.1.8000 > 10.8.3.4.39925: Flags [S.], cksum 0x3851 (correct), seq 1066199564, ack 2570439278, win 5792, options [mss 1460,sackOK,TS val 295921514 ecr 138417450,nop,wscale 7], length 0
> 10:47:07.058071 IP (tos 0x0, ttl 64, id 57160, offset 0, flags [DF], proto TCP (6), length 52)
> 10.8.3.4.39925 > 10.8.3.1.8000: Flags [.], cksum 0x7d60 (correct), seq 2570439278, ack 1066199565, win 92, options [nop,nop,TS val 138417451 ecr 295921514], length 0
> 10:47:07.058213 IP (tos 0x0, ttl 64, id 57161, offset 0, flags [DF], proto TCP (6), length 57)
> 10.8.3.4.39925 > 10.8.3.1.8000: Flags [P.], cksum 0x1a40 (incorrect -> 0x8fbc), seq 2570439278:2570439283, ack 1066199565, win 92, options [nop,nop,TS val 138417451 ecr 295921514], length 5
> 10:47:07.058410 IP (tos 0x0, ttl 64, id 36199, offset 0, flags [DF], proto TCP (6), length 52)
> 10.8.3.1.8000 > 10.8.3.4.39925: Flags [.], cksum 0x7d89 (correct), seq 1066199565, ack 2570439283, win 46, options [nop,nop,TS val 295921514 ecr 138417451], length 0
> 10:47:07.253294 IP (tos 0x0, ttl 64, id 57162, offset 0, flags [DF], proto TCP (6), length 53)
> 10.8.3.4.39925 > 10.8.3.1.8000: Flags [P.], cksum 0x1a3c (incorrect -> 0x7321), seq 2570439283:2570439284, ack 1066199565, win 92, options [nop,nop,TS val 138417500 ecr 295921514], length 1
> 10:47:07.253468 IP (tos 0x0, ttl 64, id 36200, offset 0, flags [DF], proto TCP (6), length 52)
> 10.8.3.1.8000 > 10.8.3.4.39925: Flags [.], cksum 0x7d27 (correct), seq 1066199565, ack 2570439284, win 46, options [nop,nop,TS val 295921562 ecr 138417500], length 0
> 10:47:08.060213 IP (tos 0x0, ttl 64, id 36201, offset 0, flags [DF], proto TCP (6), length 59)
> 10.8.3.1.8000 > 10.8.3.4.39925: Flags [P.], cksum 0x354c (correct), seq 1066199565:1066199572, ack 2570439284, win 46, options [nop,nop,TS val 295921765 ecr 138417500], length 7
> 10:47:08.060270 IP (tos 0x0, ttl 64, id 57163, offset 0, flags [DF], proto TCP (6), length 52)
> 10.8.3.4.39925 > 10.8.3.1.8000: Flags [.], cksum 0x7b5e (correct), seq 2570439284, ack 1066199572, win 92, options [nop,nop,TS val 138417701 ecr 295921765], length 0
> 10:47:08.060298 IP (tos 0x0, ttl 64, id 36202, offset 0, flags [DF], proto TCP (6), length 52)
> 10.8.3.1.8000 > 10.8.3.4.39925: Flags [R.], cksum 0x7c51 (correct), seq 1066199572, ack 2570439284, win 46, options [nop,nop,TS val 295921765 ecr 138417500], length 0
> 10:47:08.060613 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto TCP (6), length 40)
> 10.8.3.1.8000 > 10.8.3.4.39925: Flags [R], cksum 0xb0f5 (correct), seq 1066199572, win 0, length 0
> .
>
> The connection should in theory become an orphan. I'm saying "in theory",
> because since the following test was added to tcp_close(), if the client
> happens to send any data between the last recv() and the close(), we
> immediately send an RST to it, regardless of any pending outgoing data :
>
> /* As outlined in RFC 2525, section 2.17, we send a RST here because
> * data was lost. To witness the awful effects of the old behavior of
> * always doing a FIN, run an older 2.1.x kernel or 2.0.x, start a bulk
> * GET in an FTP client, suspend the process, wait for the client to
> * advertise a zero window, then kill -9 the FTP client, wheee...
> * Note: timeout is always zero in such a case.
> */
> if (data_was_unread) {
> /* Unread data was tossed, zap the connection. */
> NET_INC_STATS_USER(sock_net(sk), LINUX_MIB_TCPABORTONCLOSE);
> tcp_set_state(sk, TCP_CLOSE);
> tcp_send_active_reset(sk, sk->sk_allocation);
> }
>
> The immediate effect then is that the client receives an abort before it
> even gets the last data that were scheduled for being sent.
>
> I've read RFC 2525 #2.17 and it shows quite interesting examples of what
> it wanted to protect against. However, the recommendation did not consider
> the fact that there could be some unacked pending data in the outgoing
> buffers.
>
> What is even more more embarrassing is that the HTTP working group is
> trying to encourage browsers to enable pipelining by default. That means
> that the situation above can become much more common, where two requests
> will be pipeline, the first one will cause a short response followed by
> a close(), and the simple presence of the second one will kill the first
> one's data.
>
> I tried to think about a finer way to process those unwanted data. Ideally,
> we should just ignore until the ACK indicates that our last segment was
> properly received. Then we could emit the RST.
>
> I made a few attempts by first changing the test above like this :
>
> - if (data_was_unread) {
> + if (data_was_unread && !tcp_sk(sk)->packets_out) {
>
> then fiddling a little bit in tcp_input.c:tcp_rcv_state_process() for
> the TCP_FIN_WAIT1 state, but I'm not satisfied with my experimentations,
> they were a bit too much experimental for the results to be considered
> reliable.
>
> What I was looking for was a way to only send an RST when the socket is
> an orphan and all of its outgoing data has been ACKed. This would cover
> the situations that RFC 2525 #2.17 tries to fix without rendering orphans
> unusable.
>
> Has anyone an opinion on this, or even could suggest a patch to relax
> the conditions in which we send an RST ?
How could we delay the close() ? We must either send a FIN or RST.
I would say, fix the program, so that RST is avoided ?
The program does :
recv() // read the request
send() // queue the answer
close() // could work if world was perfect...
Change it to
recv()
send()
shutdown()
recv() // read & flush in excess data
close()
This for sure will send FIN after all queued data is sent.
I am not sure the final rcv() is even needed, its Sunday after all ;)
^ permalink raw reply
* Re: [PATCH v11 00/17] Provide a zero-copy method on KVM virtio-net.
From: Michael S. Tsirkin @ 2010-09-26 17:01 UTC (permalink / raw)
To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mingo, davem, herbert, jdike
In-Reply-To: <1285388855-27410-1-git-send-email-xiaohui.xin@intel.com>
On Sat, Sep 25, 2010 at 12:27:18PM +0800, xiaohui.xin@intel.com wrote:
> We provide an zero-copy method which driver side may get external
> buffers to DMA. Here external means driver don't use kernel space
> to allocate skb buffers. Currently the external buffer can be from
> guest virtio-net driver.
>
> The idea is simple, just to pin the guest VM user space and then
> let host NIC driver has the chance to directly DMA to it.
> The patches are based on vhost-net backend driver. We add a device
> which provides proto_ops as sendmsg/recvmsg to vhost-net to
> send/recv directly to/from the NIC driver. KVM guest who use the
> vhost-net backend may bind any ethX interface in the host side to
> get copyless data transfer thru guest virtio-net frontend.
> patch 01-10: net core and kernel changes.
> patch 11-13: new device as interface to mantpulate external buffers.
> patch 14: for vhost-net.
> patch 15: An example on modifying NIC driver to using napi_gro_frags().
> patch 16: An example how to get guest buffers based on driver
> who using napi_gro_frags().
> patch 17: It's a patch to address comments from Michael S. Thirkin
> to add 2 new ioctls in mp device.
> We split it out here to make easier reiewer.
I commented on how to avoid mm semaphore on data path separately, and
since you didn't have time to review that yet, I won't repeat that here.
At this point what are the plans on macvtap integration?
You indicated this is the interface you intend to use longterm.
^ permalink raw reply
* [PATCH] net/9p: Mount only matching virtio channels
From: Sven Eckelmann @ 2010-09-26 15:28 UTC (permalink / raw)
To: ericvh; +Cc: davem, netdev, linux-kernel, Sven Eckelmann
p9_virtio_create will only compare the the channel's tag characters
against the device name till the end of the channel's tag but not till
the end of the device name. This means that if a user defines channels
with the tags foo and foobar then he would mount foo when he requested
foonot and may mount foo when he requested foobar.
Thus it is necessary to check both string lengths against each other in
case of a successful partial string match.
Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>
---
net/9p/trans_virtio.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index dcfbe99..b885159 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -329,7 +329,8 @@ p9_virtio_create(struct p9_client *client, const char *devname, char *args)
mutex_lock(&virtio_9p_lock);
list_for_each_entry(chan, &virtio_chan_list, chan_list) {
- if (!strncmp(devname, chan->tag, chan->tag_len)) {
+ if (!strncmp(devname, chan->tag, chan->tag_len) &&
+ strlen(devname) == chan->tag_len) {
if (!chan->inuse) {
chan->inuse = true;
found = 1;
--
1.7.2.3
^ permalink raw reply related
* Re: [patch] ipvs: Keep track of backlog connections
From: Sven Wegener @ 2010-09-26 14:59 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, Wensong Zhang, Julian Anastasov,
Venkata Mohan Reddy Koppula
In-Reply-To: <alpine.LNX.2.00.1009261629240.609@titan.stealer.net>
On Sun, 26 Sep 2010, Sven Wegener wrote:
> On Sun, 26 Sep 2010, Simon Horman wrote:
>
> > here is an updated though as yet untested version of your patch from Julian
> > to take into account recent changes. In particualr, the ip_vs_sync.c
> > portion is no longer needed as only the flags in the first 16 bits are
> > synced now. It applies against Patrick McHardy's nf-next-2.6 tree.
> >
> > You mentioned in your original post that you would work on an ipvsadm
> > patch for this feature. Have you had time to do so?
> >
> > Also, are you in a position to test this? If not I can do so.
>
> I've tested your patch with the below addition to ipvsadm and it works
> correctly.
And here is a small patch for the manpage:
diff --git a/ipvsadm.8 b/ipvsadm.8
index 6874c0e..78d67ef 100644
--- a/ipvsadm.8
+++ b/ipvsadm.8
@@ -392,6 +392,15 @@ information of each server in service listing. The persistent
connection is used to forward the actual connections from the same
client/network to the same server.
.TP
+.B --backlog-conn
+Output of backlog connection information. The \fIlist\fP command
+with this option will display the backlog connection counter
+information of each server in service listing. A backlog connection
+is a connection that is inactive, but is expected to move to active
+soon. Speaking in TCP language, a connection from which we have seen
+the initial SYN packet, but the three-way handshake has not finished
+yet.
+.TP
.B --sort
Sort the list of virtual services and real servers. The virtual
service entries are sorted in ascending order by <protocol, address,
^ permalink raw reply related
* Re: [patch] ipvs: Keep track of backlog connections
From: Sven Wegener @ 2010-09-26 14:31 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, Wensong Zhang, Julian Anastasov,
Venkata Mohan Reddy Koppula
In-Reply-To: <20100926133101.GA9004@verge.net.au>
On Sun, 26 Sep 2010, Simon Horman wrote:
> here is an updated though as yet untested version of your patch from Julian
> to take into account recent changes. In particualr, the ip_vs_sync.c
> portion is no longer needed as only the flags in the first 16 bits are
> synced now. It applies against Patrick McHardy's nf-next-2.6 tree.
>
> You mentioned in your original post that you would work on an ipvsadm
> patch for this feature. Have you had time to do so?
>
> Also, are you in a position to test this? If not I can do so.
Hi,
I've tested your patch with the below addition to ipvsadm and it works
correctly.
Sven
From: Sven Wegener <sven.wegener@stealer.net>
Subject: [PATCH] Show backlog connections
Signed-off-by: Sven Wegener <sven.wegener@stealer.net>
---
ipvsadm.c | 51 ++++++++++++++++++++++++++++++--------------
libipvs/ip_vs.h | 4 +++
libipvs/ip_vs_nl_policy.c | 1 +
libipvs/libipvs.c | 3 ++
4 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/ipvsadm.c b/ipvsadm.c
index 8557d21..54ae110 100644
--- a/ipvsadm.c
+++ b/ipvsadm.c
@@ -181,7 +181,8 @@ static const char* cmdnames[] = {
#define OPT_SYNCID 0x080000
#define OPT_EXACT 0x100000
#define OPT_ONEPACKET 0x200000
-#define NUMBER_OF_OPT 22
+#define OPT_BACKLOGCONN 0x400000
+#define NUMBER_OF_OPT 23
static const char* optnames[] = {
"numeric",
@@ -206,6 +207,7 @@ static const char* optnames[] = {
"syncid",
"exact",
"ops",
+ "backlog-conn",
};
/*
@@ -218,21 +220,21 @@ static const char* optnames[] = {
*/
static const char commands_v_options[NUMBER_OF_CMD][NUMBER_OF_OPT] =
{
- /* -n -c svc -s -p -M -r fwd -w -x -y -mc tot dmn -st -rt thr -pc srt sid -ex ops */
-/*ADD*/ {'x', 'x', '+', ' ', ' ', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' '},
-/*EDIT*/ {'x', 'x', '+', ' ', ' ', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' '},
-/*DEL*/ {'x', 'x', '+', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
-/*FLUSH*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
-/*LIST*/ {' ', '1', '1', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', '1', '1', ' ', ' ', ' ', ' ', ' ', ' ', ' ', 'x'},
-/*ADDSRV*/ {'x', 'x', '+', 'x', 'x', 'x', '+', ' ', ' ', ' ', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
-/*DELSRV*/ {'x', 'x', '+', 'x', 'x', 'x', '+', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
-/*EDITSRV*/ {'x', 'x', '+', 'x', 'x', 'x', '+', ' ', ' ', ' ', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
-/*TIMEOUT*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
-/*STARTD*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' ', 'x', 'x'},
-/*STOPD*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' ', 'x', 'x'},
-/*RESTORE*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
-/*SAVE*/ {' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
-/*ZERO*/ {'x', 'x', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+ /* -n -c svc -s -p -M -r fwd -w -x -y -mc tot dmn -st -rt thr -pc srt sid -ex ops, blc */
+/*ADD*/ {'x', 'x', '+', ' ', ' ', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' ', 'x'},
+/*EDIT*/ {'x', 'x', '+', ' ', ' ', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' ', 'x'},
+/*DEL*/ {'x', 'x', '+', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+/*FLUSH*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+/*LIST*/ {' ', '1', '1', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', '1', '1', ' ', ' ', ' ', ' ', ' ', ' ', ' ', 'x', ' '},
+/*ADDSRV*/ {'x', 'x', '+', 'x', 'x', 'x', '+', ' ', ' ', ' ', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+/*DELSRV*/ {'x', 'x', '+', 'x', 'x', 'x', '+', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+/*EDITSRV*/ {'x', 'x', '+', 'x', 'x', 'x', '+', ' ', ' ', ' ', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+/*TIMEOUT*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+/*STARTD*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' ', 'x', 'x', 'x'},
+/*STOPD*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', ' ', 'x', 'x', 'x'},
+/*RESTORE*/ {'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+/*SAVE*/ {' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
+/*ZERO*/ {'x', 'x', ' ', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x', 'x'},
};
/* printing format flags */
@@ -245,6 +247,7 @@ static const char commands_v_options[NUMBER_OF_CMD][NUMBER_OF_OPT] =
#define FMT_PERSISTENTCONN 0x0020
#define FMT_NOSORT 0x0040
#define FMT_EXACT 0x0080
+#define FMT_BACKLOGCONN 0x0100
#define SERVICE_NONE 0x0000
#define SERVICE_ADDR 0x0001
@@ -282,6 +285,7 @@ enum {
TAG_PERSISTENTCONN,
TAG_SORT,
TAG_NO_SORT,
+ TAG_BACKLOGCONN,
};
/* various parsing helpers & parsing functions */
@@ -421,6 +425,8 @@ parse_options(int argc, char **argv, struct ipvs_command_entry *ce,
{ "exact", 'X', POPT_ARG_NONE, NULL, 'X', NULL, NULL },
{ "ipv6", '6', POPT_ARG_NONE, NULL, '6', NULL, NULL },
{ "ops", 'o', POPT_ARG_NONE, NULL, 'o', NULL, NULL },
+ { "backlog-conn", '\0', POPT_ARG_NONE, NULL,
+ TAG_BACKLOGCONN, NULL, NULL },
{ NULL, 0, 0, NULL, 0, NULL, NULL }
};
@@ -647,6 +653,10 @@ parse_options(int argc, char **argv, struct ipvs_command_entry *ce,
set_option(options, OPT_ONEPACKET);
ce->svc.flags |= IP_VS_SVC_F_ONEPACKET;
break;
+ case TAG_BACKLOGCONN:
+ set_option(options, OPT_BACKLOGCONN);
+ *format |= FMT_BACKLOGCONN;
+ break;
default:
fail(2, "invalid option `%s'",
poptBadOption(context, POPT_BADOPTION_NOALIAS));
@@ -1396,6 +1406,11 @@ static void print_title(unsigned int format)
" -> RemoteAddress:Port\n",
"Prot LocalAddress:Port",
"Weight", "PersistConn", "ActiveConn", "InActConn");
+ else if (format & FMT_BACKLOGCONN)
+ printf("%-33s %-9s %-11s %-10s %-10s\n"
+ " -> RemoteAddress:Port\n",
+ "Prot LocalAddress:Port",
+ "Weight", "BacklogConn", "ActiveConn", "InActConn");
else if (!(format & FMT_RULE))
printf("Prot LocalAddress:Port Scheduler Flags\n"
" -> RemoteAddress:Port Forward Weight ActiveConn InActConn\n");
@@ -1539,6 +1554,10 @@ print_service_entry(ipvs_service_entry_t *se, unsigned int format)
printf(" -> %-28s %-9u %-11u %-10u %-10u\n", dname,
e->weight, e->persistconns,
e->activeconns, e->inactconns);
+ } else if (format & FMT_BACKLOGCONN) {
+ printf(" -> %-28s %-9u %-11u %-10u %-10u\n", dname,
+ e->weight, e->backlogconns,
+ e->activeconns, e->inactconns);
} else
printf(" -> %-28s %-7s %-6d %-10u %-10u\n",
dname, fwd_name(e->conn_flags),
diff --git a/libipvs/ip_vs.h b/libipvs/ip_vs.h
index 843c51a..4c2c265 100644
--- a/libipvs/ip_vs.h
+++ b/libipvs/ip_vs.h
@@ -277,6 +277,8 @@ struct ip_vs_dest_entry {
struct ip_vs_stats_user stats;
u_int16_t af;
union nf_inet_addr addr;
+
+ u_int32_t backlogconns; /* backlog connections */
};
/* The argument to IP_VS_SO_GET_DESTS */
@@ -455,6 +457,8 @@ enum {
IPVS_DEST_ATTR_PERSIST_CONNS, /* persistent connections */
IPVS_DEST_ATTR_STATS, /* nested attribute for dest stats */
+
+ IPVS_DEST_ATTR_BACKLOG_CONNS, /* backlog connections */
__IPVS_DEST_ATTR_MAX,
};
diff --git a/libipvs/ip_vs_nl_policy.c b/libipvs/ip_vs_nl_policy.c
index c80083e..d06a490 100644
--- a/libipvs/ip_vs_nl_policy.c
+++ b/libipvs/ip_vs_nl_policy.c
@@ -40,6 +40,7 @@ struct nla_policy ipvs_dest_policy[IPVS_DEST_ATTR_MAX + 1] = {
[IPVS_DEST_ATTR_INACT_CONNS] = { .type = NLA_U32 },
[IPVS_DEST_ATTR_PERSIST_CONNS] = { .type = NLA_U32 },
[IPVS_DEST_ATTR_STATS] = { .type = NLA_NESTED },
+ [IPVS_DEST_ATTR_BACKLOG_CONNS] = { .type = NLA_U32 },
};
struct nla_policy ipvs_stats_policy[IPVS_STATS_ATTR_MAX + 1] = {
diff --git a/libipvs/libipvs.c b/libipvs/libipvs.c
index 979d5bd..e06f9fa 100644
--- a/libipvs/libipvs.c
+++ b/libipvs/libipvs.c
@@ -748,6 +748,8 @@ static int ipvs_dests_parse_cb(struct nl_msg *msg, void *arg)
d->entrytable[i].l_threshold = nla_get_u32(dest_attrs[IPVS_DEST_ATTR_L_THRESH]);
d->entrytable[i].activeconns = nla_get_u32(dest_attrs[IPVS_DEST_ATTR_ACTIVE_CONNS]);
d->entrytable[i].inactconns = nla_get_u32(dest_attrs[IPVS_DEST_ATTR_INACT_CONNS]);
+ if (dest_attrs[IPVS_DEST_ATTR_BACKLOG_CONNS])
+ d->entrytable[i].backlogconns = nla_get_u32(dest_attrs[IPVS_DEST_ATTR_BACKLOG_CONNS]);
d->entrytable[i].persistconns = nla_get_u32(dest_attrs[IPVS_DEST_ATTR_PERSIST_CONNS]);
d->entrytable[i].af = d->af;
@@ -853,6 +855,7 @@ ipvs_nl_dest_failure:
sizeof(struct ip_vs_dest_entry_kern));
d->entrytable[i].af = AF_INET;
d->entrytable[i].addr.ip = d->entrytable[i].__addr_v4;
+ d->entrytable[i].backlogconns = 0;
}
free(dk);
return d;
^ permalink raw reply related
* [patch] ipvs: Keep track of backlog connections
From: Simon Horman @ 2010-09-26 13:31 UTC (permalink / raw)
To: Sven Wegener
Cc: lvs-devel, netdev, Wensong Zhang, Julian Anastasov,
Venkata Mohan Reddy Koppula
From: Sven Wegener <sven.wegener@stealer.net>
A backlog connection is a connection that is on its way from inactive to
active. Speaking in TCP language, a connection from which we've seen the
initial SYN packet, but the three-way handshake hasn't finished yet.
These connections are expected to move to active soon. When a
destination is overloaded or isn't able to successfully establish
connections for various reasons, this count increases quickly and is an
indication for a problem.
Signed-off-by: Sven Wegener <sven.wegener@stealer.net>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
Hi Sven,
here is an updated though as yet untested version of your patch from Julian
to take into account recent changes. In particualr, the ip_vs_sync.c
portion is no longer needed as only the flags in the first 16 bits are
synced now. It applies against Patrick McHardy's nf-next-2.6 tree.
You mentioned in your original post that you would work on an ipvsadm
patch for this feature. Have you had time to do so?
Also, are you in a position to test this? If not I can do so.
Hi Mohan,
do you have any thoughts on an appropriate to add this feature to SCTP?
diff -urp net-next-2.6-e548833-nfct/linux/include/linux/ip_vs.h linux/include/linux/ip_vs.h
--- net-next-2.6-e548833-nfct/linux/include/linux/ip_vs.h 2010-09-15 11:28:02.000000000 +0300
+++ linux/include/linux/ip_vs.h 2010-09-26 15:21:53.507865229 +0300
@@ -91,6 +91,7 @@
/* Flags that are not sent to backup server start from bit 16 */
#define IP_VS_CONN_F_NFCT (1 << 16) /* use netfilter conntrack */
+#define IP_VS_CONN_F_BACKLOG (1 << 17) /* backlog connection */
/* Connection flags from destination that can be changed by user space */
#define IP_VS_CONN_F_DEST_MASK (IP_VS_CONN_F_FWD_MASK | \
@@ -360,6 +361,7 @@ enum {
IPVS_DEST_ATTR_PERSIST_CONNS, /* persistent connections */
IPVS_DEST_ATTR_STATS, /* nested attribute for dest stats */
+ IPVS_DEST_ATTR_BACKLOG_CONNS, /* backlog connections */
__IPVS_DEST_ATTR_MAX,
};
diff -urp net-next-2.6-e548833-nfct/linux/include/net/ip_vs.h linux/include/net/ip_vs.h
--- net-next-2.6-e548833-nfct/linux/include/net/ip_vs.h 2010-09-16 08:53:04.000000000 +0300
+++ linux/include/net/ip_vs.h 2010-09-26 15:31:27.369865994 +0300
@@ -501,6 +501,7 @@ struct ip_vs_dest {
/* connection counters and thresholds */
atomic_t activeconns; /* active connections */
atomic_t inactconns; /* inactive connections */
+ atomic_t backlogconns; /* backlog connections */
atomic_t persistconns; /* persistent connections */
__u32 u_threshold; /* upper threshold */
__u32 l_threshold; /* lower threshold */
diff -urp net-next-2.6-e548833-nfct/linux/net/netfilter/ipvs/ip_vs_conn.c linux/net/netfilter/ipvs/ip_vs_conn.c
--- net-next-2.6-e548833-nfct/linux/net/netfilter/ipvs/ip_vs_conn.c 2010-09-15 11:14:13.000000000 +0300
+++ linux/net/netfilter/ipvs/ip_vs_conn.c 2010-09-26 15:24:48.292865793 +0300
@@ -611,6 +611,8 @@ static inline void ip_vs_unbind_dest(str
} else {
atomic_dec(&dest->activeconns);
}
+ if (cp->flags & IP_VS_CONN_F_BACKLOG)
+ atomic_dec(&dest->backlogconns);
} else {
/* It is a persistent connection/template, so decrease
the peristent connection counter */
diff -urp net-next-2.6-e548833-nfct/linux/net/netfilter/ipvs/ip_vs_ctl.c linux/net/netfilter/ipvs/ip_vs_ctl.c
--- net-next-2.6-e548833-nfct/linux/net/netfilter/ipvs/ip_vs_ctl.c 2010-09-16 08:56:34.000000000 +0300
+++ linux/net/netfilter/ipvs/ip_vs_ctl.c 2010-09-26 15:26:45.407867200 +0300
@@ -2593,6 +2593,7 @@ static const struct nla_policy ip_vs_des
[IPVS_DEST_ATTR_INACT_CONNS] = { .type = NLA_U32 },
[IPVS_DEST_ATTR_PERSIST_CONNS] = { .type = NLA_U32 },
[IPVS_DEST_ATTR_STATS] = { .type = NLA_NESTED },
+ [IPVS_DEST_ATTR_BACKLOG_CONNS] = { .type = NLA_U32 },
};
static int ip_vs_genl_fill_stats(struct sk_buff *skb, int container_type,
@@ -2840,6 +2841,8 @@ static int ip_vs_genl_fill_dest(struct s
atomic_read(&dest->activeconns));
NLA_PUT_U32(skb, IPVS_DEST_ATTR_INACT_CONNS,
atomic_read(&dest->inactconns));
+ NLA_PUT_U32(skb, IPVS_DEST_ATTR_BACKLOG_CONNS,
+ atomic_read(&dest->backlogconns));
NLA_PUT_U32(skb, IPVS_DEST_ATTR_PERSIST_CONNS,
atomic_read(&dest->persistconns));
diff -urp net-next-2.6-e548833-nfct/linux/net/netfilter/ipvs/ip_vs_proto_tcp.c linux/net/netfilter/ipvs/ip_vs_proto_tcp.c
--- net-next-2.6-e548833-nfct/linux/net/netfilter/ipvs/ip_vs_proto_tcp.c 2010-09-10 08:27:33.000000000 +0300
+++ linux/net/netfilter/ipvs/ip_vs_proto_tcp.c 2010-09-26 15:29:18.425865407 +0300
@@ -510,6 +510,15 @@ set_tcp_state(struct ip_vs_protocol *pp,
atomic_dec(&dest->inactconns);
cp->flags &= ~IP_VS_CONN_F_INACTIVE;
}
+ if (new_state == IP_VS_TCP_S_SYN_RECV &&
+ !(cp->flags & IP_VS_CONN_F_BACKLOG)) {
+ atomic_inc(&dest->backlogconns);
+ cp->flags |= IP_VS_CONN_F_BACKLOG;
+ } else if (new_state == IP_VS_TCP_S_ESTABLISHED &&
+ cp->flags & IP_VS_CONN_F_BACKLOG) {
+ atomic_dec(&dest->backlogconns);
+ cp->flags &= ~IP_VS_CONN_F_BACKLOG;
+ }
}
}
^ permalink raw reply
* TCP: orphans broken by RFC 2525 #2.17
From: Willy Tarreau @ 2010-09-26 13:17 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 6731 bytes --]
Hi,
one haproxy user was reporting occasionally truncated responses to
HTTP POST requests exclusively. After he took many captures, we
could verify that the strace dumps were showing all data to be
emitted, but network captures showed that an RST was emitted before
the end of the data.
Looking more closely, I noticed that in traces showing the issue,
the client was sending an additional CRLF after the data in a
separate packet (permitted eventhough not recommended).
I could thus finally understand what happens and I'm now able to
reproduce it very easily using the attached program. What happens
is that haproxy sends the last data to the client, followed by a
shutdown()+close(). This is mimmicked by the attached program,
which is connected to by a simple netcat from another machine
sending two distinct chunks :
server:$ ./abort-data
client:$ (echo "req1";usleep 200000; echo "req2") | nc6 server 8000
block1
("block2" is missing here)
client:$
It gives the following capture, with client=10.8.3.4 and server=10.8.3.1 :
reading from file abort-linux.cap, link-type EN10MB (Ethernet)
10:47:07.057793 IP (tos 0x0, ttl 64, id 57159, offset 0, flags [DF], proto TCP (6), length 60)
10.8.3.4.39925 > 10.8.3.1.8000: Flags [S], cksum 0xdad9 (correct), seq 2570439277, win 5840, options [mss 1460,sackOK,TS val 138417450 ecr 0,nop,wscale 6], length 0
10:47:07.058015 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto TCP (6), length 60)
10.8.3.1.8000 > 10.8.3.4.39925: Flags [S.], cksum 0x3851 (correct), seq 1066199564, ack 2570439278, win 5792, options [mss 1460,sackOK,TS val 295921514 ecr 138417450,nop,wscale 7], length 0
10:47:07.058071 IP (tos 0x0, ttl 64, id 57160, offset 0, flags [DF], proto TCP (6), length 52)
10.8.3.4.39925 > 10.8.3.1.8000: Flags [.], cksum 0x7d60 (correct), seq 2570439278, ack 1066199565, win 92, options [nop,nop,TS val 138417451 ecr 295921514], length 0
10:47:07.058213 IP (tos 0x0, ttl 64, id 57161, offset 0, flags [DF], proto TCP (6), length 57)
10.8.3.4.39925 > 10.8.3.1.8000: Flags [P.], cksum 0x1a40 (incorrect -> 0x8fbc), seq 2570439278:2570439283, ack 1066199565, win 92, options [nop,nop,TS val 138417451 ecr 295921514], length 5
10:47:07.058410 IP (tos 0x0, ttl 64, id 36199, offset 0, flags [DF], proto TCP (6), length 52)
10.8.3.1.8000 > 10.8.3.4.39925: Flags [.], cksum 0x7d89 (correct), seq 1066199565, ack 2570439283, win 46, options [nop,nop,TS val 295921514 ecr 138417451], length 0
10:47:07.253294 IP (tos 0x0, ttl 64, id 57162, offset 0, flags [DF], proto TCP (6), length 53)
10.8.3.4.39925 > 10.8.3.1.8000: Flags [P.], cksum 0x1a3c (incorrect -> 0x7321), seq 2570439283:2570439284, ack 1066199565, win 92, options [nop,nop,TS val 138417500 ecr 295921514], length 1
10:47:07.253468 IP (tos 0x0, ttl 64, id 36200, offset 0, flags [DF], proto TCP (6), length 52)
10.8.3.1.8000 > 10.8.3.4.39925: Flags [.], cksum 0x7d27 (correct), seq 1066199565, ack 2570439284, win 46, options [nop,nop,TS val 295921562 ecr 138417500], length 0
10:47:08.060213 IP (tos 0x0, ttl 64, id 36201, offset 0, flags [DF], proto TCP (6), length 59)
10.8.3.1.8000 > 10.8.3.4.39925: Flags [P.], cksum 0x354c (correct), seq 1066199565:1066199572, ack 2570439284, win 46, options [nop,nop,TS val 295921765 ecr 138417500], length 7
10:47:08.060270 IP (tos 0x0, ttl 64, id 57163, offset 0, flags [DF], proto TCP (6), length 52)
10.8.3.4.39925 > 10.8.3.1.8000: Flags [.], cksum 0x7b5e (correct), seq 2570439284, ack 1066199572, win 92, options [nop,nop,TS val 138417701 ecr 295921765], length 0
10:47:08.060298 IP (tos 0x0, ttl 64, id 36202, offset 0, flags [DF], proto TCP (6), length 52)
10.8.3.1.8000 > 10.8.3.4.39925: Flags [R.], cksum 0x7c51 (correct), seq 1066199572, ack 2570439284, win 46, options [nop,nop,TS val 295921765 ecr 138417500], length 0
10:47:08.060613 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto TCP (6), length 40)
10.8.3.1.8000 > 10.8.3.4.39925: Flags [R], cksum 0xb0f5 (correct), seq 1066199572, win 0, length 0
.
The connection should in theory become an orphan. I'm saying "in theory",
because since the following test was added to tcp_close(), if the client
happens to send any data between the last recv() and the close(), we
immediately send an RST to it, regardless of any pending outgoing data :
/* As outlined in RFC 2525, section 2.17, we send a RST here because
* data was lost. To witness the awful effects of the old behavior of
* always doing a FIN, run an older 2.1.x kernel or 2.0.x, start a bulk
* GET in an FTP client, suspend the process, wait for the client to
* advertise a zero window, then kill -9 the FTP client, wheee...
* Note: timeout is always zero in such a case.
*/
if (data_was_unread) {
/* Unread data was tossed, zap the connection. */
NET_INC_STATS_USER(sock_net(sk), LINUX_MIB_TCPABORTONCLOSE);
tcp_set_state(sk, TCP_CLOSE);
tcp_send_active_reset(sk, sk->sk_allocation);
}
The immediate effect then is that the client receives an abort before it
even gets the last data that were scheduled for being sent.
I've read RFC 2525 #2.17 and it shows quite interesting examples of what
it wanted to protect against. However, the recommendation did not consider
the fact that there could be some unacked pending data in the outgoing
buffers.
What is even more more embarrassing is that the HTTP working group is
trying to encourage browsers to enable pipelining by default. That means
that the situation above can become much more common, where two requests
will be pipeline, the first one will cause a short response followed by
a close(), and the simple presence of the second one will kill the first
one's data.
I tried to think about a finer way to process those unwanted data. Ideally,
we should just ignore until the ACK indicates that our last segment was
properly received. Then we could emit the RST.
I made a few attempts by first changing the test above like this :
- if (data_was_unread) {
+ if (data_was_unread && !tcp_sk(sk)->packets_out) {
then fiddling a little bit in tcp_input.c:tcp_rcv_state_process() for
the TCP_FIN_WAIT1 state, but I'm not satisfied with my experimentations,
they were a bit too much experimental for the results to be considered
reliable.
What I was looking for was a way to only send an RST when the socket is
an orphan and all of its outgoing data has been ACKed. This would cover
the situations that RFC 2525 #2.17 tries to fix without rendering orphans
unusable.
Has anyone an opinion on this, or even could suggest a patch to relax
the conditions in which we send an RST ?
Thanks,
Willy
[-- Attachment #2: abort-data.c --]
[-- Type: text/plain, Size: 1512 bytes --]
#include <stdio.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>
int port = 8000;
int one = 1;
struct sockaddr_in lst_a;
int lst_fd, srv_fd;
int lbuf;
char buf[1024];
void die_msg(const char *msg)
{
if (msg)
fprintf(stderr, "%s\n", msg);
exit(1);
}
void die_err(const char *msg)
{
perror(msg);
exit(1);
}
int main(int argc, char **argv)
{
if (argc > 1)
port = atol(argv[1]);
if ((lst_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)) == -1)
die_err("socket");
if ((setsockopt(lst_fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one))) == -1)
die_err("setsockopt");
bzero((char *)&lst_a, sizeof(lst_a));
lst_a.sin_family = AF_INET;
lst_a.sin_addr.s_addr = htonl(INADDR_ANY);
lst_a.sin_port = htons(port);
if (bind(lst_fd, (struct sockaddr *)&lst_a, sizeof(lst_a)) == -1)
die_err("bind");
if (listen(lst_fd, 1) == -1)
die_err("listen");
if ((srv_fd = accept(lst_fd, NULL, NULL)) == -1)
die_err("accept");
fprintf(stderr, "accept() returns %d\n", srv_fd);
if ((lbuf = recv(srv_fd, buf, sizeof(buf), 0)) == -1)
die_err("recv");
fprintf(stderr, "recv() returns %d\n", lbuf);
/* now let's pretend some processing time. If the sender sends any more
* data during the sleep(), it causes the response to be truncated.
*/
sleep(1);
send(srv_fd, "block1\n", 7, 0);
send(srv_fd, "block2\n", 7, 0);
//shutdown(srv_fd, SHUT_WR);
close(srv_fd);
return 0;
}
^ permalink raw reply
* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Ben Hutchings @ 2010-09-26 12:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, romieu, sgruszka, netdev
In-Reply-To: <1285481398.2436.7.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 920 bytes --]
On Sun, 2010-09-26 at 08:09 +0200, Eric Dumazet wrote:
> Le dimanche 26 septembre 2010 à 00:33 +0100, Ben Hutchings a écrit :
> > On Sat, 2010-09-25 at 11:12 +0200, Eric Dumazet wrote:
> > [...]
> > > The r8169 current problem is its currently copying all incoming frames.
> > > I guess nobody cares or noticed the performance drop.
> > > (But commit c0cd884a is recent (2.6.34), this is not yet in
> > > production...)
> >
> > Since that was a security fix it was backported to 2.6.32.12 and
> > probably most distribution kernels. So yes it is in production.
>
> Maybe in your company, not a single machine in mine ;)
>
> Most linux servers in production run much older kernels.
>
> rhel 5 -> 2.6.18
> debian 5 -> 2.6.26.x
Those both have the fix. Look for CVE-2009-4537 in the changelog.
Ben.
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
From: Michael S. Tsirkin @ 2010-09-26 11:50 UTC (permalink / raw)
To: Xin, Xiaohui
Cc: netdev@vger.kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, mingo@elte.hu, davem@davemloft.net,
herbert@gondor.hengli.com.au, jdike@linux.intel.com
In-Reply-To: <F2E9EB7348B8264F86B6AB8151CE2D792D17185524@shsmsx502.ccr.corp.intel.com>
On Thu, Sep 23, 2010 at 08:56:33PM +0800, Xin, Xiaohui wrote:
> >-----Original Message-----
> >From: Michael S. Tsirkin [mailto:mst@redhat.com]
> >Sent: Wednesday, September 22, 2010 7:55 PM
> >To: Xin, Xiaohui
> >Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> >mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
> >jdike@linux.intel.com
> >Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
> >
> >On Wed, Sep 22, 2010 at 07:41:36PM +0800, Xin, Xiaohui wrote:
> >> >-----Original Message-----
> >> >From: Michael S. Tsirkin [mailto:mst@redhat.com]
> >> >Sent: Tuesday, September 21, 2010 9:14 PM
> >> >To: Xin, Xiaohui
> >> >Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> >mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
> >> >jdike@linux.intel.com
> >> >Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
> >> >
> >> >On Tue, Sep 21, 2010 at 09:39:31AM +0800, Xin, Xiaohui wrote:
> >> >> >From: Michael S. Tsirkin [mailto:mst@redhat.com]
> >> >> >Sent: Monday, September 20, 2010 7:37 PM
> >> >> >To: Xin, Xiaohui
> >> >> >Cc: netdev@vger.kernel.org; kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> >> >mingo@elte.hu; davem@davemloft.net; herbert@gondor.hengli.com.au;
> >> >> >jdike@linux.intel.com
> >> >> >Subject: Re: [RFC PATCH v9 12/16] Add mp(mediate passthru) device.
> >> >> >
> >> >> >On Mon, Sep 20, 2010 at 04:08:48PM +0800, xiaohui.xin@intel.com wrote:
> >> >> >> From: Xin Xiaohui <xiaohui.xin@intel.com>
> >> >> >>
> >> >> >> ---
> >> >> >> Michael,
> >> >> >> I have move the ioctl to configure the locked memory to vhost
> >> >> >
> >> >> >It's ok to move this to vhost but vhost does not
> >> >> >know how much memory is needed by the backend.
> >> >>
> >> >> I think the backend here you mean is mp device.
> >> >> Actually, the memory needed is related to vq->num to run zero-copy
> >> >> smoothly.
> >> >> That means mp device did not know it but vhost did.
> >> >
> >> >Well, this might be so if you insist on locking
> >> >all posted buffers immediately. However, let's assume I have a
> >> >very large ring and prepost a ton of RX buffers:
> >> >there's no need to lock all of them directly:
> >> >
> >> >if we have buffers A and B, we can lock A, pass it
> >> >to hardware, and when A is consumed unlock A, lock B
> >> >and pass it to hardware.
> >> >
> >> >
> >> >It's not really critical. But note we can always have userspace
> >> >tell MP device all it wants to know, after all.
> >> >
> >> Ok. Here are two values we have mentioned, one is how much memory
> >> user application wants to lock, and one is how much memory locked
> >> is needed to run smoothly. When net backend is setup, we first need
> >> an ioctl to get how much memory is needed to lock, and then we call
> >> another ioctl to set how much it want to lock. Is that what's in your mind?
> >
> >That's fine.
> >
> >> >> And the rlimt stuff is per process, we use current pointer to set
> >> >> and check the rlimit, the operations should be in the same process.
> >> >
> >> >Well no, the ring is handled from the kernel thread: we switch the mm to
> >> >point to the owner task so copy from/to user and friends work, but you
> >> >can't access the rlimit etc.
> >> >
> >> Yes, the userspace and vhost kernel is not the same process. But we can
> >> record the task pointer as mm.
> >
> >So you will have to store mm and do device->mm, not current->mm.
> >Anyway, better not touch mm on data path.
> >
> >> >> Now the check operations are in vhost process, as mp_recvmsg() or
> >> >> mp_sendmsg() are called by vhost.
> >> >
> >> >Hmm, what do you mean by the check operations?
> >> >send/recv are data path operations, they shouldn't
> >> >do any checks, should they?
> >> >
> >> As you mentioned what infiniband driver done:
> >> down_write(¤t->mm->mmap_sem);
> >>
> >> locked = npages + current->mm->locked_vm;
> >> lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> >>
> >> if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) {
> >> ret = -ENOMEM;
> >> goto out;
> >> }
> >>
> >> cur_base = addr & PAGE_MASK;
> >>
> >> ret = 0;
> >> while (npages) {
> >> ret = get_user_pages(current, current->mm, cur_base,
> >> min_t(unsigned long, npages,
> >> PAGE_SIZE / sizeof (struct page *)),
> >> 1, !umem->writable, page_list, vma_list);
> >>
> >> I think it's a data path too.
> >
> >in infiniband this is used to 'register memory' which is not data path.
> >
> >> We do the check because get_user_pages() really pin and locked
> >> the memory.
> >
> >Don't do this. Performance will be bad.
> >Do the check once in ioctl and increment locked_vm by max amount you will use.
> >On data path just make sure you do not exceed what userspace told you
> >to.
>
> What's in my mind is that in the ioctl which to get the memory locked needed to run smoothly,
> it just return a value of how much memory is needed by mp device.
> And then in the ioctl which to set the memory locked by user space, it check the rlimit and
> increment locked_vm by user want.
Fine.
> But I'm not sure how to "make sure do not exceed what
> userspace told to". If we don't check locked_vm, what do we use to check? And Is it another kind of check on data path?
An example: on ioctl we have incremented locked_vm by say 128K.
We will record this number 128K in mp data structure and on data path
verify that amount of memory we actually lock with get_user_pages_fast
does not exceed 128K. This is not part of mm and so can use
any locking scheme, no need to take mm semaphore.
> >
> >>
> >> >> So set operations should be in
> >> >> vhost process too, it's natural.
> >> >>
> >> >> >So I think we'll need another ioctl in the backend
> >> >> >to tell userspace how much memory is needed?
> >> >> >
> >> >> Except vhost tells it to mp device, mp did not know
> >> >> how much memory is needed to run zero-copy smoothly.
> >> >> Is userspace interested about the memory mp is needed?
> >> >
> >> >Couldn't parse this last question.
> >> >I think userspace generally does want control over
> >> >how much memory we'll lock. We should not just lock
> >> >as much as we can.
> >> >
> >> >--
> >> >MST
^ permalink raw reply
* Re: kvm networking todo wiki
From: Michael S. Tsirkin @ 2010-09-26 11:21 UTC (permalink / raw)
To: Sridhar Samudrala
Cc: David Stevens, sri, Anthony Liguori, Rusty Russell,
Krishna Kumar2, Shirley Ma, Xin, Xiaohui, jdike, herbert, lmr,
akong, kvm, qemu-devel, netdev
In-Reply-To: <4C9A873E.6010403@us.ibm.com>
On Wed, Sep 22, 2010 at 03:46:22PM -0700, Sridhar Samudrala wrote:
> On Tue, 2010-09-21 at 18:11 +0200, Michael S. Tsirkin wrote:
>
> I've put up a wiki page with a kvm networking todo list,
> mainly to avoid effort duplication, but also in the hope
> to draw attention to what I think we should try addressing
> in KVM:
>
> http://www.linux-kvm.org/page/NetworkingTodo
>
> This page could cover all networking related activity in KVM,
> currently most info is related to virtio-net.
>
> Note: if there's no developer listed for an item,
> this just means I don't know of anyone actively working
> on an issue at the moment, not that no one intends to.
>
> I would appreciate it if others working on one of the items on this list
> would add their names so we can communicate better. If others like this
> wiki page, please go ahead and add stuff you are working on if any.
>
> It would be especially nice to add autotest projects:
> there is just a short test matrix and a catch-all
> 'Cover test matrix with autotest', currently.
>
> Currently there are some links to Red Hat bugzilla entries,
> feel free to add links to other bugzillas.
>
>
> Thanks for capturing these items. It is really useful.
>
> Another item that is missing is
> - support assigning SR-IOV VF to a guest via tap/macvtap
>
> Currently, this requires
> - VF to be put in promiscuous mode when using a bridge/tap
> - add a new mac address to VF when using macvtap.
>
> I don't think any of the VF drivers provide these capabilities
> at this time.
>
> -Sridhar
I think this is part of what is needed for the work item:
* guest programmable mac/vlan filtering with macvtap
If yes pls add this detail if not add another item.
More importantly: anyone's going to work on this?
--
MST
^ permalink raw reply
* [PATCH 3/9] sgiseeq: use free_netdev(netdev) instead of kfree()
From: Vasiliy Kulikov @ 2010-09-26 9:58 UTC (permalink / raw)
To: kernel-janitors
Cc: David S. Miller, Eric Dumazet, Uwe Kleine-König,
Julia Lawall, Tejun Heo, netdev, linux-kernel
Freeing netdev without free_netdev() leads to net, tx leaks.
I might lead to dereferencing freed pointer.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
@@
struct net_device* dev;
@@
-kfree(dev)
+free_netdev(dev)
---
drivers/net/sgiseeq.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/sgiseeq.c b/drivers/net/sgiseeq.c
index cc4bd8c..9265315 100644
--- a/drivers/net/sgiseeq.c
+++ b/drivers/net/sgiseeq.c
@@ -804,7 +804,7 @@ static int __devinit sgiseeq_probe(struct platform_device *pdev)
err_out_free_page:
free_page((unsigned long) sp->srings);
err_out_free_dev:
- kfree(dev);
+ free_netdev(dev);
err_out:
return err;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/9] rionet: use free_netdev(netdev) instead of kfree()
From: Vasiliy Kulikov @ 2010-09-26 9:58 UTC (permalink / raw)
To: kernel-janitors; +Cc: Tejun Heo, netdev, linux-kernel
Freeing netdev without free_netdev() leads to net, tx leaks.
I might lead to dereferencing freed pointer.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
@@
struct net_device* dev;
@@
-kfree(dev)
+free_netdev(dev)
---
drivers/net/rionet.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/rionet.c b/drivers/net/rionet.c
index 07eb884..44150f2 100644
--- a/drivers/net/rionet.c
+++ b/drivers/net/rionet.c
@@ -384,7 +384,7 @@ static void rionet_remove(struct rio_dev *rdev)
free_pages((unsigned long)rionet_active, rdev->net->hport->sys_size ?
__ilog2(sizeof(void *)) + 4 : 0);
unregister_netdev(ndev);
- kfree(ndev);
+ free_netdev(ndev);
list_for_each_entry_safe(peer, tmp, &rionet_peers, node) {
list_del(&peer->node);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/9] ibm_newemac: use free_netdev(netdev) instead of kfree()
From: Vasiliy Kulikov @ 2010-09-26 9:58 UTC (permalink / raw)
To: kernel-janitors-u79uwXL29TY76Z2rM5mHXA
Cc: Jiri Pirko, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David S. Miller
Freeing netdev without free_netdev() leads to net, tx leaks.
I might lead to dereferencing freed pointer.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
@@
struct net_device* dev;
@@
-kfree(dev)
+free_netdev(dev)
---
drivers/net/ibm_newemac/core.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c
index 3506fd6..519e19e 100644
--- a/drivers/net/ibm_newemac/core.c
+++ b/drivers/net/ibm_newemac/core.c
@@ -2928,7 +2928,7 @@ static int __devinit emac_probe(struct platform_device *ofdev,
if (dev->emac_irq != NO_IRQ)
irq_dispose_mapping(dev->emac_irq);
err_free:
- kfree(ndev);
+ free_netdev(ndev);
err_gone:
/* if we were on the bootlist, remove us as we won't show up and
* wake up all waiters to notify them in case they were waiting
@@ -2971,7 +2971,7 @@ static int __devexit emac_remove(struct platform_device *ofdev)
if (dev->emac_irq != NO_IRQ)
irq_dispose_mapping(dev->emac_irq);
- kfree(dev->ndev);
+ free_netdev(dev->ndev);
return 0;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Eric Dumazet @ 2010-09-26 6:09 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, romieu, sgruszka, netdev
In-Reply-To: <1285457597.2697.183.camel@localhost>
Le dimanche 26 septembre 2010 à 00:33 +0100, Ben Hutchings a écrit :
> On Sat, 2010-09-25 at 11:12 +0200, Eric Dumazet wrote:
> [...]
> > The r8169 current problem is its currently copying all incoming frames.
> > I guess nobody cares or noticed the performance drop.
> > (But commit c0cd884a is recent (2.6.34), this is not yet in
> > production...)
>
> Since that was a security fix it was backported to 2.6.32.12 and
> probably most distribution kernels. So yes it is in production.
Maybe in your company, not a single machine in mine ;)
Most linux servers in production run much older kernels.
rhel 5 -> 2.6.18
debian 5 -> 2.6.26.x
^ permalink raw reply
* Re: Deadlock in Bluetooth code in 2.6.36
From: Gustavo F. Padovan @ 2010-09-26 0:14 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
marcel-kz+m5ild9QBg9hUCZPvPmw, mathewm-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <20100924.211824.242130957.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
* David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> [2010-09-24 21:18:24 -0700]:
> From: "Gustavo F. Padovan" <padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
> Date: Tue, 21 Sep 2010 18:20:12 -0300
>
> > My questions here is on how to fix this properly. Maybe
> > sock_alloc_send_skb() should not be used with SOCK_STREAM and reliable
> > protocols and I'm not aware of that? And should I use something like
> > sk_stream_alloc_skb() instead?
> >
> > Any comments are welcome. With lucky we can fix that for 2.6.36 and
> > together with others fixes we have queued deliver a fully functional
> > L2CAP layer on 2.6.36.
>
> Use sock_alloc_send_skb() as you do now, but if it fails wait for socket
> space to become available just like TCP does, then loop back and try to
> allocate again if the space-wait doesn't return an error.
sock_alloc_send_skb() doesn't fail when there is no space available
it sleeps and try again later. That is the problem. So if
sock_alloc_send_skb() is called with the socket lock held potentially we
can have a starvation in the backlog queue and then the deadlock due to
be unable to read the acknowledgments packets residing in the backlog
queue.
Our current solution is release the lock before call
sock_alloc_send_skb() and acquire it again when sock_alloc_send_skb()
returns. Patch follows:
------
Bluetooth: Fix deadlock in the ERTM logic
The Enhanced Retransmission Mode(ERTM) is a reliable mode of operation
of the Bluetooth L2CAP layer. Think on it like a simplified version of
TCP.
The problem we were facing here was a deadlock. ERTM uses a backlog
queue to queue incomimg packets while the user is helding the lock. At
some moment the sk_sndbuf can be exceeded and we can't alloc new skbs
then the code sleep with the lock to wait for memory, that stalls the
ERTM connection once we can't read the acknowledgements packets in the
backlog queue to free memory and make the allocation of outcoming skb
successful.
This patch actually affect all users of bt_skb_send_alloc(), i.e., all
L2CAP modes and SCO.
Signed-off-by: Gustavo F. Padovan <padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
Signed-off-by: Ulisses Furquim <ulisses-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
---
include/net/bluetooth/bluetooth.h | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..118b69b 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -161,10 +161,21 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk, unsigned long l
{
struct sk_buff *skb;
+ release_sock(sk);
if ((skb = sock_alloc_send_skb(sk, len + BT_SKB_RESERVE, nb, err))) {
skb_reserve(skb, BT_SKB_RESERVE);
bt_cb(skb)->incoming = 0;
}
+ lock_sock(sk);
+
+ if (*err)
+ return NULL;
+
+ *err = sock_error(sk);
+ if (*err) {
+ kfree_skb(skb);
+ return NULL;
+ }
return skb;
}
--
1.7.2.2
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply related
* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Ben Hutchings @ 2010-09-25 23:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, romieu, sgruszka, netdev
In-Reply-To: <1285405945.2478.255.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 510 bytes --]
On Sat, 2010-09-25 at 11:12 +0200, Eric Dumazet wrote:
[...]
> The r8169 current problem is its currently copying all incoming frames.
> I guess nobody cares or noticed the performance drop.
> (But commit c0cd884a is recent (2.6.34), this is not yet in
> production...)
Since that was a security fix it was backported to 2.6.32.12 and
probably most distribution kernels. So yes it is in production.
Ben.
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH] de2104x: fix TP link detection
From: Jeff Garzik @ 2010-09-25 20:58 UTC (permalink / raw)
To: Ondrej Zary; +Cc: netdev, Kernel development list
In-Reply-To: <201009252239.21376.linux@rainbow-software.org>
On Sat, Sep 25, 2010 at 4:39 PM, Ondrej Zary <linux@rainbow-software.org> wrote:
> @@ -1160,14 +1164,29 @@ no_link_yet:
> static void de_media_interrupt (struct de_private *de, u32 status)
> {
> if (status & LinkPass) {
> + /* Ignore if current media is AUI or BNC and we can't use TP */
> + if ((de->media_type == DE_MEDIA_AUI ||
> + de->media_type == DE_MEDIA_BNC) &&
> + (de->media_lock ||
> + !de_ok_to_advertise(de, DE_MEDIA_TP_AUTO)))
> + return;
> + /* If current media is not TP, change it to TP */
> + if ((de->media_type == DE_MEDIA_AUI ||
> + de->media_type == DE_MEDIA_BNC)) {
> + de->media_type = DE_MEDIA_TP_AUTO;
> + de_stop_rxtx(de);
> + de_set_media(de);
> + de_start_rxtx(de);
> + }
> de_link_up(de);
> mod_timer(&de->media_timer, jiffies + DE_TIMER_LINK);
> return;
> }
>
> BUG_ON(!(status & LinkFail));
> -
> - if (netif_carrier_ok(de->dev)) {
> + /* Mark the link as down only if current media is TP */
> + if (netif_carrier_ok(de->dev) && de->media_type != DE_MEDIA_AUI &&
> + de->media_type != DE_MEDIA_BNC) {
> de_link_down(de);
> mod_timer(&de->media_timer, jiffies + DE_TIMER_NO_LINK);
> }
Acked-by: Jeff Garzik <jgarzik@redhat.com>
^ permalink raw reply
* [PATCH] de2104x: fix TP link detection
From: Ondrej Zary @ 2010-09-25 20:39 UTC (permalink / raw)
To: jgarzik; +Cc: netdev, Kernel development list
Compex FreedomLine 32 PnP-PCI2 cards have only TP and BNC connectors but the
SROM contains AUI port too. When TP loses link, the driver switches to
non-existing AUI port (which reports that carrier is always present).
Connecting TP back generates LinkPass interrupt but de_media_interrupt() is
broken - it only updates the link state of currently connected media, ignoring
the fact that LinkPass and LinkFail bits of MacStatus register belong to the
TP port only (the chip documentation says that).
This patch changes de_media_interrupt() to switch media to TP when link goes
up (and media type is not locked) and also to update the link state only when
the TP port is used.
Also the NonselPortActive (and also SelPortActive) bits of SIAStatus register
need to be cleared (by writing 1) after reading or they're useless.
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
--- linux-2.6.36-rc3-/drivers/net/tulip/de2104x.c 2010-09-25 21:19:53.000000000 +0200
+++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c 2010-09-25 21:22:11.000000000 +0200
@@ -243,6 +243,7 @@ enum {
NWayState = (1 << 14) | (1 << 13) | (1 << 12),
NWayRestart = (1 << 12),
NonselPortActive = (1 << 9),
+ SelPortActive = (1 << 8),
LinkFailStatus = (1 << 2),
NetCxnErr = (1 << 1),
};
@@ -1066,6 +1067,9 @@ static void de21041_media_timer (unsigne
unsigned int carrier;
unsigned long flags;
+ /* clear port active bits */
+ dw32(SIAStatus, NonselPortActive | SelPortActive);
+
carrier = (status & NetCxnErr) ? 0 : 1;
if (carrier) {
@@ -1160,14 +1164,29 @@ no_link_yet:
static void de_media_interrupt (struct de_private *de, u32 status)
{
if (status & LinkPass) {
+ /* Ignore if current media is AUI or BNC and we can't use TP */
+ if ((de->media_type == DE_MEDIA_AUI ||
+ de->media_type == DE_MEDIA_BNC) &&
+ (de->media_lock ||
+ !de_ok_to_advertise(de, DE_MEDIA_TP_AUTO)))
+ return;
+ /* If current media is not TP, change it to TP */
+ if ((de->media_type == DE_MEDIA_AUI ||
+ de->media_type == DE_MEDIA_BNC)) {
+ de->media_type = DE_MEDIA_TP_AUTO;
+ de_stop_rxtx(de);
+ de_set_media(de);
+ de_start_rxtx(de);
+ }
de_link_up(de);
mod_timer(&de->media_timer, jiffies + DE_TIMER_LINK);
return;
}
BUG_ON(!(status & LinkFail));
-
- if (netif_carrier_ok(de->dev)) {
+ /* Mark the link as down only if current media is TP */
+ if (netif_carrier_ok(de->dev) && de->media_type != DE_MEDIA_AUI &&
+ de->media_type != DE_MEDIA_BNC) {
de_link_down(de);
mod_timer(&de->media_timer, jiffies + DE_TIMER_NO_LINK);
}
--
Ondrej Zary
^ permalink raw reply
* Re: de2104x: fix power management
From: Jeff Garzik @ 2010-09-25 17:16 UTC (permalink / raw)
To: Ondrej Zary; +Cc: netdev, Kernel development list
In-Reply-To: <201009251157.07197.linux@rainbow-software.org>
On 09/25/2010 05:57 AM, Ondrej Zary wrote:
> At least my 21041 cards come out of suspend with bus mastering disabled so
> they did not work after resume(no data transferred).
> After adding pci_set_master(), the driver oopsed immediately on resume -
> because de_clean_rings() is called on suspend but de_init_rings() call
> was missing in resume.
>
> Also disable link (reset SIA) before sleep (de4x5 does this too).
>
> Signed-off-by: Ondrej Zary<linux@rainbow-software.org>
>
> --- linux-2.6.36-rc3-/drivers/net/tulip/de2104x.c 2010-09-25 11:27:26.000000000 +0200
> +++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c 2010-09-25 11:29:22.000000000 +0200
> @@ -1231,6 +1231,7 @@ static void de_adapter_sleep (struct de_
> if (de->de21040)
> return;
>
> + dw32(CSR13, 0); /* Reset phy */
> pci_read_config_dword(de->pdev, PCIPM,&pmctl);
> pmctl |= PM_Sleep;
> pci_write_config_dword(de->pdev, PCIPM, pmctl);
> @@ -2166,6 +2167,8 @@ static int de_resume (struct pci_dev *pd
> dev_err(&dev->dev, "pci_enable_device failed in resume\n");
> goto out;
> }
> + pci_set_master(pdev);
> + de_init_rings(de);
> de_init_hw(de);
> out_attach:
> netif_device_attach(dev);
Acked-by: Jeff Garzik <jgarzik@redhat.com>
IMO suspend/resume could use another look; if netif_running is false
(interface is down), de2104x does not put the PCI device to sleep, which
seems strange.
I also wonder if rtnl_lock() couldn't be eliminated, as other net
drivers don't need it in suspend/resume.
^ permalink raw reply
* Re: [rfc] IPVS: Masq local real-servers
From: Julian Anastasov @ 2010-09-25 13:54 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netfilter-devel, netdev, Patrick McHardy,
Wensong Zhang
In-Reply-To: <20100920090911.GA4646@verge.net.au>
Hello,
On Mon, 20 Sep 2010, Simon Horman wrote:
> IPVS has a special Local forwarding mechanism that is used if the
> real-server is a local IP address. Like the Route and Tunnel forwarding
> mechanism Local does not allow port mapping, and thus the port of the
> real-server is always set to be the same as the virtual service.
>
> The Masq forwarding mechanism does allow port mapping, and this causes some
> confusion when the real-server happens to be local.
>
> This patch addresses this confusion by not using the Local forwarding
> mechanism if the masq forwarding mechanism is requested. That is, the masq
> forwarding mechanism will be used, and the real-servers may have a
> different port to the virtual service.
Idea is good but there are some things to consider:
- ip_vs_nat_xmit should check the route to real server IP
but must return NF_ACCEPT instead of creating loops by
sending the packets with IP_VS_XMIT. Checks for MTU,
hard_header_len and replacing of skb_dst should be avoided
for the 'rt->rt_flags & RTCF_LOCAL' case. As result,
the packet will be DNAT-ed by IPVS and the conntrack
will be altered to expect reply from the correct local port
and then just like ip_vs_null_xmit the traffic will
reach local stack. 127.0.0.1/8 should be rejected
for the MASQ method because the output routing called
by local stack will not want to send such saddr in replies.
- We have to check if ip_vs_out is prepared to work in
LOCAL_OUT: I hope icmp_send works from this hook.
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
>
> I considered using Local for the case where the real-server and virtual
> service ports are the same. However, this would require updating the
> real-servers if the port of the virtual-service was changed, however
> editing the forwarding mechanism of a real-server currently isn't
> supported and the extra complexity for an unmeasured performance gain seems
> to be at best left for another patch.
>
> Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c
> ===================================================================
> --- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_core.c 2010-09-19 20:51:30.000000000 +0900
> +++ nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c 2010-09-20 16:30:59.000000000 +0900
> @@ -1496,6 +1496,22 @@ static struct nf_hook_ops ip_vs_ops[] __
> .hooknum = NF_INET_FORWARD,
> .priority = 100,
> },
Double block?:
> + /* change source only for local VS/NAT */
> + {
> + .hook = ip_vs_out,
> + .owner = THIS_MODULE,
> + .pf = PF_INET,
> + .hooknum = NF_INET_LOCAL_OUT,
> + .priority = 100,
> + },
> + /* change source only for local VS/NAT */
> + {
> + .hook = ip_vs_out,
> + .owner = THIS_MODULE,
> + .pf = PF_INET,
> + .hooknum = NF_INET_LOCAL_OUT,
> + .priority = 100,
> + },
> /* After packet filtering (but before ip_vs_out_icmp), catch icmp
> * destined for 0.0.0.0/0, which is for incoming IPVS connections */
> {
> Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_ctl.c
> ===================================================================
> --- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_ctl.c 2010-09-20 15:07:27.000000000 +0900
> +++ nf-next-2.6/net/netfilter/ipvs/ip_vs_ctl.c 2010-09-20 17:45:46.000000000 +0900
> @@ -766,7 +766,7 @@ ip_vs_zero_stats(struct ip_vs_stats *sta
> * Update a destination in the given service
> */
> static void
'_' in __ip_vs_update_dest was lost?:
> -__ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
> +_ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
> struct ip_vs_dest_user_kern *udest, int add)
> {
> int conn_flags;
> @@ -777,18 +777,22 @@ __ip_vs_update_dest(struct ip_vs_service
> conn_flags |= IP_VS_CONN_F_INACTIVE;
>
> /* check if local node and update the flags */
I think, it should work for svc->fwmark too, eg.
consolidating traffic for many virtual IPs to single
listening socket.
> + if ((conn_flags & IP_VS_CONN_F_FWD_MASK) != IP_VS_CONN_F_MASQ ||
> + svc->fwmark) {
> #ifdef CONFIG_IP_VS_IPV6
> - if (svc->af == AF_INET6) {
> - if (__ip_vs_addr_is_local_v6(&udest->addr.in6)) {
> - conn_flags = (conn_flags & ~IP_VS_CONN_F_FWD_MASK)
> - | IP_VS_CONN_F_LOCALNODE;
> - }
> - } else
> + if (svc->af == AF_INET6) {
> + if (__ip_vs_addr_is_local_v6(&udest->addr.in6)) {
> + conn_flags = (conn_flags &
> + ~IP_VS_CONN_F_FWD_MASK) |
> + IP_VS_CONN_F_LOCALNODE;
> + }
> + } else
> #endif
> if (inet_addr_type(&init_net, udest->addr.ip) == RTN_LOCAL) {
> conn_flags = (conn_flags & ~IP_VS_CONN_F_FWD_MASK)
> | IP_VS_CONN_F_LOCALNODE;
> }
> + }
>
> /* set the IP_VS_CONN_F_NOOUTPUT flag if not masquerading/NAT */
> if ((conn_flags & IP_VS_CONN_F_FWD_MASK) != IP_VS_CONN_F_MASQ) {
>
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* de2104x: fix power management
From: Ondrej Zary @ 2010-09-25 9:57 UTC (permalink / raw)
To: jgarzik; +Cc: netdev, Kernel development list
At least my 21041 cards come out of suspend with bus mastering disabled so
they did not work after resume(no data transferred).
After adding pci_set_master(), the driver oopsed immediately on resume -
because de_clean_rings() is called on suspend but de_init_rings() call
was missing in resume.
Also disable link (reset SIA) before sleep (de4x5 does this too).
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
--- linux-2.6.36-rc3-/drivers/net/tulip/de2104x.c 2010-09-25 11:27:26.000000000 +0200
+++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c 2010-09-25 11:29:22.000000000 +0200
@@ -1231,6 +1231,7 @@ static void de_adapter_sleep (struct de_
if (de->de21040)
return;
+ dw32(CSR13, 0); /* Reset phy */
pci_read_config_dword(de->pdev, PCIPM, &pmctl);
pmctl |= PM_Sleep;
pci_write_config_dword(de->pdev, PCIPM, pmctl);
@@ -2166,6 +2167,8 @@ static int de_resume (struct pci_dev *pd
dev_err(&dev->dev, "pci_enable_device failed in resume\n");
goto out;
}
+ pci_set_master(pdev);
+ de_init_rings(de);
de_init_hw(de);
out_attach:
netif_device_attach(dev);
--
Ondrej Zary
^ permalink raw reply
* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Eric Dumazet @ 2010-09-25 9:12 UTC (permalink / raw)
To: David Miller; +Cc: romieu, sgruszka, netdev
In-Reply-To: <20100925.001300.193725148.davem@davemloft.net>
Le samedi 25 septembre 2010 à 00:13 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sat, 25 Sep 2010 08:06:37 +0200
>
> > Patch solves the suspend/resume, probably, but as soon as we receive
> > trafic, we can hit the allocation error anyway...
>
> It allocates 1536 + N, where N can be NET_IP_ALIGN, or some small
> value like 8.
>
> This is in the same ballpark as what tg3 allocates for RX buffers.
>
> SLAB/SLUB/whatever just wants multi-order page allocations even
> for SKBs which are about this size.
>
> Furthermore, the sleeping allocations we do at ->open() time to
> allocate the entire RX ring all at once will buddy up a lot of
> pages and make 1-order allocs more likely.
Yes, I forgot this problem about SLUB (I ended using SLAB on servers
because of this order-3 problem on kmalloc(2048))
bnx2 uses GFP_KERNEL allocations at init time, but tg3 uses GFP_ATOMIC
(because tp->lock is held).
The r8169 current problem is its currently copying all incoming frames.
I guess nobody cares or noticed the performance drop.
(But commit c0cd884a is recent (2.6.34), this is not yet in
production...)
^ permalink raw reply
* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: David Miller @ 2010-09-25 7:13 UTC (permalink / raw)
To: eric.dumazet; +Cc: romieu, sgruszka, netdev
In-Reply-To: <1285394797.2478.54.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 25 Sep 2010 08:06:37 +0200
> Patch solves the suspend/resume, probably, but as soon as we receive
> trafic, we can hit the allocation error anyway...
It allocates 1536 + N, where N can be NET_IP_ALIGN, or some small
value like 8.
This is in the same ballpark as what tg3 allocates for RX buffers.
SLAB/SLUB/whatever just wants multi-order page allocations even
for SKBs which are about this size.
Furthermore, the sleeping allocations we do at ->open() time to
allocate the entire RX ring all at once will buddy up a lot of
pages and make 1-order allocs more likely.
^ permalink raw reply
* Re: [PATCH] net: reset skb queue mapping when rx'ing over tunnel
From: Eric Dumazet @ 2010-09-25 6:49 UTC (permalink / raw)
To: Tom Herbert; +Cc: netdev, davem, chavey
In-Reply-To: <AANLkTimG9ZCZwDyo_9bYURShn7hk6ROzX4-qypo7nG5i@mail.gmail.com>
Le vendredi 24 septembre 2010 à 09:18 -0700, Tom Herbert a écrit :
> > Hmm...
> >
> > This would need to be reverted later when tunnels are updated to be
> > multiqueue aware ? I made an attempt with GRE some days ago.
> >
> > I dont understand why this patch is needed, since get_rps_cpu() has a
> > check anyway
> >
> I think the skb queue_mapping should correspond to a queue in the
> skb's device as an invariant. In the case that an skb's device is
> change from a multiqueue device to single queue device (like GRE), the
> inconsistency in the queue_mapping is fairly innocuous, we get one
> warning but will pretty much take the unlikely branch for GRE packets
> then on. But imagine a case where skb's device was change from one
> multiqueue device to another, but the queue mapping was not also
> updated. This would cause poor weighting in get_rps_cpus. For
> example, if the new device had fewer queues than the old one,
> get_rps_cpu will bias toward using queue 0's rps mask.
I believe your patch is fine. Thanks
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
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