If I use sendfile() to send to a accept()ed AF_ALG socket set up for
"hash", I get the wrong
answer, if I read() and then write() I get the right answer. None of
the system calls return an error.
test case attached.
--
---
Shawn Landden
+1 360 389 3001 (SMS preferred)
heres a version of the test case that builds.....
Sorry about that.
On Sun, Nov 24, 2013 at 9:21 AM, Shawn Landden <[email protected]> wrote:
> If I use sendfile() to send to a accept()ed AF_ALG socket set up for
> "hash", I get the wrong
> answer, if I read() and then write() I get the right answer. None of
> the system calls return an error.
>
> test case attached.
>
> --
>
> ---
> Shawn Landden
> +1 360 389 3001 (SMS preferred)
--
---
Shawn Landden
+1 360 389 3001 (SMS preferred)
If you build https://kernel.googlesource.com/pub/scm/network/connman/connman/+/0.80/tools/alg-test.c
from the connman source code and compare the output to coreutils
sha1sum you can see the problem.
shawn@debian-T61:~/git/test$ make connman_afalg
cc connman_afalg.c -o connman_afalg
shawn@debian-T61:~/git/test$ ./connman_afalg /bin/true
send 27080 bytes
recv 20 bytes
45384483cf9cd0d82eba164131795b4807c6d39d /bin/true
shawn@debian-T61:~/git/test$ sha1sum /bin/true
82667ba2ec681d8e55b0ee3b6db2970f9911680d /bin/true
On Sun, Nov 24, 2013 at 2:00 PM, Shawn Landden <[email protected]> wrote:
> heres a version of the test case that builds.....
>
> Sorry about that.
>
> On Sun, Nov 24, 2013 at 9:21 AM, Shawn Landden <[email protected]> wrote:
>> If I use sendfile() to send to a accept()ed AF_ALG socket set up for
>> "hash", I get the wrong
>> answer, if I read() and then write() I get the right answer. None of
>> the system calls return an error.
>>
>> test case attached.
>>
>> --
>>
>> ---
>> Shawn Landden
>> +1 360 389 3001 (SMS preferred)
>
>
>
> --
>
> ---
> Shawn Landden
> +1 360 389 3001 (SMS preferred)
--
---
Shawn Landden
+1 360 389 3001 (SMS preferred)
Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
added an internal flag MSG_SENDPAGE_NOTLAST.
We have to ensure that MSG_MORE is also set if we set MSG_SENDPAGE_NOTLAST.
Otherwise users that check against MSG_MORE will not see it.
This fixes sendfile() on AF_ALG.
Cc: Tom Herbert <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: <[email protected]> # 3.4.x
Reported-and-tested-by: Shawn Landden <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
---
fs/splice.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/splice.c b/fs/splice.c
index 3b7ee65..b93f1b8 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -701,7 +701,7 @@ static int pipe_to_sendpage(struct pipe_inode_info *pipe,
more = (sd->flags & SPLICE_F_MORE) ? MSG_MORE : 0;
if (sd->len < sd->total_len && pipe->nrbufs > 1)
- more |= MSG_SENDPAGE_NOTLAST;
+ more |= MSG_SENDPAGE_NOTLAST | MSG_MORE;
return file->f_op->sendpage(file, buf->page, buf->offset,
sd->len, &pos, more);
--
1.8.1.4
On Sun, Nov 24, 2013 at 3:42 PM, Richard Weinberger <[email protected]> wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST.
> We have to ensure that MSG_MORE is also set if we set MSG_SENDPAGE_NOTLAST.
> Otherwise users that check against MSG_MORE will not see it.
>
> This fixes sendfile() on AF_ALG.
>
> Cc: Tom Herbert <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: <[email protected]> # 3.4.x
The offending commit also got backported to the 3.2 stable kernel, so
we need this fix there as well.
---
Shawn Landden
+1 360 389 3001 (SMS preferred)
On Mon, 2013-11-25 at 00:42 +0100, Richard Weinberger wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST.
> We have to ensure that MSG_MORE is also set if we set MSG_SENDPAGE_NOTLAST.
> Otherwise users that check against MSG_MORE will not see it.
>
> This fixes sendfile() on AF_ALG.
>
> Cc: Tom Herbert <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: <[email protected]> # 3.4.x
> Reported-and-tested-by: Shawn Landden <[email protected]>
> Signed-off-by: Richard Weinberger <[email protected]>
> ---
> fs/splice.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/splice.c b/fs/splice.c
> index 3b7ee65..b93f1b8 100644
> --- a/fs/splice.c
> +++ b/fs/splice.c
> @@ -701,7 +701,7 @@ static int pipe_to_sendpage(struct pipe_inode_info *pipe,
> more = (sd->flags & SPLICE_F_MORE) ? MSG_MORE : 0;
>
> if (sd->len < sd->total_len && pipe->nrbufs > 1)
> - more |= MSG_SENDPAGE_NOTLAST;
> + more |= MSG_SENDPAGE_NOTLAST | MSG_MORE;
>
> return file->f_op->sendpage(file, buf->page, buf->offset,
> sd->len, &pos, more);
I do not think this patch is right. It looks like a revert of a useful
patch for TCP zero copy. Given the time it took to discover this
regression, I bet tcp zero copy has more users than AF_ALG, by 5 or 6
order of magnitude ;)
Here we want to make the difference between the two flags, not merge
them.
If AF_ALG do not care of the difference, try instead :
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index ef5356cd280a..850246206b12 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
struct hash_ctx *ctx = ask->private;
int err;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
On Sun, Nov 24, 2013 at 5:25 PM, Eric Dumazet <[email protected]> wrote:
> On Mon, 2013-11-25 at 00:42 +0100, Richard Weinberger wrote:
>> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
>> added an internal flag MSG_SENDPAGE_NOTLAST.
>> We have to ensure that MSG_MORE is also set if we set MSG_SENDPAGE_NOTLAST.
>> Otherwise users that check against MSG_MORE will not see it.
>>
>> This fixes sendfile() on AF_ALG.
>>
>> Cc: Tom Herbert <[email protected]>
>> Cc: Eric Dumazet <[email protected]>
>> Cc: David S. Miller <[email protected]>
>> Cc: <[email protected]> # 3.4.x
>> Reported-and-tested-by: Shawn Landden <[email protected]>
>> Signed-off-by: Richard Weinberger <[email protected]>
>> ---
>> fs/splice.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/splice.c b/fs/splice.c
>> index 3b7ee65..b93f1b8 100644
>> --- a/fs/splice.c
>> +++ b/fs/splice.c
>> @@ -701,7 +701,7 @@ static int pipe_to_sendpage(struct pipe_inode_info *pipe,
>> more = (sd->flags & SPLICE_F_MORE) ? MSG_MORE : 0;
>>
>> if (sd->len < sd->total_len && pipe->nrbufs > 1)
>> - more |= MSG_SENDPAGE_NOTLAST;
>> + more |= MSG_SENDPAGE_NOTLAST | MSG_MORE;
>>
>> return file->f_op->sendpage(file, buf->page, buf->offset,
>> sd->len, &pos, more);
>
> I do not think this patch is right. It looks like a revert of a useful
> patch for TCP zero copy. Given the time it took to discover this
> regression, I bet tcp zero copy has more users than AF_ALG, by 5 or 6
> order of magnitude ;)
>
> Here we want to make the difference between the two flags, not merge
> them.
>
> If AF_ALG do not care of the difference, try instead :
>
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index ef5356cd280a..850246206b12 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
> struct hash_ctx *ctx = ask->private;
> int err;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> lock_sock(sk);
> sg_init_table(ctx->sgl.sg, 1);
> sg_set_page(ctx->sgl.sg, page, size, offset);
>
>From my testing this works.
--
---
Shawn Landden
+1 360 389 3001 (SMS preferred)
algif_hash and algif_skcipher used MSG_MORE from tcp_sendpages()
and need to see the new flag as identical to MSG_MORE.
This fixes sendfile() on AF_ALG.
Cc: Tom Herbert <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: <[email protected]> # 3.4.x + 3.2.x
Reported-and-tested-by: Shawn Landden <[email protected]>
Original-patch: Richard Weinberger <[email protected]>
Signed-off-by: Shawn Landden <[email protected]>
---
crypto/algif_hash.c | 3 +++
crypto/algif_skcipher.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index ef5356c..8502462 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
struct hash_ctx *ctx = ask->private;
int err;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 6a6dfc0..a19c027 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -378,6 +378,9 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
struct skcipher_sg_list *sgl;
int err = -EINVAL;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
if (!ctx->more && ctx->used)
goto unlock;
--
1.8.4.4
Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
added an internal flag MSG_SENDPAGE_NOTLAST, similar to
MSG_MORE.
algif_hash and algif_skcipher used MSG_MORE from tcp_sendpages()
and need to see the new flag as identical to MSG_MORE.
This fixes sendfile() on AF_ALG.
Cc: Tom Herbert <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: <[email protected]> # 3.4.x + 3.2.x
Reported-and-tested-by: Shawn Landden <[email protected]>
Original-patch: Richard Weinberger <[email protected]>
Signed-off-by: Shawn Landden <[email protected]>
---
crypto/algif_hash.c | 3 +++
crypto/algif_skcipher.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index ef5356c..8502462 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
struct hash_ctx *ctx = ask->private;
int err;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 6a6dfc0..a19c027 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -378,6 +378,9 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
struct skcipher_sg_list *sgl;
int err = -EINVAL;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
if (!ctx->more && ctx->used)
goto unlock;
--
1.8.4.4
On Sun, Nov 24, 2013 at 06:08:59PM -0800, Shawn Landden wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash and algif_skcipher used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
Don't we need a similar fix for udp_sendpage?
Greetings,
Hannes
Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
added an internal flag MSG_SENDPAGE_NOTLAST, similar to
MSG_MORE.
algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
and need to see the new flag as identical to MSG_MORE.
This fixes sendfile() on AF_ALG.
v3: also fix udp
Cc: Tom Herbert <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: <[email protected]> # 3.4.x + 3.2.x
Reported-and-tested-by: Shawn Landden <[email protected]>
Original-patch: Richard Weinberger <[email protected]>
Signed-off-by: Shawn Landden <[email protected]>
---
crypto/algif_hash.c | 3 +++
crypto/algif_skcipher.c | 3 +++
net/ipv4/udp.c | 3 +++
3 files changed, 9 insertions(+)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index ef5356c..8502462 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
struct hash_ctx *ctx = ask->private;
int err;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 6a6dfc0..a19c027 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -378,6 +378,9 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
struct skcipher_sg_list *sgl;
int err = -EINVAL;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
lock_sock(sk);
if (!ctx->more && ctx->used)
goto unlock;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5944d7d..8bd04df 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1098,6 +1098,9 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
struct udp_sock *up = udp_sk(sk);
int ret;
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
if (!up->pending) {
struct msghdr msg = { .msg_flags = flags|MSG_MORE };
--
1.8.4.4
Am Sonntag, 24. November 2013, 17:25:06 schrieb Eric Dumazet:
> On Mon, 2013-11-25 at 00:42 +0100, Richard Weinberger wrote:
> > Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> > added an internal flag MSG_SENDPAGE_NOTLAST.
> > We have to ensure that MSG_MORE is also set if we set
> > MSG_SENDPAGE_NOTLAST.
> > Otherwise users that check against MSG_MORE will not see it.
> >
> > This fixes sendfile() on AF_ALG.
> >
> > Cc: Tom Herbert <[email protected]>
> > Cc: Eric Dumazet <[email protected]>
> > Cc: David S. Miller <[email protected]>
> > Cc: <[email protected]> # 3.4.x
> > Reported-and-tested-by: Shawn Landden <[email protected]>
> > Signed-off-by: Richard Weinberger <[email protected]>
> > ---
> >
> > fs/splice.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/splice.c b/fs/splice.c
> > index 3b7ee65..b93f1b8 100644
> > --- a/fs/splice.c
> > +++ b/fs/splice.c
> > @@ -701,7 +701,7 @@ static int pipe_to_sendpage(struct pipe_inode_info
> > *pipe,>
> > more = (sd->flags & SPLICE_F_MORE) ? MSG_MORE : 0;
> >
> > if (sd->len < sd->total_len && pipe->nrbufs > 1)
> >
> > - more |= MSG_SENDPAGE_NOTLAST;
> > + more |= MSG_SENDPAGE_NOTLAST | MSG_MORE;
> >
> > return file->f_op->sendpage(file, buf->page, buf->offset,
> >
> > sd->len, &pos, more);
>
> I do not think this patch is right. It looks like a revert of a useful
> patch for TCP zero copy. Given the time it took to discover this
> regression, I bet tcp zero copy has more users than AF_ALG, by 5 or 6
> order of magnitude ;)
Yeah, but AF_ALG broke. That's why I did the patch.
> Here we want to make the difference between the two flags, not merge
> them.
>
> If AF_ALG do not care of the difference, try instead :
>
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index ef5356cd280a..850246206b12 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct
> page *page, struct hash_ctx *ctx = ask->private;
> int err;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
In the commit message of your patch you wrote "For all sendpage() providers,
its a transparent change.". Why does AF_ALG need special handling?
If users have to care about MSG_SENDPAGE_NOTLAST it is no longer really an
internal flag.
Thanks,
//richard
On Mon, Nov 25, 2013 at 7:36 AM, Shawn Landden <[email protected]> wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
>
> v3: also fix udp
>
> Cc: Tom Herbert <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: <[email protected]> # 3.4.x + 3.2.x
> Reported-and-tested-by: Shawn Landden <[email protected]>
> Original-patch: Richard Weinberger <[email protected]>
> Signed-off-by: Shawn Landden <[email protected]>
May I ask why you took over the my patch without even CC'in me nor
replying to the original
thread "[PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we
set MSG_SENDPAGE_NOTLAST"?
You are acting very rude.
The discussion at the original thread is not done.
Does skcipher_sendpage() really also need fixing? or UDP?
I didn't send another patch because I'm waiting for Eric's answer first.
Thanks,
//richard
> ---
> crypto/algif_hash.c | 3 +++
> crypto/algif_skcipher.c | 3 +++
> net/ipv4/udp.c | 3 +++
> 3 files changed, 9 insertions(+)
>
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index ef5356c..8502462 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -114,6 +114,9 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
> struct hash_ctx *ctx = ask->private;
> int err;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> lock_sock(sk);
> sg_init_table(ctx->sgl.sg, 1);
> sg_set_page(ctx->sgl.sg, page, size, offset);
> diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
> index 6a6dfc0..a19c027 100644
> --- a/crypto/algif_skcipher.c
> +++ b/crypto/algif_skcipher.c
> @@ -378,6 +378,9 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
> struct skcipher_sg_list *sgl;
> int err = -EINVAL;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> lock_sock(sk);
> if (!ctx->more && ctx->used)
> goto unlock;
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 5944d7d..8bd04df 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1098,6 +1098,9 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
> struct udp_sock *up = udp_sk(sk);
> int ret;
>
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> if (!up->pending) {
> struct msghdr msg = { .msg_flags = flags|MSG_MORE };
>
> --
> 1.8.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
//richard
On Mon, 2013-11-25 at 08:42 +0100, Richard Weinberger wrote:
> In the commit message of your patch you wrote "For all sendpage() providers,
> its a transparent change.". Why does AF_ALG need special handling?
> If users have to care about MSG_SENDPAGE_NOTLAST it is no longer really an
> internal flag.
MSG_SENDPAGE_NOTLAST is an internal (kernel) flag.
Fact that I missed some MSG_MORE 'users' in the kernel was an oversight.
I am not saying your patch is not needed, I am only saying it reverts
a useful TCP optimization, and we can do better, dont we ?
On Mon, Nov 25, 2013 at 7:36 AM, Shawn Landden <[email protected]> wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
>
> v3: also fix udp
>
> Cc: Tom Herbert <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: <[email protected]> # 3.4.x + 3.2.x
> Reported-and-tested-by: Shawn Landden <[email protected]>
> Original-patch: Richard Weinberger <[email protected]>
> Signed-off-by: Shawn Landden <[email protected]>
>May I ask why you took over the my patch without even CC'in me nor
>replying to the original
>thread "[PATCH] pipe_to_sendpage: Ensure that MSG_MORE is set if we
>set MSG_SENDPAGE_NOTLAST"?
>You are acting very rude.
Not CCing you was an oversight.
>The discussion at the original thread is not done.
>Does skcipher_sendpage() really also need fixing? or UDP?
UDP needs it or it will send out packets mid-sendfile. skcipher needs
it or it will produce incorrect
output like hash.
>I didn't send another patch because I'm waiting for Eric's answer first.
Eric forgot to update consumers internal consumers of MSG_MORE when he created
this distinction in sendpage. That became clear when he first
responded. Changing this where the consumers are makes sense. I just
want this bug fixed, feel free to send your next version.
>Thanks,
>//richard
-Shawn
On Sun, Nov 24, 2013 at 10:36:28PM -0800, Shawn Landden wrote:
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
>
> v3: also fix udp
The UDP bits look fine to me.
Greetings,
Hannes
Shawn,
Am Freitag, 29. November 2013, 06:47:04 schrieb Hannes Frederic Sowa:
> On Sun, Nov 24, 2013 at 10:36:28PM -0800, Shawn Landden wrote:
> > Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> > added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> > MSG_MORE.
> >
> > algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> > and need to see the new flag as identical to MSG_MORE.
> >
> > This fixes sendfile() on AF_ALG.
> >
> > v3: also fix udp
>
> The UDP bits look fine to me.
please don't forget to resend the patch (only the UDP fix).
I sent the AF_ALG part already.
Thanks,
//richard
From: Shawn Landden <[email protected]>
Date: Sun, 24 Nov 2013 22:36:28 -0800
> Commit 35f9c09fe (tcp: tcp_sendpages() should call tcp_push() once)
> added an internal flag MSG_SENDPAGE_NOTLAST, similar to
> MSG_MORE.
>
> algif_hash, algif_skcipher, and udp used MSG_MORE from tcp_sendpages()
> and need to see the new flag as identical to MSG_MORE.
>
> This fixes sendfile() on AF_ALG.
>
> v3: also fix udp
>
> Cc: Tom Herbert <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: <[email protected]> # 3.4.x + 3.2.x
> Reported-and-tested-by: Shawn Landden <[email protected]>
> Original-patch: Richard Weinberger <[email protected]>
> Signed-off-by: Shawn Landden <[email protected]>
Applied and queued up for -stable, thanks everyone.