fcntl() failures in TCP / MPTCP tests

[Davide Caratti]

hello,

running packetdrill on latest "net-next" [1], we observed systematic
failures of some tests. In details, the return value of

fcntl(3, F_GETFL) = 0x2 (flags O_RDWR)

is no more 0x2, but it's 0x4000002 (O_RDWR | O_NOATIME).
Apparently this is caused by commit:

commit e9c15badbb7b20ccdbadf5da14e0a68fbad51015
Author: Mel Gorman <mgo...@techsingularity.net>
Date: Mon Jun 15 13:13:58 2020 +0100

fs: Do not check if there is a fsnotify watcher on pseudo inodes


Where do you think we should develop a fix? adjusting the script/the
program (to let them accept a bitmask instead of a scalar number), or
maybe it's the kernel code that needs to be corrected?

any feedback appreciated, thank you in advance!
--
davide

[1] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git

[Neal Cardwell]

Hi,

Thanks for the report. Can you please provide the exact net-next SHA1 you are testing, and attach a copy of the script you are running?

thanks,

neal

--
You received this message because you are subscribed to the Google Groups "packetdrill" group.
To unsubscribe from this group and stop receiving emails from it, send an email to packetdrill...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/packetdrill/f2700b909cf367add3526b74db9b687d8907de57.camel%40redhat.com.

[Davide Caratti]

hello Neal,

On Mon, 2020-06-29 at 10:39 -0400, Neal Cardwell wrote:
> Hi,
>
> Thanks for the report. Can you please provide the exact net-next SHA1
> you are testing, and attach a copy of the script you are running?

thanks for looking a this. See below:

# git log --oneline -1
ee4a3be (HEAD, google/master) net-test: packetdrill: fix memory leak in netdev_receive_loop()
# cat prova.pkt
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
0.1 fcntl(3, F_GETFL) = 0x2 (flags O_RDWR)
# ./packetdrill -vvv ./prova.pkt
socket syscall: 1593441787.750605
fcntl syscall: 1593441787.850536
./prova.pkt:2: runtime error in fcntl call: Expected result 2 but got 67108866 with errno 2 (No such file or directory)
# uname -a
Linux dhcp-158-243.lab.eng.brq.redhat.com 5.8.0-rc2+ #207 SMP Mon Jun 29 12:46:16 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
#

the kernel source is compiled from:

bed14551659 (mptcp/net-next) Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net

--
davide

[Eric Dumazet]

This seems a kernel bug to me.

FMODE_NONOTIFY was not meant to be visible to user space. (otherwise
there would be a O_NONOTIFY)


(This is mentioned in tools/include/uapi/asm-generic/fcntl.h)

[Eric Dumazet]

This is already discussed on lkml, Mel posted a fix already.

[Eric Dumazet]

From a81c729a5c52ddb2d8d98220478e492b71956574 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir...@gmail.com>
Date: Sun, 28 Jun 2020 15:36:56 +0300
Subject: [PATCH] fsnotify: suppress access/modify events on stream files

We wanted to suppress all fsnotify events on anonymous pipes/sockets,
but chromioum seems to be relying on some of those events.

Let's try to suppress only access/modify events on stream files.

Reported-by: Maxim Levitsky <mlev...@redhat.com>
Link: https://lore.kernel.org/lkml/7b4aa1e985007c6d582fffe...@redhat.com
Fixes: e9c15badbb7b ("fs: Do not check if there is a fsnotify watcher on pseudo inodes")
Signed-off-by: Amir Goldstein <amir...@gmail.com>
---
 fs/file_table.c          | 2 +-
 include/linux/fsnotify.h | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index 65603502fed6..656647f9575a 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -230,7 +230,7 @@ struct file *alloc_file_pseudo(struct inode *inode, struct vfsmount *mnt,
  d_set_d_op(path.dentry, &anon_ops);
  path.mnt = mntget(mnt);
  d_instantiate(path.dentry, inode);
- file = alloc_file(&path, flags | FMODE_NONOTIFY, fops);
+ file = alloc_file(&path, flags, fops);
  if (IS_ERR(file)) {
  ihold(inode);
  path_put(&path);
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 5ab28f6c7d26..3a07824332f5 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -246,6 +246,9 @@ static inline void fsnotify_rmdir(struct inode *dir, struct dentry *dentry)
  */
 static inline void fsnotify_access(struct file *file)
 {
+ if (file->f_mode & FMODE_STREAM)
+ return 0;
+
  fsnotify_file(file, FS_ACCESS);
 }
 
@@ -254,6 +257,9 @@ static inline void fsnotify_access(struct file *file)
  */
 static inline void fsnotify_modify(struct file *file)
 {
+ if (file->f_mode & FMODE_STREAM)
+ return 0;
+
  fsnotify_file(file, FS_MODIFY);
 }
 
--
2.17.1

[Davide Caratti]

On Mon, 2020-06-29 at 08:04 -0700, Eric Dumazet wrote:
> From a81c729a5c52ddb2d8d98220478e492b71956574 Mon Sep 17 00:00:00 2001
> From: Amir Goldstein <amir...@gmail.com>
> Date: Sun, 28 Jun 2020 15:36:56 +0300
> Subject: [PATCH] fsnotify: suppress access/modify events on stream files
>
> We wanted to suppress all fsnotify events on anonymous pipes/sockets,
> but chromioum seems to be relying on some of those events.
>
> Let's try to suppress only access/modify events on stream files.
>
> Reported-by: Maxim Levitsky <mlev...@redhat.com>
> Link: https://lore.kernel.org/lkml/7b4aa1e985007c6d582fffe...@redhat.com
> Fixes: e9c15badbb7b ("fs: Do not check if there is a fsnotify watcher on pseudo inodes")
> Signed-off-by: Amir Goldstein <amir...@gmail.com>
> ---

hello Eric,

sorry, I didn't notice there was a fix: thanks a lot for pointing to it!
regards,
--
davide