* Re: [REGRESSION][PATCH] bpf_jit drops the ball on indirect negative mem references
From: Eric Dumazet @ 2012-03-28 20:39 UTC (permalink / raw)
To: Jan Seiffert; +Cc: netdev, linux-kernel, David S. Miller, Matt Evans
In-Reply-To: <CAPwCz7_tnjq32duJSDj21LpRe5TFvGGszH4joQr1DGTjNd7GcA@mail.gmail.com>
On Wed, 2012-03-28 at 22:26 +0200, Jan Seiffert wrote:
> 2012/3/28 Eric Dumazet <eric.dumazet@gmail.com>:
> > On Wed, 2012-03-28 at 21:15 +0200, Jan Seiffert wrote:
> >> Consider the following test program:
> >>
> >> #include <stdio.h>
> >> #include <sys/types.h>
> >> #include <sys/socket.h>
> >> #include <netinet/in.h>
> >> #include <pcap-bpf.h>
> >>
> >> #define die(x) do {perror(x); return 1;} while (0)
> >> struct bpf_insn udp_filter[] = {
> >> /* 0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(0)), /* leax net[0] */
> >> /* 1 */ BPF_STMT(BPF_LD|BPF_B|BPF_IND, 0), /* ldb [x+0] */
> >> /* 2 */ BPF_STMT(BPF_RET|BPF_A, 0), /* ret a */
> >> };
> >
> > When this point was raised some weeks ago, we wanted to see a _real_ use
> > of negative mem reference.
> >
> > You provide a test program but what this filter is supposed to do
> > exactly ?
> >
>
> Say you have a UDP socket, and you want to filter for bogus source
> addresses (drop already in kernel to save the context switch).
> To have only one bpf program for ipv4 and ipv6 (you have to checked
> the same bogus v4 addresses in mapped space), there is a point where
> it elegant to have a negative offset saved in the X register.
Cool, thats a valid use, thanks.
Problem is you slow down the jit in its normal use, for a very specific
use.
Please rework your patch so that absolute loads of positive offsets
(known at compile time) dont have to test negative offsets at run time.
You add two instructions per load, and thats not good.
Something like :
sk_load_word:
.globl sk_load_word
test %esi,%esi
js bpf_slow_path_word_neg
sk_load_word_positive_offset:
.globl sk_load_word_positive_offset
mov %r9d,%eax # hlen
sub %esi,%eax # hlen - offset
cmp $3,%eax
...
^ permalink raw reply
* Re: [PATCH 2/3] tcp: Initial repair mode
From: Ben Hutchings @ 2012-03-28 20:39 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <4F73302C.9030209@parallels.com>
On Wed, 2012-03-28 at 19:37 +0400, Pavel Emelyanov wrote:
[...]
> * Ability to forcibly bind a socket to a port
>
> The sk->sk_reuse is set to 2 denoting, that the socket is question
> should be bound as if all the others in the system are configured
> with the SO_REUSEADDR option.
Shouldn't this constant be named?
[...]
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
[...]
> + case TCP_REPAIR_QUEUE:
> + if (!tp->repair)
> + err = -EPERM;
> + else if (val <= TCP_QUEUES_NR)
Off-by-one.
> + tp->repair_queue = val;
> + else
> + err = -EINVAL;
> + break;
[...]
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [REGRESSION][PATCH] bpf_jit drops the ball on indirect negative mem references
From: Jan Seiffert @ 2012-03-28 20:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, linux-kernel, David S. Miller, Matt Evans
In-Reply-To: <1332965129.2325.12.camel@edumazet-glaptop>
2012/3/28 Eric Dumazet <eric.dumazet@gmail.com>:
> On Wed, 2012-03-28 at 21:15 +0200, Jan Seiffert wrote:
>> Consider the following test program:
>>
>> #include <stdio.h>
>> #include <sys/types.h>
>> #include <sys/socket.h>
>> #include <netinet/in.h>
>> #include <pcap-bpf.h>
>>
>> #define die(x) do {perror(x); return 1;} while (0)
>> struct bpf_insn udp_filter[] = {
>> /* 0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(0)), /* leax net[0] */
>> /* 1 */ BPF_STMT(BPF_LD|BPF_B|BPF_IND, 0), /* ldb [x+0] */
>> /* 2 */ BPF_STMT(BPF_RET|BPF_A, 0), /* ret a */
>> };
>
> When this point was raised some weeks ago, we wanted to see a _real_ use
> of negative mem reference.
>
> You provide a test program but what this filter is supposed to do
> exactly ?
>
Say you have a UDP socket, and you want to filter for bogus source
addresses (drop already in kernel to save the context switch).
To have only one bpf program for ipv4 and ipv6 (you have to checked
the same bogus v4 addresses in mapped space), there is a point where
it elegant to have a negative offset saved in the X register.
This is how i found the Bug.
But use case or not, the jits behavior is different than the interpreter.
Example (not the complete program):
struct bpf_insn UDPPValid[] = {
/* 0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(12)),
/* 1 */ BPF_STMT(BPF_LD|BPF_B|BPF_ABS, -1048576+(0)),
/* 2 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xf0),
/* 3 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x40, 22 - 4, 0),
/* 4 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x60, 5 - 5, 39 - 5),
/* 5 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 12 - 6, 0),
/* 6 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010DB8, 39 - 7, 0),
/* 7 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010002, 18 - 8, 0),
/* 8 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xfffffff0),
/* 9 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010010, 39 - 10, 0),
/* 10 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xff000000),
/* 11 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xff000000, 39 - 12, 37 - 12),
/* 12 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(12)),
/* 13 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 0, 37 - 14),
/* 14 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(16)),
/* 15 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xffff, 21 - 16, 0),
/* 16 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x0064FF9B, 21 - 17, 0),
/* 17 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 39 - 18, 37 -
18),
/* 18 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(12)),
/* 19 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xffff0000),
/* 20 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 39 - 21, 37 -
21),
/* 21 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(20)),
/* 22 */ BPF_STMT(BPF_LD|BPF_W|BPF_IND, 0),
/* 23 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xffffffff, 39 - 24,
0),
/* 24 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xffffff00),
/* 25 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0000000, 39 - 26,
0),
/* 26 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0000200, 39 - 27,
0),
/* 27 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC6336400, 39 - 28,
0),
/* 28 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xCB007100, 39 - 29,
0),
/* 29 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0586300, 39 - 30,
0),
/* 30 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xfffe0000),
/* 31 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC6120000, 39 - 32,
0),
/* 32 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xff000000),
/* 33 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 39 - 34, 0),
/* 34 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xf0000000),
/* 35 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xE0000000, 39 - 36,
0),
/* 36 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xF0000000, 39 - 37,
0),
/* 37 */ BPF_STMT(BPF_LD|BPF_W|BPF_LEN, 0),
/* 38 */ BPF_STMT(BPF_RET|BPF_A, 0),
/* 39 */ BPF_STMT(BPF_RET|BPF_K, 0),
};
>
>
Greetings
Jan
--
˙qɐɥ ʇɟnɐʞǝƃ ʎɐqǝ ıǝq ɹnʇɐʇsɐʇ ǝuıǝ ɹıɯ ɥɔı sɐp lɐɯ ǝʇzʇǝl sɐp ʇsı sɐp
'ʇɯɯɐpɹǝʌ
^ permalink raw reply
* Re: [PATCH] tcp: bind() use stronger condition for bind_conflict
From: Eric Dumazet @ 2012-03-28 20:12 UTC (permalink / raw)
To: Daniel Baluta
Cc: Flavio Leitner, Alexandru Copot, davem, kuznet, jmorris, yoshfuji,
kaber, netdev, linux-kernel
In-Reply-To: <CAEnQRZD+0AQFfFCzWZCR5JR0BUxxNEsincD+_O9F0ne2Zryp_w@mail.gmail.com>
On Wed, 2012-03-28 at 23:08 +0300, Daniel Baluta wrote:
> Eric, Flavio, thanks a lot for your comments. We will resubmit this
> when net-next opens.
>
> Please let us know if you have any other observations.
Some performance data would be welcomed, in the case many sockets are
already bound...
Thanks
^ permalink raw reply
* Re: [PATCH] tcp: bind() use stronger condition for bind_conflict
From: Daniel Baluta @ 2012-03-28 20:08 UTC (permalink / raw)
To: Eric Dumazet, Flavio Leitner
Cc: Alexandru Copot, davem, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <1332946113.2325.9.camel@edumazet-glaptop>
On Wed, Mar 28, 2012 at 5:48 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2012-03-28 at 10:52 -0300, Flavio Leitner wrote:
>> On Wed, 28 Mar 2012 00:11:52 +0300
>> Alexandru Copot <alex.mihai.c@gmail.com> wrote:
>>
>
>> > diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
>> > index 02dd203..dfc8bc3 100644
>> > --- a/net/ipv6/inet6_connection_sock.c
>> > +++ b/net/ipv6/inet6_connection_sock.c
>> > @@ -28,7 +28,8 @@
>> > #include <net/inet6_connection_sock.h>
>> >
>> > int inet6_csk_bind_conflict(const struct sock *sk,
>> > - const struct inet_bind_bucket *tb)
>> > + const struct inet_bind_bucket *tb,
>> > + int relax)
>>
>> Same here
>> Maybe you can wait for more feedbacks before spin another patch
>> version just to fix that.
>>
>
>
>
> Also 'int relax' should be 'bool relax'... 1/0 -> true/false
>
> Anyway net-next is not open yet...
Eric, Flavio, thanks a lot for your comments. We will resubmit this
when net-next opens.
Please let us know if you have any other observations.
thanks,
Daniel.
^ permalink raw reply
* Re: [REGRESSION][PATCH] bpf_jit drops the ball on indirect negative mem references
From: Eric Dumazet @ 2012-03-28 20:05 UTC (permalink / raw)
To: Jan Seiffert; +Cc: netdev, linux-kernel, David S. Miller, Matt Evans
In-Reply-To: <CAPwCz79wO=5zTZmCG+ePfRuEu-p_Vosi18bYZ-MVNZGOdbE3aQ@mail.gmail.com>
On Wed, 2012-03-28 at 21:15 +0200, Jan Seiffert wrote:
> Consider the following test program:
>
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <netinet/in.h>
> #include <pcap-bpf.h>
>
> #define die(x) do {perror(x); return 1;} while (0)
> struct bpf_insn udp_filter[] = {
> /* 0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(0)), /* leax net[0] */
> /* 1 */ BPF_STMT(BPF_LD|BPF_B|BPF_IND, 0), /* ldb [x+0] */
> /* 2 */ BPF_STMT(BPF_RET|BPF_A, 0), /* ret a */
> };
When this point was raised some weeks ago, we wanted to see a _real_ use
of negative mem reference.
You provide a test program but what this filter is supposed to do
exactly ?
^ permalink raw reply
* [REGRESSION][PATCH] bpf_jit drops the ball on indirect negative mem references
From: Jan Seiffert @ 2012-03-28 19:15 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, David S. Miller, Matt Evans, Eric Dumazet
[-- Attachment #1: Type: text/plain, Size: 2386 bytes --]
Consider the following test program:
#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <pcap-bpf.h>
#define die(x) do {perror(x); return 1;} while (0)
struct bpf_insn udp_filter[] = {
/* 0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(0)), /* leax net[0] */
/* 1 */ BPF_STMT(BPF_LD|BPF_B|BPF_IND, 0), /* ldb [x+0] */
/* 2 */ BPF_STMT(BPF_RET|BPF_A, 0), /* ret a */
};
int main(int argc, char *argv[])
{
char buf[512];
struct sockaddr_in addr;
struct bpf_program prg;
socklen_t addr_s;
ssize_t res;
int fd;
addr.sin_family = AF_INET;
addr.sin_port = htons(5000);
addr.sin_addr.s_addr = 0;
addr_s = sizeof(addr);
prg.bf_len = sizeof(udp_filter)/sizeof(udp_filter[0]);
prg.bf_insns = udp_filter;
if(-1 == (fd = socket(AF_INET, SOCK_DGRAM, 0)))
die("socket");
if(-1 == bind(fd, (struct sockaddr *)&addr, sizeof(addr)))
die("bind");
if(-1 == setsockopt(fd, SOL_SOCKET, SO_ATTACH_FILTER, &prg, sizeof(prg)))
die("setsockopt");
res = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *)&addr, &addr_s);
if(res != -1)
printf("packet received: %zi bytes\n", res);
else
die("recvfrom");
return 0;
}
when used with the bpf jit disabled works:
console 1 $ ./bpf
console 2 $ echo "hello" | nc -u localhost 5000
console 1: packet received: 6 bytes
When the bpf jit gets enabled (echo 100 >
/proc/sys/net/core/bpf_jit_enable) the same program stops working:
console 1 $ ./bpf
console 2 $ echo "hello" | nc -u localhost 5000
console 1:
The reason is that both jits (x86 and powerpc) do not handle negative
memory references like SKF_NET_OFF or SKF_LL_OFF, only the simple
ancillary data references are supported (by mapping to special
instructions).
In the case of an absolute reference, the jit aborts the translation
if a negative reference is seen, also a negative k on the indirect
load aborts the translation, but if X is negative to begin with, only
the error handler is reached at runtime which drops the whole packet.
I propose the following patch to fix this situation.
Lightly tested on x86, but the powerpc asm part is prop. wrong.
Signed-of-by: Jan Seiffert <kaffeemonster@googlemail.com>
--
I know this is not a 100% submission, please refrain from shooting me,
i'm not really set up for professional kernel development, i just want
this fixed.
Greetings
Jan
[-- Attachment #2: bpf_neg.diff --]
[-- Type: application/octet-stream, Size: 10559 bytes --]
diff --git a/arch/powerpc/net/bpf_jit_64.S b/arch/powerpc/net/bpf_jit_64.S
index ff4506e..87dc6e4 100644
--- a/arch/powerpc/net/bpf_jit_64.S
+++ b/arch/powerpc/net/bpf_jit_64.S
@@ -31,14 +31,11 @@
* then branch directly to slow_path_XXX if required. (In fact, could
* load a spare GPR with the address of slow_path_generic and pass size
* as an argument, making the call site a mtlr, li and bllr.)
- *
- * Technically, the "is addr < 0" check is unnecessary & slowing down
- * the ABS path, as it's statically checked on generation.
*/
.globl sk_load_word
sk_load_word:
cmpdi r_addr, 0
- blt bpf_error
+ blt bpf_slow_path_neg_word
/* Are we accessing past headlen? */
subi r_scratch1, r_HL, 4
cmpd r_scratch1, r_addr
@@ -51,7 +48,7 @@ sk_load_word:
.globl sk_load_half
sk_load_half:
cmpdi r_addr, 0
- blt bpf_error
+ blt bpf_slow_path_neg_half
subi r_scratch1, r_HL, 2
cmpd r_scratch1, r_addr
blt bpf_slow_path_half
@@ -61,7 +58,7 @@ sk_load_half:
.globl sk_load_byte
sk_load_byte:
cmpdi r_addr, 0
- blt bpf_error
+ blt bpf_slow_path_neg_byte
cmpd r_HL, r_addr
ble bpf_slow_path_byte
lbzx r_A, r_D, r_addr
@@ -69,16 +66,22 @@ sk_load_byte:
/*
* BPF_S_LDX_B_MSH: ldxb 4*([offset]&0xf)
- * r_addr is the offset value, already known positive
+ * r_addr is the offset value
*/
.globl sk_load_byte_msh
sk_load_byte_msh:
+ cmpdi r_addr, 0
+ blt bpf_slow_path_neg_byte_msh
cmpd r_HL, r_addr
ble bpf_slow_path_byte_msh
lbzx r_X, r_D, r_addr
rlwinm r_X, r_X, 2, 32-4-2, 31-2
blr
+bpf_error_slow:
+ /* fabricate a cr0 = lt */
+ li r_scratch1, -1
+ cmpdi r_scratch1, 0
bpf_error:
/* Entered with cr0 = lt */
li r3, 0
@@ -136,3 +139,57 @@ bpf_slow_path_byte_msh:
lbz r_X, BPF_PPC_STACK_BASIC+(2*8)(r1)
rlwinm r_X, r_X, 2, 32-4-2, 31-2
blr
+
+/* Call out to bpf_internal_load_pointer_neg_helper:
+ * We'll need to back up our volatile regs first; we have
+ * local variable space at r1+(BPF_PPC_STACK_BASIC).
+ * Allocate a new stack frame here to remain ABI-compliant in
+ * stashing LR.
+ */
+#define bpf_slow_path_neg_common(SIZE) \
+ lis r_scratch1,-32; /* SKF_LL_OFF */ \
+ cmpd r_addr, r_scratch1; /* addr < SKF_* */ \
+ blt bpf_error; /* cr0 = LT */ \
+ mflr r0; \
+ std r0, 16(r1); \
+ /* R3 goes in parameter space of caller's frame */ \
+ std r_skb, (BPF_PPC_STACKFRAME+48)(r1); \
+ std r_A, (BPF_PPC_STACK_BASIC+(0*8))(r1); \
+ std r_X, (BPF_PPC_STACK_BASIC+(1*8))(r1); \
+ stdu r1, -BPF_PPC_SLOWPATH_FRAME(r1); \
+ /* R3 = r_skb, as passed */ \
+ mr r4, r_addr; \
+ li r5, SIZE; \
+ bl bpf_internal_load_pointer_neg_helper; \
+ /* R3 != 0 on success */ \
+ addi r1, r1, BPF_PPC_SLOWPATH_FRAME; \
+ ld r0, 16(r1); \
+ ld r_A, (BPF_PPC_STACK_BASIC+(0*8))(r1); \
+ ld r_X, (BPF_PPC_STACK_BASIC+(1*8))(r1); \
+ mtlr r0; \
+ cmpldi r3, 0; \
+ beq bpf_error_slow; /* cr0 = EQ */ \
+ mr r_addr, r3; \
+ ld r_skb, (BPF_PPC_STACKFRAME+48)(r1); \
+ /* Great success! */
+
+bpf_slow_path_neg_word:
+ bpf_slow_path_neg_common(4)
+ lwz r_A, (r_addr)
+ blr
+
+bpf_slow_path_neg_half:
+ bpf_slow_path_neg_common(2)
+ lhz r_A, (r_addr)
+ blr
+
+bpf_slow_path_neg_byte:
+ bpf_slow_path_neg_common(1)
+ lbz r_A, (r_addr)
+ blr
+
+bpf_slow_path_neg_byte_msh:
+ bpf_slow_path_neg_common(1)
+ lbz r_X, (r_addr)
+ rlwinm r_X, r_X, 2, 32-4-2, 31-2
+ blr
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 73619d3..0019f70 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -400,12 +400,10 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
func = sk_load_byte;
common_load:
/*
- * Load from [K]. Reference with the (negative)
- * SKF_NET_OFF/SKF_LL_OFF offsets is unsupported.
+ * Load from [K]. Negative offsets are tested for
+ * in the helper functions.
*/
ctx->seen |= SEEN_DATAREF;
- if ((int)K < 0)
- return -ENOTSUPP;
PPC_LI64(r_scratch1, func);
PPC_MTLR(r_scratch1);
PPC_LI32(r_addr, K);
@@ -429,7 +427,7 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
common_load_ind:
/*
* Load from [X + K]. Negative offsets are tested for
- * in the helper functions, and result in a 'ret 0'.
+ * in the helper functions.
*/
ctx->seen |= SEEN_DATAREF | SEEN_XREG;
PPC_LI64(r_scratch1, func);
@@ -443,12 +441,6 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
break;
case BPF_S_LDX_B_MSH:
- /*
- * x86 version drops packet (RET 0) when K<0, whereas
- * interpreter does allow K<0 (__load_pointer, special
- * ancillary data). common_load returns ENOTSUPP if K<0,
- * so we fall back to interpreter & filter works.
- */
func = sk_load_byte_msh;
goto common_load;
break;
diff --git a/arch/x86/net/bpf_jit.S b/arch/x86/net/bpf_jit.S
index 6687022..1201783 100644
--- a/arch/x86/net/bpf_jit.S
+++ b/arch/x86/net/bpf_jit.S
@@ -18,17 +18,13 @@
* r9d : hlen = skb->len - skb->data_len
*/
#define SKBDATA %r8
-
-sk_load_word_ind:
- .globl sk_load_word_ind
-
- add %ebx,%esi /* offset += X */
-# test %esi,%esi /* if (offset < 0) goto bpf_error; */
- js bpf_error
+#define SKF_MAX_NEG_OFF $(-0x200000) /* SKF_LL_OFF from filter.h */
sk_load_word:
.globl sk_load_word
+ test %esi,%esi
+ js bpf_slow_path_word_neg
mov %r9d,%eax # hlen
sub %esi,%eax # hlen - offset
cmp $3,%eax
@@ -37,16 +33,11 @@ sk_load_word:
bswap %eax /* ntohl() */
ret
-
-sk_load_half_ind:
- .globl sk_load_half_ind
-
- add %ebx,%esi /* offset += X */
- js bpf_error
-
sk_load_half:
.globl sk_load_half
+ test %esi,%esi
+ js bpf_slow_path_half_neg
mov %r9d,%eax
sub %esi,%eax # hlen - offset
cmp $1,%eax
@@ -55,14 +46,11 @@ sk_load_half:
rol $8,%ax # ntohs()
ret
-sk_load_byte_ind:
- .globl sk_load_byte_ind
- add %ebx,%esi /* offset += X */
- js bpf_error
-
sk_load_byte:
.globl sk_load_byte
+ test %esi,%esi
+ js bpf_slow_path_byte_neg
cmp %esi,%r9d /* if (offset >= hlen) goto bpf_slow_path_byte */
jle bpf_slow_path_byte
movzbl (SKBDATA,%rsi),%eax
@@ -73,10 +61,12 @@ sk_load_byte:
*
* Implements BPF_S_LDX_B_MSH : ldxb 4*([offset]&0xf)
* Must preserve A accumulator (%eax)
- * Inputs : %esi is the offset value, already known positive
+ * Inputs : %esi is the offset value
*/
ENTRY(sk_load_byte_msh)
CFI_STARTPROC
+ test %esi,%esi
+ js bpf_slow_path_byte_msh_neg
cmp %esi,%r9d /* if (offset >= hlen) goto bpf_slow_path_byte_msh */
jle bpf_slow_path_byte_msh
movzbl (SKBDATA,%rsi),%ebx
@@ -138,3 +128,45 @@ bpf_slow_path_byte_msh:
shl $2,%al
xchg %eax,%ebx
ret
+
+#define bpf_slow_path_neg_common(SIZE) \
+ cmp SKF_MAX_NEG_OFF, %esi; /* test range */ \
+ jl bpf_error; /* offset lower -> error */ \
+ push %rdi; /* save skb */ \
+ push %r9; \
+ push SKBDATA; \
+/* rsi already has offset */ \
+ mov $SIZE,%ecx; /* size */ \
+ call bpf_internal_load_pointer_neg_helper; \
+ test %rax,%rax; \
+ pop SKBDATA; \
+ pop %r9; \
+ pop %rdi; \
+ jz bpf_error
+
+bpf_slow_path_word_neg:
+ bpf_slow_path_neg_common(4)
+ mov (%rax), %eax
+ bswap %eax
+ ret
+
+bpf_slow_path_half_neg:
+ bpf_slow_path_neg_common(2)
+ mov (%rax),%ax
+ rol $8,%ax
+ movzwl %ax,%eax
+ ret
+
+bpf_slow_path_byte_neg:
+ bpf_slow_path_neg_common(1)
+ movzbl (%rax), %eax
+ ret
+
+bpf_slow_path_byte_msh_neg:
+ xchg %eax,%ebx /* dont lose A , X is about to be scratched */
+ bpf_slow_path_neg_common(1)
+ movzbl (%rax),%eax
+ and $15,%al
+ shl $2,%al
+ xchg %eax,%ebx
+ ret
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 5671752..3c4a454 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -30,7 +30,6 @@ int bpf_jit_enable __read_mostly;
* assembly code in arch/x86/net/bpf_jit.S
*/
extern u8 sk_load_word[], sk_load_half[], sk_load_byte[], sk_load_byte_msh[];
-extern u8 sk_load_word_ind[], sk_load_half_ind[], sk_load_byte_ind[];
static inline u8 *emit_code(u8 *ptr, u32 bytes, unsigned int len)
{
@@ -475,10 +474,6 @@ void bpf_jit_compile(struct sk_filter *fp)
case BPF_S_LD_W_ABS:
func = sk_load_word;
common_load: seen |= SEEN_DATAREF;
- if ((int)K < 0) {
- /* Abort the JIT because __load_pointer() is needed. */
- goto out;
- }
t_offset = func - (image + addrs[i]);
EMIT1_off32(0xbe, K); /* mov imm32,%esi */
EMIT1_off32(0xe8, t_offset); /* call */
@@ -490,27 +485,28 @@ common_load: seen |= SEEN_DATAREF;
func = sk_load_byte;
goto common_load;
case BPF_S_LDX_B_MSH:
- if ((int)K < 0) {
- /* Abort the JIT because __load_pointer() is needed. */
- goto out;
- }
seen |= SEEN_DATAREF | SEEN_XREG;
t_offset = sk_load_byte_msh - (image + addrs[i]);
EMIT1_off32(0xbe, K); /* mov imm32,%esi */
EMIT1_off32(0xe8, t_offset); /* call sk_load_byte_msh */
break;
case BPF_S_LD_W_IND:
- func = sk_load_word_ind;
+ func = sk_load_word;
common_load_ind: seen |= SEEN_DATAREF | SEEN_XREG;
t_offset = func - (image + addrs[i]);
- EMIT1_off32(0xbe, K); /* mov imm32,%esi */
+ if (K) {
+ EMIT2(0x8d, 0xb3); /* lea imm32(%rbx),%esi */
+ EMIT(K, 4);
+ } else {
+ EMIT2(0x89,0xde); /* mov %ebx,%esi */
+ }
EMIT1_off32(0xe8, t_offset); /* call sk_load_xxx_ind */
break;
case BPF_S_LD_H_IND:
- func = sk_load_half_ind;
+ func = sk_load_half;
goto common_load_ind;
case BPF_S_LD_B_IND:
- func = sk_load_byte_ind;
+ func = sk_load_byte;
goto common_load_ind;
case BPF_S_JMP_JA:
t_offset = addrs[i + K] - addrs[i];
diff --git a/net/core/filter.c b/net/core/filter.c
index 5dea452..04ca613 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -41,7 +41,7 @@
#include <linux/ratelimit.h>
/* No hurry in this branch */
-static void *__load_pointer(const struct sk_buff *skb, int k, unsigned int size)
+void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, unsigned int size)
{
u8 *ptr = NULL;
@@ -54,13 +54,14 @@ static void *__load_pointer(const struct sk_buff *skb, int k, unsigned int size)
return ptr;
return NULL;
}
+EXPORT_SYMBOL(bpf_internal_load_pointer_neg_helper);
static inline void *load_pointer(const struct sk_buff *skb, int k,
unsigned int size, void *buffer)
{
if (k >= 0)
return skb_header_pointer(skb, k, size, buffer);
- return __load_pointer(skb, k, size);
+ return bpf_internal_load_pointer_neg_helper(skb, k, size);
}
/**
^ permalink raw reply related
* Re: [PATCH 1/1] net/hyperv: Add flow control based on hi/low watermark
From: Ben Hutchings @ 2012-03-28 18:05 UTC (permalink / raw)
To: David Miller; +Cc: gregkh, haiyangz, netdev, devel, olaf, linux-kernel
In-Reply-To: <20120326.191237.2192763107267474232.davem@davemloft.net>
On Mon, 2012-03-26 at 19:12 -0400, David Miller wrote:
> From: Greg KH <gregkh@linuxfoundation.org>
> Date: Mon, 26 Mar 2012 16:10:17 -0700
>
> > David, please do NOT apply this as-is.
>
> BTW, ethtool had controls exactly for stuff like this.
Not sure what you're thinking of...? We have pause frame control but I
don't think that's applicable.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 2/3] tcp: Initial repair mode
From: Glauber Costa @ 2012-03-28 17:20 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: Linux Netdev List, David Miller
In-Reply-To: <4F73302C.9030209@parallels.com>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 9e7f9ba..65ae921 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1935,7 +1935,9 @@ void tcp_close(struct sock *sk, long timeout)
> * advertise a zero window, then kill -9 the FTP client, wheee...
> * Note: timeout is always zero in such a case.
> */
> - if (data_was_unread) {
> + if (tcp_sk(sk)->repair) {
> + sk->sk_prot->disconnect(sk, 0);
> + } else 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);
> @@ -2074,6 +2076,8 @@ int tcp_disconnect(struct sock *sk, int flags)
> /* ABORT function of RFC793 */
> if (old_state == TCP_LISTEN) {
> inet_csk_listen_stop(sk);
> + } else if (unlikely(tp->repair)) {
> + sk->sk_err = ECONNABORTED;
> } else if (tcp_need_reset(old_state) ||
> (tp->snd_nxt != tp->write_seq&&
> (1<< old_state)& (TCPF_CLOSING | TCPF_LAST_ACK))) {
The patch looks good in general.
Single nitpick is that maybe you should be consistent in your use of
unlikely. All of them seems equally unlikely, so I'd say you should wrap
both.
>
> + case TCP_REPAIR:
> + if (!tcp_can_repair_sock(sk))
> + err = -EPERM;
> + else if (val == 1) {
> + tp->repair = 1;
> + sk->sk_reuse = 2;
> + tp->repair_queue = TCP_NO_QUEUE;
> + } else if (val == 0) {
> + tp->repair = 0;
> + sk->sk_reuse = 0;
> + tcp_send_window_probe(sk);
> + } else
> + err = -EINVAL;
> +
> + break;
> +
> + case TCP_REPAIR_QUEUE:
Don't we need to test tcp_can_repair_sock() in all of them?
I understand that TCP_REPAIR always comes before the other ones,
so that means the socket is already in repair mode. But what
should be the behavior in case the process drops privileges?
Should it still be able to continue with the repair?
My first impression is that we need CAP_NET_ADMIN all along, so we
should make sure it's there.
^ permalink raw reply
* Re: [PATCH] net: reference the ipv4 sysctl table header
From: Steven Rostedt @ 2012-03-28 16:32 UTC (permalink / raw)
To: David Miller; +Cc: tixxdz, ebiederm, viro, netdev
In-Reply-To: <20120326.182411.41401140080225401.davem@davemloft.net>
On Mon, Mar 26, 2012 at 06:24:11PM -0400, David Miller wrote:
> From: Djalal Harouni <tixxdz@opendz.org>
> Date: Mon, 26 Mar 2012 23:23:59 +0100
>
> > +static struct ctl_table_header *ip4_base;
> > +
> > /*
> > * We really need to sanitize the damn ipv4 init order, then all
> > * this nonsense will go away.
> > */
> > -void __init ip_static_sysctl_init(void)
> > +int __init ip_static_sysctl_init(void)
> > {
> > - register_sysctl_paths(ipv4_path, ipv4_skeleton);
> > + ip4_base = register_sysctl_paths(ipv4_path, ipv4_skeleton);
>
> This is so incredibly stupid, just panic() or similar if this
> returns NULL.
>
> And find another way to annotate this for memleak so we don't need to
> waste an entire pointer, which is never used, in the data section.
I just started using kmemleak and notice that it reports false positives
for several __init functions that call register_sysctl_paths(). The fix
you want is:
{
sturct ctl_table_header *head;
head = register_sysctl_paths(ipv4_path, ipv4_skeleton);
BUG_ON(!head);
kmemleak_ignore(head);
No need to waste a pointer just to keep the reference around for
kmemleak.
-- Steve
^ permalink raw reply
* Re: [RFC PATCH] macvlan: add FDB bridge ops
From: John Fastabend @ 2012-03-28 15:58 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Roopa Prabhu, netdev
In-Reply-To: <20120328155227.GD20176@redhat.com>
On 3/28/2012 8:52 AM, Michael S. Tsirkin wrote:
> On Wed, Mar 28, 2012 at 08:43:56AM -0700, Roopa Prabhu wrote:
>> On 3/20/12 5:26 PM, "John Fastabend" <john.r.fastabend@intel.com> wrote:
>>
>>> Add support to add/del and dump the forwarding database
>>> for macvlan passthru mode. The macvlan driver acts like
>>> a Two Port Mac Relay (TPMR 802.1Q-2011) in the passthru
>>> case so adding forwarding rules is just adding the addr
>>> to the uc or mc lists.
>>>
>>> By default the passthru mode puts the lowerdev into a
>>> promiscuous mode to receive all packets. This behavior
>>> is not changed by this patch. This is a bit problematic
>>> and needs to be solved without IMHO breaking existing
>>> mechanics. Maybe on the first add_fdb we can decrement
>>> the promisc mode? That seems to work reasonable well and
>>> keep existing functionality in place... but requires
>>> an initial add to set things up which is a bit annoying
>>> so maybe a flag is better. I haven't thought too hard
>>> about it yet so any ideas welcome
>
>
> ...
>
>> Thanks John. Looks good.
>> I added a few things to your patch below. Yes, I think the promisc check is
>> required. Made an attempt to add a flag below (I did not get a chance to
>> think about other approaches there too). Briefly tested it with the br
>> command.
>>
>>
>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>> index f975afd..9bc70ad 100644
>> --- a/drivers/net/macvlan.c
>> +++ b/drivers/net/macvlan.c
>> @@ -34,6 +34,9 @@
>>
>> #define MACVLAN_HASH_SIZE (1 << BITS_PER_BYTE)
>>
>> +/* macvlan port flags */
>> +#define MACVLAN_FLAG_PROMISC 0x1
>> +
>> struct macvlan_port {
>> struct net_device *dev;
>> struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
>> @@ -41,6 +44,7 @@ struct macvlan_port {
>> struct rcu_head rcu;
>> bool passthru;
>> int count;
>> + unsigned int flags;
>> };
>>
>> static void macvlan_port_destroy(struct net_device *dev);
>> @@ -313,6 +317,7 @@ static int macvlan_open(struct net_device *dev)
>>
>> if (vlan->port->passthru) {
>> dev_set_promiscuity(lowerdev, 1);
>> + vlan->port->flags |= MACVLAN_FLAG_PROMISC;
>> goto hash_add;
>> }
>>
>> @@ -345,10 +350,14 @@ static int macvlan_stop(struct net_device *dev)
>> struct net_device *lowerdev = vlan->lowerdev;
>>
>> if (vlan->port->passthru) {
>> - dev_set_promiscuity(lowerdev, -1);
>> + if (vlan->port->flags & MACVLAN_FLAG_PROMISC) {
>> + dev_set_promiscuity(lowerdev, -1);
>> + vlan->port->flags &= ~MACVLAN_FLAG_PROMISC;
>> + }
>> goto hash_del;
>> }
>>
>> + dev_uc_unsync(lowerdev, dev);
>> dev_mc_unsync(lowerdev, dev);
>> if (dev->flags & IFF_ALLMULTI)
>> dev_set_allmulti(lowerdev, -1);
>> @@ -403,6 +412,7 @@ static void macvlan_set_multicast_list(struct net_device
>> *dev)
>> {
>> struct macvlan_dev *vlan = netdev_priv(dev);
>>
>> + dev_uc_sync(vlan->lowerdev, dev);
>> dev_mc_sync(vlan->lowerdev, dev);
>> }
>>
>> @@ -542,6 +552,58 @@ static int macvlan_vlan_rx_kill_vid(struct net_device
>> *dev,
>> return 0;
>> }
>>
>> +static int macvlan_fdb_add(struct ndmsg *ndm,
>> + struct net_device *dev,
>> + unsigned char *addr,
>> + u16 flags)
>> +{
>> + struct macvlan_dev *vlan = netdev_priv(dev);
>> + struct net_device *lowerdev = vlan->lowerdev;
>> + const struct net_device_ops *ops = lowerdev->netdev_ops;
>> + int err = -EINVAL;
>> +
>> + if (!vlan->port->passthru)
>> + return -EOPNOTSUPP;
>> +
>> + if (vlan->port->flags & MACVLAN_FLAG_PROMISC) {
>> + dev_set_promiscuity (lowerdev, -1);
>> + vlan->port->flags &= ~MACVLAN_FLAG_PROMISC;
>> + }
>> +
>> + if (ops->ndo_fdb_add)
>> + return ops->ndo_fdb_add(ndm, lowerdev, addr, flags);
>> +
>> + if (is_unicast_ether_addr(addr))
>> + err = dev_uc_add_excl(lowerdev, addr);
>> + else if (is_multicast_ether_addr(addr))
>> + err = dev_mc_add(lowerdev, addr);
>> +
>> + return err;
>> +}
>> +
>> +static int macvlan_fdb_del(struct ndmsg *ndm,
>> + struct net_device *dev,
>> + unsigned char *addr)
>> +{
>> + struct macvlan_dev *vlan = netdev_priv(dev);
>> + struct net_device *lowerdev = vlan->lowerdev;
>> + const struct net_device_ops *ops = lowerdev->netdev_ops;
>> + int err = -EINVAL;
>> +
>> + if (!vlan->port->passthru)
>> + return -EOPNOTSUPP;
>> +
>> + if (ops->ndo_fdb_del)
>> + return ops->ndo_fdb_del(ndm, lowerdev, addr);
>> +
>> + if (is_unicast_ether_addr(addr))
>> + err = dev_uc_del(lowerdev, addr);
>> + else if (is_multicast_ether_addr(addr))
>> + err = dev_mc_del(lowerdev, addr);
>> +
>> + return err;
>> +}
>> +
>> static void macvlan_ethtool_get_drvinfo(struct net_device *dev,
>> struct ethtool_drvinfo *drvinfo)
>> {
>> @@ -577,6 +639,9 @@ static const struct net_device_ops macvlan_netdev_ops =
>> {
>> .ndo_validate_addr = eth_validate_addr,
>> .ndo_vlan_rx_add_vid = macvlan_vlan_rx_add_vid,
>> .ndo_vlan_rx_kill_vid = macvlan_vlan_rx_kill_vid,
>> + .ndo_fdb_add = macvlan_fdb_add,
>> + .ndo_fdb_del = macvlan_fdb_del,
>> + .ndo_fdb_dump = ndo_dflt_fdb_dump,
>> };
>>
>> void macvlan_common_setup(struct net_device *dev)
>>
>
>
> So this clears the promisc on the first add which
> is a bit annoying. How about a simple flag, set when
> we create the macvlan?
>
Agreed. This probably needs a new attrib maybe IFLA_MACVLAN_FLAGS unless
there already exists a per "kind" (rtnl_link_ops) flags field we can use.
I scanned the code briefly and didn't see any such thing so likely we need
the new attribute.
.John
^ permalink raw reply
* Re: [RFC PATCH] macvlan: add FDB bridge ops
From: Michael S. Tsirkin @ 2012-03-28 15:52 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: John Fastabend, netdev
In-Reply-To: <CB987FCC.4A654%roprabhu@cisco.com>
On Wed, Mar 28, 2012 at 08:43:56AM -0700, Roopa Prabhu wrote:
> On 3/20/12 5:26 PM, "John Fastabend" <john.r.fastabend@intel.com> wrote:
>
> > Add support to add/del and dump the forwarding database
> > for macvlan passthru mode. The macvlan driver acts like
> > a Two Port Mac Relay (TPMR 802.1Q-2011) in the passthru
> > case so adding forwarding rules is just adding the addr
> > to the uc or mc lists.
> >
> > By default the passthru mode puts the lowerdev into a
> > promiscuous mode to receive all packets. This behavior
> > is not changed by this patch. This is a bit problematic
> > and needs to be solved without IMHO breaking existing
> > mechanics. Maybe on the first add_fdb we can decrement
> > the promisc mode? That seems to work reasonable well and
> > keep existing functionality in place... but requires
> > an initial add to set things up which is a bit annoying
> > so maybe a flag is better. I haven't thought too hard
> > about it yet so any ideas welcome
...
> Thanks John. Looks good.
> I added a few things to your patch below. Yes, I think the promisc check is
> required. Made an attempt to add a flag below (I did not get a chance to
> think about other approaches there too). Briefly tested it with the br
> command.
>
>
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index f975afd..9bc70ad 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -34,6 +34,9 @@
>
> #define MACVLAN_HASH_SIZE (1 << BITS_PER_BYTE)
>
> +/* macvlan port flags */
> +#define MACVLAN_FLAG_PROMISC 0x1
> +
> struct macvlan_port {
> struct net_device *dev;
> struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
> @@ -41,6 +44,7 @@ struct macvlan_port {
> struct rcu_head rcu;
> bool passthru;
> int count;
> + unsigned int flags;
> };
>
> static void macvlan_port_destroy(struct net_device *dev);
> @@ -313,6 +317,7 @@ static int macvlan_open(struct net_device *dev)
>
> if (vlan->port->passthru) {
> dev_set_promiscuity(lowerdev, 1);
> + vlan->port->flags |= MACVLAN_FLAG_PROMISC;
> goto hash_add;
> }
>
> @@ -345,10 +350,14 @@ static int macvlan_stop(struct net_device *dev)
> struct net_device *lowerdev = vlan->lowerdev;
>
> if (vlan->port->passthru) {
> - dev_set_promiscuity(lowerdev, -1);
> + if (vlan->port->flags & MACVLAN_FLAG_PROMISC) {
> + dev_set_promiscuity(lowerdev, -1);
> + vlan->port->flags &= ~MACVLAN_FLAG_PROMISC;
> + }
> goto hash_del;
> }
>
> + dev_uc_unsync(lowerdev, dev);
> dev_mc_unsync(lowerdev, dev);
> if (dev->flags & IFF_ALLMULTI)
> dev_set_allmulti(lowerdev, -1);
> @@ -403,6 +412,7 @@ static void macvlan_set_multicast_list(struct net_device
> *dev)
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
>
> + dev_uc_sync(vlan->lowerdev, dev);
> dev_mc_sync(vlan->lowerdev, dev);
> }
>
> @@ -542,6 +552,58 @@ static int macvlan_vlan_rx_kill_vid(struct net_device
> *dev,
> return 0;
> }
>
> +static int macvlan_fdb_add(struct ndmsg *ndm,
> + struct net_device *dev,
> + unsigned char *addr,
> + u16 flags)
> +{
> + struct macvlan_dev *vlan = netdev_priv(dev);
> + struct net_device *lowerdev = vlan->lowerdev;
> + const struct net_device_ops *ops = lowerdev->netdev_ops;
> + int err = -EINVAL;
> +
> + if (!vlan->port->passthru)
> + return -EOPNOTSUPP;
> +
> + if (vlan->port->flags & MACVLAN_FLAG_PROMISC) {
> + dev_set_promiscuity (lowerdev, -1);
> + vlan->port->flags &= ~MACVLAN_FLAG_PROMISC;
> + }
> +
> + if (ops->ndo_fdb_add)
> + return ops->ndo_fdb_add(ndm, lowerdev, addr, flags);
> +
> + if (is_unicast_ether_addr(addr))
> + err = dev_uc_add_excl(lowerdev, addr);
> + else if (is_multicast_ether_addr(addr))
> + err = dev_mc_add(lowerdev, addr);
> +
> + return err;
> +}
> +
> +static int macvlan_fdb_del(struct ndmsg *ndm,
> + struct net_device *dev,
> + unsigned char *addr)
> +{
> + struct macvlan_dev *vlan = netdev_priv(dev);
> + struct net_device *lowerdev = vlan->lowerdev;
> + const struct net_device_ops *ops = lowerdev->netdev_ops;
> + int err = -EINVAL;
> +
> + if (!vlan->port->passthru)
> + return -EOPNOTSUPP;
> +
> + if (ops->ndo_fdb_del)
> + return ops->ndo_fdb_del(ndm, lowerdev, addr);
> +
> + if (is_unicast_ether_addr(addr))
> + err = dev_uc_del(lowerdev, addr);
> + else if (is_multicast_ether_addr(addr))
> + err = dev_mc_del(lowerdev, addr);
> +
> + return err;
> +}
> +
> static void macvlan_ethtool_get_drvinfo(struct net_device *dev,
> struct ethtool_drvinfo *drvinfo)
> {
> @@ -577,6 +639,9 @@ static const struct net_device_ops macvlan_netdev_ops =
> {
> .ndo_validate_addr = eth_validate_addr,
> .ndo_vlan_rx_add_vid = macvlan_vlan_rx_add_vid,
> .ndo_vlan_rx_kill_vid = macvlan_vlan_rx_kill_vid,
> + .ndo_fdb_add = macvlan_fdb_add,
> + .ndo_fdb_del = macvlan_fdb_del,
> + .ndo_fdb_dump = ndo_dflt_fdb_dump,
> };
>
> void macvlan_common_setup(struct net_device *dev)
>
So this clears the promisc on the first add which
is a bit annoying. How about a simple flag, set when
we create the macvlan?
--
MST
^ permalink raw reply
* Re: [PATCH linux-next] dmaengine: add context parameter fixups
From: Mark Brown @ 2012-03-28 15:51 UTC (permalink / raw)
To: Takashi Iwai
Cc: fabio.estevam, Vinod Koul, samuel, alsa-devel, paul.gortmaker,
netdev, linux-kernel, Alexandre Bounine, linux-next,
dan.j.williams, Fabio Estevam, lrg
In-Reply-To: <s5hwr64wx0p.wl%tiwai@suse.de>
[-- Attachment #1.1: Type: text/plain, Size: 540 bytes --]
On Wed, Mar 28, 2012 at 05:44:22PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:
> > No, not yet - it was only sent after the merge window. Quite why nobody
> > managed to notice it before then I don't know. It'll go to him soon, or
> > at least to Takashi, but don't know if it'll make -rc1 or not.
> How is the situation now?
> FYI, I'm going to send a pull request to Linus tomorrow or on Friday.
> If anything needed to be merged in rc1, it must be there in time.
It's in my tree, I didn't see an enormous rush to get it in TBH.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH linux-next] dmaengine: add context parameter fixups
From: Takashi Iwai @ 2012-03-28 15:44 UTC (permalink / raw)
To: Mark Brown
Cc: Vinod Koul, Fabio Estevam, Alexandre Bounine, paul.gortmaker,
dan.j.williams, samuel, lrg, perex, linux-kernel, linux-next,
netdev, fabio.estevam, alsa-devel
In-Reply-To: <20120326094200.GA3098@opensource.wolfsonmicro.com>
At Mon, 26 Mar 2012 10:42:01 +0100,
Mark Brown wrote:
>
> On Mon, Mar 26, 2012 at 11:04:42AM +0530, Vinod Koul wrote:
> > On Sat, 2012-03-24 at 12:23 -0300, Fabio Estevam wrote:
>
> > > http://permalink.gmane.org/gmane.linux.alsa.devel/96215
>
> > And this seems to be linux-next.
>
> > Mark, Is this part of the sound updates that went to Linus?
>
> No, not yet - it was only sent after the merge window. Quite why nobody
> managed to notice it before then I don't know. It'll go to him soon, or
> at least to Takashi, but don't know if it'll make -rc1 or not.
How is the situation now?
FYI, I'm going to send a pull request to Linus tomorrow or on Friday.
If anything needed to be merged in rc1, it must be there in time.
thanks,
Takashi
^ permalink raw reply
* Re: [RFC PATCH] macvlan: add FDB bridge ops
From: Roopa Prabhu @ 2012-03-28 15:43 UTC (permalink / raw)
To: John Fastabend; +Cc: netdev, mst
In-Reply-To: <20120321002647.16118.87199.stgit@jf-dev1-dcblab>
On 3/20/12 5:26 PM, "John Fastabend" <john.r.fastabend@intel.com> wrote:
> Add support to add/del and dump the forwarding database
> for macvlan passthru mode. The macvlan driver acts like
> a Two Port Mac Relay (TPMR 802.1Q-2011) in the passthru
> case so adding forwarding rules is just adding the addr
> to the uc or mc lists.
>
> By default the passthru mode puts the lowerdev into a
> promiscuous mode to receive all packets. This behavior
> is not changed by this patch. This is a bit problematic
> and needs to be solved without IMHO breaking existing
> mechanics. Maybe on the first add_fdb we can decrement
> the promisc mode? That seems to work reasonable well and
> keep existing functionality in place... but requires
> an initial add to set things up which is a bit annoying
> so maybe a flag is better. I haven't thought too hard
> about it yet so any ideas welcome
>
> This patch is a result of Roopa Prabhu's work. Follow up
> patches are needed for VEPA and VEB macvlan modes.
>
> Only lightly touch tested at this point.
>
> CC: Roopa Prabhu <roprabhu@cisco.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>
> drivers/net/macvlan.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 42 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index f975afd..86af56b 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -349,6 +349,7 @@ static int macvlan_stop(struct net_device *dev)
> goto hash_del;
> }
>
> + dev_uc_unsync(lowerdev, dev);
> dev_mc_unsync(lowerdev, dev);
> if (dev->flags & IFF_ALLMULTI)
> dev_set_allmulti(lowerdev, -1);
> @@ -403,6 +404,7 @@ static void macvlan_set_multicast_list(struct net_device
> *dev)
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
>
> + dev_uc_sync(vlan->lowerdev, dev);
> dev_mc_sync(vlan->lowerdev, dev);
> }
>
> @@ -542,6 +544,43 @@ static int macvlan_vlan_rx_kill_vid(struct net_device
> *dev,
> return 0;
> }
>
> +static int macvlan_fdb_add(struct ndmsg *ndm,
> + struct net_device *dev,
> + unsigned char *addr,
> + u16 flags)
> +{
> + struct macvlan_dev *vlan = netdev_priv(dev);
> + int err = -EINVAL;
> +
> + if (!vlan->port->passthru)
> + return -EOPNOTSUPP;
> +
> + if (is_unicast_ether_addr(addr))
> + err = dev_uc_add(dev, addr);
> + else if (is_multicast_ether_addr(addr))
> + err = dev_mc_add(dev, addr);
> +
> + return err;
> +}
> +
> +static int macvlan_fdb_del(struct ndmsg *ndm,
> + struct net_device *dev,
> + unsigned char *addr)
> +{
> + struct macvlan_dev *vlan = netdev_priv(dev);
> + int err = -EINVAL;
> +
> + if (!vlan->port->passthru)
> + return -EOPNOTSUPP;
> +
> + if (is_unicast_ether_addr(addr))
> + err = dev_uc_del(dev, addr);
> + else if (is_multicast_ether_addr(addr))
> + err = dev_mc_del(dev, addr);
> +
> + return err;
> +}
> +
> static void macvlan_ethtool_get_drvinfo(struct net_device *dev,
> struct ethtool_drvinfo *drvinfo)
> {
> @@ -577,6 +616,9 @@ static const struct net_device_ops macvlan_netdev_ops = {
> .ndo_validate_addr = eth_validate_addr,
> .ndo_vlan_rx_add_vid = macvlan_vlan_rx_add_vid,
> .ndo_vlan_rx_kill_vid = macvlan_vlan_rx_kill_vid,
> + .ndo_fdb_add = macvlan_fdb_add,
> + .ndo_fdb_del = macvlan_fdb_del,
> + .ndo_fdb_dump = ndo_dflt_fdb_dump,
> };
>
> void macvlan_common_setup(struct net_device *dev)
>
Thanks John. Looks good.
I added a few things to your patch below. Yes, I think the promisc check is
required. Made an attempt to add a flag below (I did not get a chance to
think about other approaches there too). Briefly tested it with the br
command.
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index f975afd..9bc70ad 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -34,6 +34,9 @@
#define MACVLAN_HASH_SIZE (1 << BITS_PER_BYTE)
+/* macvlan port flags */
+#define MACVLAN_FLAG_PROMISC 0x1
+
struct macvlan_port {
struct net_device *dev;
struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
@@ -41,6 +44,7 @@ struct macvlan_port {
struct rcu_head rcu;
bool passthru;
int count;
+ unsigned int flags;
};
static void macvlan_port_destroy(struct net_device *dev);
@@ -313,6 +317,7 @@ static int macvlan_open(struct net_device *dev)
if (vlan->port->passthru) {
dev_set_promiscuity(lowerdev, 1);
+ vlan->port->flags |= MACVLAN_FLAG_PROMISC;
goto hash_add;
}
@@ -345,10 +350,14 @@ static int macvlan_stop(struct net_device *dev)
struct net_device *lowerdev = vlan->lowerdev;
if (vlan->port->passthru) {
- dev_set_promiscuity(lowerdev, -1);
+ if (vlan->port->flags & MACVLAN_FLAG_PROMISC) {
+ dev_set_promiscuity(lowerdev, -1);
+ vlan->port->flags &= ~MACVLAN_FLAG_PROMISC;
+ }
goto hash_del;
}
+ dev_uc_unsync(lowerdev, dev);
dev_mc_unsync(lowerdev, dev);
if (dev->flags & IFF_ALLMULTI)
dev_set_allmulti(lowerdev, -1);
@@ -403,6 +412,7 @@ static void macvlan_set_multicast_list(struct net_device
*dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
+ dev_uc_sync(vlan->lowerdev, dev);
dev_mc_sync(vlan->lowerdev, dev);
}
@@ -542,6 +552,58 @@ static int macvlan_vlan_rx_kill_vid(struct net_device
*dev,
return 0;
}
+static int macvlan_fdb_add(struct ndmsg *ndm,
+ struct net_device *dev,
+ unsigned char *addr,
+ u16 flags)
+{
+ struct macvlan_dev *vlan = netdev_priv(dev);
+ struct net_device *lowerdev = vlan->lowerdev;
+ const struct net_device_ops *ops = lowerdev->netdev_ops;
+ int err = -EINVAL;
+
+ if (!vlan->port->passthru)
+ return -EOPNOTSUPP;
+
+ if (vlan->port->flags & MACVLAN_FLAG_PROMISC) {
+ dev_set_promiscuity (lowerdev, -1);
+ vlan->port->flags &= ~MACVLAN_FLAG_PROMISC;
+ }
+
+ if (ops->ndo_fdb_add)
+ return ops->ndo_fdb_add(ndm, lowerdev, addr, flags);
+
+ if (is_unicast_ether_addr(addr))
+ err = dev_uc_add_excl(lowerdev, addr);
+ else if (is_multicast_ether_addr(addr))
+ err = dev_mc_add(lowerdev, addr);
+
+ return err;
+}
+
+static int macvlan_fdb_del(struct ndmsg *ndm,
+ struct net_device *dev,
+ unsigned char *addr)
+{
+ struct macvlan_dev *vlan = netdev_priv(dev);
+ struct net_device *lowerdev = vlan->lowerdev;
+ const struct net_device_ops *ops = lowerdev->netdev_ops;
+ int err = -EINVAL;
+
+ if (!vlan->port->passthru)
+ return -EOPNOTSUPP;
+
+ if (ops->ndo_fdb_del)
+ return ops->ndo_fdb_del(ndm, lowerdev, addr);
+
+ if (is_unicast_ether_addr(addr))
+ err = dev_uc_del(lowerdev, addr);
+ else if (is_multicast_ether_addr(addr))
+ err = dev_mc_del(lowerdev, addr);
+
+ return err;
+}
+
static void macvlan_ethtool_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo *drvinfo)
{
@@ -577,6 +639,9 @@ static const struct net_device_ops macvlan_netdev_ops =
{
.ndo_validate_addr = eth_validate_addr,
.ndo_vlan_rx_add_vid = macvlan_vlan_rx_add_vid,
.ndo_vlan_rx_kill_vid = macvlan_vlan_rx_kill_vid,
+ .ndo_fdb_add = macvlan_fdb_add,
+ .ndo_fdb_del = macvlan_fdb_del,
+ .ndo_fdb_dump = ndo_dflt_fdb_dump,
};
void macvlan_common_setup(struct net_device *dev)
^ permalink raw reply related
* [PATCH 3/3] tcp: Repair socket queues
From: Pavel Emelyanov @ 2012-03-28 15:38 UTC (permalink / raw)
To: Linux Netdev List, David Miller
In-Reply-To: <4F732FE1.9040906@parallels.com>
Reading queues under repair mode is done with recvmsg call.
The queue-under-repair set by TCP_REPAIR_QUEUE option is used
to determine which queue should be read. Thus both send and
receive queue can be read with this.
Caller must pass the MSG_PEEK flag.
Writing to queues is done with sendmsg call and yet again --
the repair-queue option can be used to push data into the
receive queue.
When putting an skb into receive queue a zero tcp header is
appented to its head to address the tcp_hdr(skb)->syn and
the ->fin checks by the (after repair) tcp_recvmsg. These
flags flags are both set to zero and that's why.
The fin cannot be met in the queue while reading the source
socket, since the repair only works for closed/established
sockets and queueing fin packet always changes its state.
The syn in the queue denotes that the respective skb's seq
is "off-by-one" as compared to the actual payload lenght. Thus,
at the rcv queue refill we can just drop this flag and set the
skb's sequences to precice values. IOW -- emulate the situation
when the packet with data and syn is splitted into two -- a
packet with syn and a packet with data and the former one is
already "eaten".
When the repair mode is turned off, the write queue seqs are
updated so that the whole queue is considered to be 'already sent,
waiting for ACKs' (write_seq = snd_nxt <= snd_una). From the
protocol POV the send queue looks like it was sent, but the data
between the write_seq and snd_nxt is lost in the network.
This helps to avoid another sockoption for setting the snd_nxt
sequence. Leaving the whole queue in a 'not yet sent' state (as
it will be after sendmsg-s) will not allow to receive any acks
from the peer since the ack_seq will be after the snd_nxt. Thus
even the ack for the window probe will be dropped and the
connection will be 'locked' with the zero peer window.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
net/ipv4/tcp.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++--
net/ipv4/tcp_output.c | 1 +
2 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 65ae921..2ab3a31 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -911,6 +911,39 @@ static inline int select_size(const struct sock *sk, bool sg)
return tmp;
}
+static int tcp_send_rcvq(struct sock *sk, struct msghdr *msg, size_t size)
+{
+ struct sk_buff *skb;
+ struct tcp_skb_cb *cb;
+ struct tcphdr *th;
+
+ skb = alloc_skb(size + sizeof(*th), sk->sk_allocation);
+ if (!skb)
+ goto err;
+
+ th = (struct tcphdr *)skb_put(skb, sizeof(*th));
+ skb_reset_transport_header(skb);
+ memset(th, 0, sizeof(*th));
+
+ if (memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size))
+ goto err_free;
+
+ cb = TCP_SKB_CB(skb);
+
+ TCP_SKB_CB(skb)->seq = tcp_sk(sk)->rcv_nxt;
+ TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(skb)->seq + size;
+ TCP_SKB_CB(skb)->ack_seq = tcp_sk(sk)->snd_una - 1;
+
+ tcp_queue_rcv(sk, skb, sizeof(*th));
+
+ return size;
+
+err_free:
+ kfree_skb(skb);
+err:
+ return -ENOMEM;
+}
+
int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
size_t size)
{
@@ -932,6 +965,19 @@ int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
if ((err = sk_stream_wait_connect(sk, &timeo)) != 0)
goto out_err;
+ if (unlikely(tp->repair)) {
+ if (tp->repair_queue == TCP_RECV_QUEUE) {
+ copied = tcp_send_rcvq(sk, msg, size);
+ goto out;
+ }
+
+ err = -EINVAL;
+ if (tp->repair_queue == TCP_NO_QUEUE)
+ goto out_err;
+
+ /* 'common' sending to sendq */
+ }
+
/* This should be in poll */
clear_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
@@ -1089,7 +1135,7 @@ new_segment:
if ((seglen -= copy) == 0 && iovlen == 0)
goto out;
- if (skb->len < max || (flags & MSG_OOB))
+ if (skb->len < max || (flags & MSG_OOB) || tp->repair)
continue;
if (forced_push(tp)) {
@@ -1102,7 +1148,7 @@ new_segment:
wait_for_sndbuf:
set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
wait_for_memory:
- if (copied)
+ if (copied && !tp->repair)
tcp_push(sk, flags & ~MSG_MORE, mss_now, TCP_NAGLE_PUSH);
if ((err = sk_stream_wait_memory(sk, &timeo)) != 0)
@@ -1113,7 +1159,7 @@ wait_for_memory:
}
out:
- if (copied)
+ if (copied && !tp->repair)
tcp_push(sk, flags, mss_now, tp->nonagle);
release_sock(sk);
return copied;
@@ -1187,6 +1233,24 @@ static int tcp_recv_urg(struct sock *sk, struct msghdr *msg, int len, int flags)
return -EAGAIN;
}
+static int tcp_peek_sndq(struct sock *sk, struct msghdr *msg, int len)
+{
+ struct sk_buff *skb;
+ int copied = 0, err = 0;
+
+ /* XXX -- need to support SO_PEEK_OFF */
+
+ skb_queue_walk(&sk->sk_write_queue, skb) {
+ err = skb_copy_datagram_iovec(skb, 0, msg->msg_iov, skb->len);
+ if (err)
+ break;
+
+ copied += skb->len;
+ }
+
+ return err ?: copied;
+}
+
/* Clean up the receive buffer for full frames taken by the user,
* then send an ACK if necessary. COPIED is the number of bytes
* tcp_recvmsg has given to the user so far, it speeds up the
@@ -1432,6 +1496,21 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
if (flags & MSG_OOB)
goto recv_urg;
+ if (unlikely(tp->repair)) {
+ err = -EPERM;
+ if (!(flags & MSG_PEEK))
+ goto out;
+
+ if (tp->repair_queue == TCP_SEND_QUEUE)
+ goto recv_sndq;
+
+ err = -EINVAL;
+ if (tp->repair_queue == TCP_NO_QUEUE)
+ goto out;
+
+ /* 'common' recv queue MSG_PEEK-ing */
+ }
+
seq = &tp->copied_seq;
if (flags & MSG_PEEK) {
peek_seq = tp->copied_seq;
@@ -1783,6 +1862,10 @@ out:
recv_urg:
err = tcp_recv_urg(sk, msg, len, flags);
goto out;
+
+recv_sndq:
+ err = tcp_peek_sndq(sk, msg, len);
+ goto out;
}
EXPORT_SYMBOL(tcp_recvmsg);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 4e2ce39..b29d612 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2796,6 +2796,7 @@ void tcp_send_window_probe(struct sock *sk)
{
if (sk->sk_state == TCP_ESTABLISHED) {
tcp_sk(sk)->snd_wl1 = tcp_sk(sk)->rcv_nxt - 1;
+ tcp_sk(sk)->snd_nxt = tcp_sk(sk)->write_seq;
tcp_xmit_probe_skb(sk, 0);
}
}
--
1.5.5.6
^ permalink raw reply related
* [PATCH 2/3] tcp: Initial repair mode
From: Pavel Emelyanov @ 2012-03-28 15:37 UTC (permalink / raw)
To: Linux Netdev List, David Miller
In-Reply-To: <4F732FE1.9040906@parallels.com>
This includes (according the the previous description):
* TCP_REPAIR sockoption
This one just puts the socket in/out of the repair mode.
Allowed for CAP_NET_ADMIN and for closed/establised sockets only.
When repair mode is turned off and the socket happens to be in
the established state the window probe is sent to the peer to
'unlock' the connection.
* TCP_REPAIR_QUEUE sockoption
This one sets the queue which we're about to repair. The
'no-queue' is set by default.
* TCP_QUEUE_SEQ socoption
Sets the write_seq/rcv_nxt of a selected repaired queue.
Allowed for TCP_CLOSE-d sockets only. When the socket changes
its state the other seq-s are changed by the kernel according
to the protocol rules (most of the existing code is actually
reused).
* Ability to forcibly bind a socket to a port
The sk->sk_reuse is set to 2 denoting, that the socket is question
should be bound as if all the others in the system are configured
with the SO_REUSEADDR option.
* Immediate connect modification
The connect syscall initializes the connection, then directly jumps
to the code which finalizes it.
* Silent close modification
The close just aborts the connection (similar to SO_LINGER with 0
time) but without sending any FIN/RST-s to peer.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
include/linux/tcp.h | 14 +++++++-
include/net/tcp.h | 2 +
net/ipv4/inet_connection_sock.c | 3 ++
net/ipv4/tcp.c | 68 ++++++++++++++++++++++++++++++++++++++-
net/ipv4/tcp_ipv4.c | 19 +++++++++--
net/ipv4/tcp_output.c | 16 +++++++--
6 files changed, 114 insertions(+), 8 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index b6c62d2..4e90e6a 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -106,6 +106,16 @@ enum {
#define TCP_THIN_LINEAR_TIMEOUTS 16 /* Use linear timeouts for thin streams*/
#define TCP_THIN_DUPACK 17 /* Fast retrans. after 1 dupack */
#define TCP_USER_TIMEOUT 18 /* How long for loss retry before timeout */
+#define TCP_REPAIR 19 /* TCP sock is under repair right now */
+#define TCP_REPAIR_QUEUE 20
+#define TCP_QUEUE_SEQ 21
+
+enum {
+ TCP_NO_QUEUE,
+ TCP_RECV_QUEUE,
+ TCP_SEND_QUEUE,
+ TCP_QUEUES_NR,
+};
/* for TCP_INFO socket option */
#define TCPI_OPT_TIMESTAMPS 1
@@ -353,7 +363,9 @@ struct tcp_sock {
u8 nonagle : 4,/* Disable Nagle algorithm? */
thin_lto : 1,/* Use linear timeouts for thin streams */
thin_dupack : 1,/* Fast retransmit on first dupack */
- unused : 2;
+ repair : 1,
+ unused : 1;
+ u8 repair_queue;
/* RTT measurement */
u32 srtt; /* smoothed round trip time << 3 */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index c7a00eb..f577052 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -612,6 +612,8 @@ static inline u32 tcp_receive_window(const struct tcp_sock *tp)
*/
extern u32 __tcp_select_window(struct sock *sk);
+void tcp_send_window_probe(struct sock *sk);
+
/* TCP timestamps are only 32-bits, this causes a slight
* complication on 64-bit systems since we store a snapshot
* of jiffies in the buffer control blocks below. We decided
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 19d66ce..92788af 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -172,6 +172,9 @@ have_snum:
goto tb_not_found;
tb_found:
if (!hlist_empty(&tb->owners)) {
+ if (sk->sk_reuse == 2)
+ goto success;
+
if (tb->fastreuse > 0 &&
sk->sk_reuse && sk->sk_state != TCP_LISTEN &&
smallest_size == -1) {
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9e7f9ba..65ae921 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1935,7 +1935,9 @@ void tcp_close(struct sock *sk, long timeout)
* advertise a zero window, then kill -9 the FTP client, wheee...
* Note: timeout is always zero in such a case.
*/
- if (data_was_unread) {
+ if (tcp_sk(sk)->repair) {
+ sk->sk_prot->disconnect(sk, 0);
+ } else 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);
@@ -2074,6 +2076,8 @@ int tcp_disconnect(struct sock *sk, int flags)
/* ABORT function of RFC793 */
if (old_state == TCP_LISTEN) {
inet_csk_listen_stop(sk);
+ } else if (unlikely(tp->repair)) {
+ sk->sk_err = ECONNABORTED;
} else if (tcp_need_reset(old_state) ||
(tp->snd_nxt != tp->write_seq &&
(1 << old_state) & (TCPF_CLOSING | TCPF_LAST_ACK))) {
@@ -2125,6 +2129,12 @@ int tcp_disconnect(struct sock *sk, int flags)
}
EXPORT_SYMBOL(tcp_disconnect);
+static inline int tcp_can_repair_sock(struct sock *sk)
+{
+ return capable(CAP_NET_ADMIN) &&
+ ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_ESTABLISHED));
+}
+
/*
* Socket option code for TCP.
*/
@@ -2297,6 +2307,42 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
tp->thin_dupack = val;
break;
+ case TCP_REPAIR:
+ if (!tcp_can_repair_sock(sk))
+ err = -EPERM;
+ else if (val == 1) {
+ tp->repair = 1;
+ sk->sk_reuse = 2;
+ tp->repair_queue = TCP_NO_QUEUE;
+ } else if (val == 0) {
+ tp->repair = 0;
+ sk->sk_reuse = 0;
+ tcp_send_window_probe(sk);
+ } else
+ err = -EINVAL;
+
+ break;
+
+ case TCP_REPAIR_QUEUE:
+ if (!tp->repair)
+ err = -EPERM;
+ else if (val <= TCP_QUEUES_NR)
+ tp->repair_queue = val;
+ else
+ err = -EINVAL;
+ break;
+
+ case TCP_QUEUE_SEQ:
+ if (sk->sk_state != TCP_CLOSE)
+ err = -EPERM;
+ else if (tp->repair_queue == TCP_SEND_QUEUE)
+ tp->write_seq = val;
+ else if (tp->repair_queue == TCP_RECV_QUEUE)
+ tp->rcv_nxt = val;
+ else
+ err = -EINVAL;
+ break;
+
case TCP_CORK:
/* When set indicates to always queue non-full frames.
* Later the user clears this option and we transmit
@@ -2632,6 +2678,26 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
val = tp->thin_dupack;
break;
+ case TCP_REPAIR:
+ val = tp->repair;
+ break;
+
+ case TCP_REPAIR_QUEUE:
+ if (tp->repair)
+ val = tp->repair_queue;
+ else
+ return -EINVAL;
+ break;
+
+ case TCP_QUEUE_SEQ:
+ if (tp->repair_queue == TCP_SEND_QUEUE)
+ val = tp->write_seq;
+ else if (tp->repair_queue == TCP_RECV_QUEUE)
+ val = tp->rcv_nxt;
+ else
+ return -EINVAL;
+ break;
+
case TCP_USER_TIMEOUT:
val = jiffies_to_msecs(icsk->icsk_user_timeout);
break;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3a25cf7..69190a9 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -138,6 +138,14 @@ int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp)
}
EXPORT_SYMBOL_GPL(tcp_twsk_unique);
+static int tcp_repair_connect(struct sock *sk)
+{
+ tcp_connect_init(sk);
+ tcp_finish_connect(sk, NULL);
+
+ return 0;
+}
+
/* This will initiate an outgoing connection. */
int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
{
@@ -196,7 +204,8 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
/* Reset inherited state */
tp->rx_opt.ts_recent = 0;
tp->rx_opt.ts_recent_stamp = 0;
- tp->write_seq = 0;
+ if (!tp->repair)
+ tp->write_seq = 0;
}
if (tcp_death_row.sysctl_tw_recycle &&
@@ -247,7 +256,7 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
sk->sk_gso_type = SKB_GSO_TCPV4;
sk_setup_caps(sk, &rt->dst);
- if (!tp->write_seq)
+ if (!tp->write_seq && !tp->repair)
tp->write_seq = secure_tcp_sequence_number(inet->inet_saddr,
inet->inet_daddr,
inet->inet_sport,
@@ -255,7 +264,11 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
inet->inet_id = tp->write_seq ^ jiffies;
- err = tcp_connect(sk);
+ if (likely(!tp->repair))
+ err = tcp_connect(sk);
+ else
+ err = tcp_repair_connect(sk);
+
rt = NULL;
if (err)
goto failure;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 1db25af..4e2ce39 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2617,9 +2617,11 @@ void tcp_connect_init(struct sock *sk)
tp->snd_sml = tp->write_seq;
tp->snd_up = tp->write_seq;
tp->snd_nxt = tp->write_seq;
- tp->rcv_nxt = 0;
- tp->rcv_wup = 0;
- tp->copied_seq = 0;
+
+ if (!tp->repair)
+ tp->rcv_nxt = 0;
+ tp->rcv_wup = tp->rcv_nxt;
+ tp->copied_seq = tp->rcv_nxt;
inet_csk(sk)->icsk_rto = TCP_TIMEOUT_INIT;
inet_csk(sk)->icsk_retransmits = 0;
@@ -2790,6 +2792,14 @@ static int tcp_xmit_probe_skb(struct sock *sk, int urgent)
return tcp_transmit_skb(sk, skb, 0, GFP_ATOMIC);
}
+void tcp_send_window_probe(struct sock *sk)
+{
+ if (sk->sk_state == TCP_ESTABLISHED) {
+ tcp_sk(sk)->snd_wl1 = tcp_sk(sk)->rcv_nxt - 1;
+ tcp_xmit_probe_skb(sk, 0);
+ }
+}
+
/* Initiate keepalive or window probe from timer. */
int tcp_write_wakeup(struct sock *sk)
{
--
1.5.5.6
^ permalink raw reply related
* [PATCH 1/3] tcp: Move code around
From: Pavel Emelyanov @ 2012-03-28 15:36 UTC (permalink / raw)
To: Linux Netdev List, David Miller
In-Reply-To: <4F732FE1.9040906@parallels.com>
This is just the preparation patch, which makes the needed for
TCP repair code ready for use.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
include/net/tcp.h | 3 ++
net/ipv4/tcp.c | 2 +-
net/ipv4/tcp_input.c | 81 +++++++++++++++++++++++++++++--------------------
net/ipv4/tcp_output.c | 4 +-
4 files changed, 54 insertions(+), 36 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index f75a04d..c7a00eb 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -435,6 +435,9 @@ extern struct sk_buff * tcp_make_synack(struct sock *sk, struct dst_entry *dst,
struct request_values *rvp);
extern int tcp_disconnect(struct sock *sk, int flags);
+void tcp_connect_init(struct sock *sk);
+void tcp_finish_connect(struct sock *sk, struct sk_buff *skb);
+void tcp_queue_rcv(struct sock *sk, struct sk_buff *skb, int hdrlen);
/* From syncookies.c */
extern __u32 syncookie_secret[2][16-4+SHA_DIGEST_WORDS];
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index cfd7edd..9e7f9ba 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -918,7 +918,7 @@ int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *skb;
int iovlen, flags, err, copied;
- int mss_now, size_goal;
+ int mss_now = 0, size_goal;
bool sg;
long timeo;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e886e2f..3767a1a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5322,6 +5322,14 @@ discard:
return 0;
}
+void tcp_queue_rcv(struct sock *sk, struct sk_buff *skb, int hdrlen)
+{
+ __skb_pull(skb, hdrlen);
+ __skb_queue_tail(&sk->sk_receive_queue, skb);
+ skb_set_owner_r(skb, sk);
+ tcp_sk(sk)->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
+}
+
/*
* TCP receive function for the ESTABLISHED state.
*
@@ -5487,10 +5495,7 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPHPHITS);
/* Bulk data transfer: receiver */
- __skb_pull(skb, tcp_header_len);
- __skb_queue_tail(&sk->sk_receive_queue, skb);
- skb_set_owner_r(skb, sk);
- tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
+ tcp_queue_rcv(sk, skb, tcp_header_len);
}
tcp_event_data_recv(sk, skb);
@@ -5556,6 +5561,44 @@ discard:
}
EXPORT_SYMBOL(tcp_rcv_established);
+void tcp_finish_connect(struct sock *sk, struct sk_buff *skb)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct inet_connection_sock *icsk = inet_csk(sk);
+
+ tcp_set_state(sk, TCP_ESTABLISHED);
+
+ if (skb != NULL)
+ security_inet_conn_established(sk, skb);
+
+ /* Make sure socket is routed, for correct metrics. */
+ icsk->icsk_af_ops->rebuild_header(sk);
+
+ tcp_init_metrics(sk);
+
+ tcp_init_congestion_control(sk);
+
+ /* Prevent spurious tcp_cwnd_restart() on first data
+ * packet.
+ */
+ tp->lsndtime = tcp_time_stamp;
+
+ tcp_init_buffer_space(sk);
+
+ if (sock_flag(sk, SOCK_KEEPOPEN))
+ inet_csk_reset_keepalive_timer(sk, keepalive_time_when(tp));
+
+ if (!tp->rx_opt.snd_wscale)
+ __tcp_fast_path_on(tp, tp->snd_wnd);
+ else
+ tp->pred_flags = 0;
+
+ if (!sock_flag(sk, SOCK_DEAD)) {
+ sk->sk_state_change(sk);
+ sk_wake_async(sk, SOCK_WAKE_IO, POLL_OUT);
+ }
+}
+
static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
const struct tcphdr *th, unsigned int len)
{
@@ -5688,36 +5731,8 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
}
smp_mb();
- tcp_set_state(sk, TCP_ESTABLISHED);
-
- security_inet_conn_established(sk, skb);
-
- /* Make sure socket is routed, for correct metrics. */
- icsk->icsk_af_ops->rebuild_header(sk);
-
- tcp_init_metrics(sk);
- tcp_init_congestion_control(sk);
-
- /* Prevent spurious tcp_cwnd_restart() on first data
- * packet.
- */
- tp->lsndtime = tcp_time_stamp;
-
- tcp_init_buffer_space(sk);
-
- if (sock_flag(sk, SOCK_KEEPOPEN))
- inet_csk_reset_keepalive_timer(sk, keepalive_time_when(tp));
-
- if (!tp->rx_opt.snd_wscale)
- __tcp_fast_path_on(tp, tp->snd_wnd);
- else
- tp->pred_flags = 0;
-
- if (!sock_flag(sk, SOCK_DEAD)) {
- sk->sk_state_change(sk);
- sk_wake_async(sk, SOCK_WAKE_IO, POLL_OUT);
- }
+ tcp_finish_connect(sk, skb);
if (sk->sk_write_pending ||
icsk->icsk_accept_queue.rskq_defer_accept ||
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 364784a..1db25af 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2561,7 +2561,7 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
EXPORT_SYMBOL(tcp_make_synack);
/* Do all connect socket setups that can be done AF independent. */
-static void tcp_connect_init(struct sock *sk)
+void tcp_connect_init(struct sock *sk)
{
const struct dst_entry *dst = __sk_dst_get(sk);
struct tcp_sock *tp = tcp_sk(sk);
@@ -2616,6 +2616,7 @@ static void tcp_connect_init(struct sock *sk)
tp->snd_una = tp->write_seq;
tp->snd_sml = tp->write_seq;
tp->snd_up = tp->write_seq;
+ tp->snd_nxt = tp->write_seq;
tp->rcv_nxt = 0;
tp->rcv_wup = 0;
tp->copied_seq = 0;
@@ -2641,7 +2642,6 @@ int tcp_connect(struct sock *sk)
/* Reserve space for headers. */
skb_reserve(buff, MAX_TCP_HEADER);
- tp->snd_nxt = tp->write_seq;
tcp_init_nondata_skb(buff, tp->write_seq++, TCPHDR_SYN);
TCP_ECN_send_syn(sk, buff);
--
1.5.5.6
^ permalink raw reply related
* [PATCH net-next 0/3] TCP connection repair (v3)
From: Pavel Emelyanov @ 2012-03-28 15:36 UTC (permalink / raw)
To: Linux Netdev List, David Miller
Hi!
Attempt #3 with transparent TCP connection hijacking
(previous one is here http://lists.openwall.net/netdev/2012/03/06/65).
Changes since v2:
* The CAP_NET_ADMIN is required to turn repair on, not CAP_SYS_ADMIN
* Changed read queue seq sockoption to work on the rcv_nxt, not the
copied_seq to address the issue with syn flag in the fake header
(see below).
* Resolved issues with syn and fin flags in fake headers.
Fin can and should be dropped. The repair mode is currently allowed
only for closed and established sockets and thus we cannot meet an
skb with this flag in the original socket (queuing fin to receive
queue switches the established state to the close-wait one).
Syn can also be dropped. This flag in the recv queue's skb means the
respective skb's seq is off-by-one relative to the actual amount of
data on it. Thus, removing the flag from fake skb and fixing the seq
respectively solves the issue.
However, in order to do so it's not enough to know the copied_seq and
recv queue length only (rcv_nxt should be copied_seq plus data length
plus "syn-is-there"). Thus, the rcv queue seq get/set sockoption is
changed to work on the rcv_nxt itself. IOW I emulate the situation
when the packet with data and syn is splitted into two -- a packet with
syn and a packet with data and the former one is already "eaten".
Thanks,
Pavel
^ permalink raw reply
* bridge + vlan + igmp snooping
From: Thomas Voegtle @ 2012-03-28 15:16 UTC (permalink / raw)
To: netdev
Hello,
I tried multicast in vlan on a bridge with BRIDGE_IGMP_SNOOPING
swtiched on and it didn't work.
Switching off snooping helps (echo 0 > /sys..) or switching off vlan.
Tested on latest 3.0 and 3.2.
Is there a known issue?
Thanks,
Thomas
^ permalink raw reply
* Re: [PATCH] tcp: bind() use stronger condition for bind_conflict
From: Eric Dumazet @ 2012-03-28 14:48 UTC (permalink / raw)
To: Flavio Leitner
Cc: Alexandru Copot, davem, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel, Daniel Baluta
In-Reply-To: <20120328105212.1cb8dfe5@asterix.rh>
On Wed, 2012-03-28 at 10:52 -0300, Flavio Leitner wrote:
> On Wed, 28 Mar 2012 00:11:52 +0300
> Alexandru Copot <alex.mihai.c@gmail.com> wrote:
>
> > diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
> > index 02dd203..dfc8bc3 100644
> > --- a/net/ipv6/inet6_connection_sock.c
> > +++ b/net/ipv6/inet6_connection_sock.c
> > @@ -28,7 +28,8 @@
> > #include <net/inet6_connection_sock.h>
> >
> > int inet6_csk_bind_conflict(const struct sock *sk,
> > - const struct inet_bind_bucket *tb)
> > + const struct inet_bind_bucket *tb,
> > + int relax)
>
> Same here
> Maybe you can wait for more feedbacks before spin another patch
> version just to fix that.
>
Also 'int relax' should be 'bool relax'... 1/0 -> true/false
Anyway net-next is not open yet...
^ permalink raw reply
* Question about timewait timer
From: Chao Pei @ 2012-03-28 14:21 UTC (permalink / raw)
To: netdev
Hi,
I have a qestion about timewait timer in tcp_keepalive_timer. In
the following code:
int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
const struct tcphdr *th, unsigned int len)
{
.....
case FIN_WAIT_1:
......
tmo = tcp_fin_time(sk);
if (tmo > TCP_TIMEWAIT_LEN) {
inet_csk_reset_keepalive_timer(sk, tmo - TCP_TIMEWAIT_LEN);
} else if (.....)
......
}
If tmo is larger than TCP_TIMEWAIT_LEN and the FIN flag is not
set, schedule keepalive timer. But in tcp_keepalive_timer, the timer
is set to tcp_fin_time(sk) -TCP_TIMEWAIT_LEN again. See the source
code below:
if (sk->sk_state == TCP_FIN_WAIT2 && sock_flag(sk, SOCK_DEAD)) {
if (tp->linger2 >= 0) {
const int tmo = tcp_fin_time(sk) - TCP_TIMEWAIT_LEN;
if (tmo > 0) {
tcp_time_wait(sk, TCP_FIN_WAIT2, tmo);
goto out;
}
}
.....
}
I think the first piece of code set keepalive timer to
tmo-TCP_TIMEWAIT_LEN is because TCP_TIMEWAIT_LEN is the longest timer
that tcp_death_row can handle. So the extra time should be taken care
of by the keepalive timer first. If this is correct, then the timer
should be set to TCP_TIMEWAIT_LEN in tcp_keepalive_timer, instead of
tcp_fin_time(sk) - TCP_TIMEWAIT_LEN.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] net/garp: avoid infinite loop if attribute already exists
From: Ward, David - 0663 - MITLL @ 2012-03-28 13:28 UTC (permalink / raw)
To: jorge@dti2.net, netdev@vger.kernel.org; +Cc: kaber@trash.net
In-Reply-To: <4F72F5D6.5080903@dti2.net>
[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]
On 28/03/12 07:28, Jorge Boncompte [DTI2] wrote:
> El 27/03/2012 21:01, David Ward escribió:
>> An infinite loop occurred if garp_attr_create was called with the values
>> of an existing attribute. This might happen if a previous leave request
>> for the attribute has not yet been followed by a PDU transmission (or,
>> if the application previously issued a join request for the attribute
>> and is now issuing another one, without having issued a leave request).
>>
>> If garp_attr_create finds an existing attribute having the same values,
>> return the address to it. Its state will then get updated (i.e., if it
>> was in a leaving state, it will move into a non-leaving state and not
>> get deleted during the next PDU transmission).
>>
>> To accomplish this fix, collapse garp_attr_insert into garp_attr_create
>> (which is its only caller).
>>
> I wouldn't mind if you credited me given that your patch now is based on my fix
> and analysis. ;-)
Please add to commit message:
Thanks to Jorge Boncompte [DTI2] <jorge@dti2.net> for contributing to
this fix.
> Anyway...
>
> Acked-by: Jorge Boncompte [DTI2]<jorge@dti2.net>
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4558 bytes --]
^ permalink raw reply
* Re: [PATCH] tcp: bind() use stronger condition for bind_conflict
From: Flavio Leitner @ 2012-03-28 13:52 UTC (permalink / raw)
To: Alexandru Copot
Cc: davem, eric.dumazet, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel, Daniel Baluta
In-Reply-To: <1332882712-10756-1-git-send-email-alex.mihai.c@gmail.com>
On Wed, 28 Mar 2012 00:11:52 +0300
Alexandru Copot <alex.mihai.c@gmail.com> wrote:
> We must try harder to get unique (addr, port) pairs when
> doing port autoselection for sockets with SO_REUSEADDR
> option set.
>
> We achieve this by adding a relaxation parameter to
> inet_csk_bind_conflict. When 'relax' parameter is off
> we return a conflict whenever the current searched
> pair (addr, port) is not unique.
>
> This tries to address the problems reported in patch:
> 8d238b25b1ec22a73b1c2206f111df2faaff8285
> Revert "tcp: bind() fix when many ports are bound"
>
> Signed-off-by: Alexandru Copot <alex.mihai.c@gmail.com>
> Signed-off-by: Daniel Baluta <dbaluta@ixiacom.com>
> ---
> include/net/inet6_connection_sock.h | 2 +-
> include/net/inet_connection_sock.h | 4 ++--
> net/ipv4/inet_connection_sock.c | 17 +++++++++++++----
> net/ipv6/inet6_connection_sock.c | 3 ++-
> 4 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/inet6_connection_sock.h b/include/net/inet6_connection_sock.h
> index 3207e58..9d29ae2 100644
> --- a/include/net/inet6_connection_sock.h
> +++ b/include/net/inet6_connection_sock.h
> @@ -23,7 +23,7 @@ struct sock;
> struct sockaddr;
>
> extern int inet6_csk_bind_conflict(const struct sock *sk,
> - const struct inet_bind_bucket *tb);
> + const struct inet_bind_bucket *tb, int relax);
>
> extern struct dst_entry* inet6_csk_route_req(struct sock *sk,
> const struct request_sock *req);
> diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
> index dbf9aab..f05a032 100644
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -60,7 +60,7 @@ struct inet_connection_sock_af_ops {
> #endif
> void (*addr2sockaddr)(struct sock *sk, struct sockaddr *);
> int (*bind_conflict)(const struct sock *sk,
> - const struct inet_bind_bucket *tb);
> + const struct inet_bind_bucket *tb, int relax);
> };
>
> /** inet_connection_sock - INET connection oriented sock
> @@ -245,7 +245,7 @@ extern struct request_sock *inet_csk_search_req(const struct sock *sk,
> const __be32 raddr,
> const __be32 laddr);
> extern int inet_csk_bind_conflict(const struct sock *sk,
> - const struct inet_bind_bucket *tb);
> + const struct inet_bind_bucket *tb, int relax);
> extern int inet_csk_get_port(struct sock *sk, unsigned short snum);
>
> extern struct dst_entry* inet_csk_route_req(struct sock *sk,
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 19d66ce..bf50e77 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -53,7 +53,7 @@ void inet_get_local_port_range(int *low, int *high)
> EXPORT_SYMBOL(inet_get_local_port_range);
>
> int inet_csk_bind_conflict(const struct sock *sk,
> - const struct inet_bind_bucket *tb)
> + const struct inet_bind_bucket *tb, int relax)
> {
> struct sock *sk2;
> struct hlist_node *node;
> @@ -79,6 +79,13 @@ int inet_csk_bind_conflict(const struct sock *sk,
> sk2_rcv_saddr == sk_rcv_saddr(sk))
> break;
> }
> + if (!relax && reuse && sk2->sk_reuse &&
> + sk2->sk_state != TCP_LISTEN) {
> + const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2);
> + if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
> + sk2_rcv_saddr == sk_rcv_saddr(sk))
I am still checking the patch, but the above is out of coding style.
perhaps:
+ if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
+ sk2_rcv_saddr == sk_rcv_saddr(sk))
> + break;
> + }
> }
> }
> return node != NULL;
> @@ -122,12 +129,13 @@ again:
> (tb->num_owners < smallest_size || smallest_size == -1)) {
> smallest_size = tb->num_owners;
> smallest_rover = rover;
> - if (atomic_read(&hashinfo->bsockets) > (high - low) + 1) {
> + if (atomic_read(&hashinfo->bsockets) > (high - low) + 1 &&
> + !inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, 0)) {
Same above.
> snum = smallest_rover;
> goto tb_found;
> }
> }
> - if (!inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
> + if (!inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, 0)) {
> snum = rover;
> goto tb_found;
> }
> @@ -178,12 +186,13 @@ tb_found:
> goto success;
> } else {
> ret = 1;
> - if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
> + if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, 1)) {
> if (sk->sk_reuse && sk->sk_state != TCP_LISTEN &&
> smallest_size != -1 && --attempts >= 0) {
> spin_unlock(&head->lock);
> goto again;
> }
> +
> goto fail_unlock;
> }
> }
> diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
> index 02dd203..dfc8bc3 100644
> --- a/net/ipv6/inet6_connection_sock.c
> +++ b/net/ipv6/inet6_connection_sock.c
> @@ -28,7 +28,8 @@
> #include <net/inet6_connection_sock.h>
>
> int inet6_csk_bind_conflict(const struct sock *sk,
> - const struct inet_bind_bucket *tb)
> + const struct inet_bind_bucket *tb,
> + int relax)
Same here
Maybe you can wait for more feedbacks before spin another patch
version just to fix that.
fbl
> {
> const struct sock *sk2;
> const struct hlist_node *node;
^ permalink raw reply
* Re: [PATCH] f_phonet: fix skb truesize underestimation
From: Felipe Balbi @ 2012-03-28 12:43 UTC (permalink / raw)
To: David Miller
Cc: balbi-l0cyMroinI0, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w
In-Reply-To: <20120328.053310.219141897175161116.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]
On Wed, Mar 28, 2012 at 05:33:10AM -0400, David Miller wrote:
> From: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> Date: Wed, 28 Mar 2012 11:55:36 +0300
>
> > On Tue, Mar 27, 2012 at 10:51:09PM -0400, David Miller wrote:
> >> From: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> Date: Tue, 27 Mar 2012 06:04:02 -0700
> >>
> >> > Now skb_add_rx_frag() has a truesize parameter, we can fix f_phonet to
> >> > properly account truesize of each fragment : a full page.
> >> >
> >> > Signed-off-by: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>
> >> Applied.
> >
> > did you apply both patches ?
>
> You never need to ask me these kinds of questions, you can
> either check my GIT tree or look at:
>
> http://patchwork.ozlabs.org/project/netdev/list/?state=3
I'm asking because drivers/usb/gadget/ should either go through me or
you should wait for my Acked-by. Now it's too late for that, but next
time for my Ack.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ 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