Bug 241954

Summary: netgraph: Fails on concurrent node rename
Product: Base System Reporter: Paul Armstrong <freebsd>
Component: kernAssignee: Lutz Donnerhacke <donner>
Status: Closed FIXED    
Severity: Affects Many People CC: afedorov, alexx, donner, net, pi
Priority: --- Flags: donner: mfc-stable13+
donner: mfc-stable12+
donner: mfc-stable11+
Version: 12.1-RELEASE   
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D30110
See Also: https://github.com/genneko/freebsd-vimage-jails/issues/2

Description Paul Armstrong 2019-11-13 18:51:29 UTC
I have a FreeBSD 12.1 host on which I'm using jng to create netgraph nodes for host interfaces. If I start the jails in series, it works fine. If I start them in parallel netgraph fails to rename the nodes and those jails fail to initialize.

jail.conf:

exec.prestart = "";
exec.start = "/bin/sh /etc/rc";
exec.stop = "/bin/sh /etc/rc.shutdown";
exec.poststop = "";
exec.clean;
mount.devfs;
mount.fdescfs;
mount.procfs;
persist;
vnet = new;
devfs_ruleset = 10; # random, log, null, fd/* and other critical items

# Dynamic wildcard parameter:
# Base the path off the jail name.
path = "/jail/${host.hostname}/root";
exec.consolelog = "/var/log/jail_${host.hostname}_console.log";

################################################################################
# External Jails
################################################################################

ext {
  host.hostname = "ext.example.com";
  exec.prestart += "jng bridge ext bridge0 bridge1";
  exec.poststop += "jng shutdown ext";
  vnet.interface = ng0_ext,ng1_ext;
}

################################################################################
# Internal Jails
################################################################################

ns0 {
  host.hostname = "ns0.example.com";
  exec.prestart += "jng bridge ns0 bridge0 bridge1";
  exec.poststop += "jng shutdown ns0";
  vnet.interface = ng0_ns0,ng1_ns0;
}

ns1 {
  host.hostname = "ns1.example.com";
  exec.prestart += "jng bridge ns1 bridge0 bridge1";
  exec.poststop += "jng shutdown ns1";
  vnet.interface = ng0_ns1,ng1_ns1;
}

wg {
  host.hostname = "wg.example.com";
  exec.prestart += "jng bridge wg bridge0 bridge1";
  exec.poststop += "jng shutdown wg";
  vnet.interface = ng0_wg,ng1_wg;
  devfs_ruleset = 11; # include tun
}

www {
  host.hostname = "www.example.com";
  exec.prestart += "jng bridge www bridge0 bridge1";
  exec.poststop += "jng shutdown www";
  vnet.interface = ng0_www,ng1_www;
  sysvshm; # postgresql
}

vault {
  host.hostname = "vault.example.com";
  exec.prestart += "jng bridge vault bridge0 bridge1";
  exec.poststop += "jng shutdown vault";
  vnet.interface = ng0_vault,ng1_vault;
}

dmesg:

WARNING: attempt to domain_add(netgraph) after domainfinalize()
bridge0: promiscuous mode enabled
ngeth0: link state changed to UP
ngeth0: changing name to 'ng0_www'
bridge1: promiscuous mode enabled
ngeth1: link state changed to UP
ngeth1: changing name to 'ng1_www'
ng_ether_ifnet_arrival_event: can't re-name node ng0_www
ng_ether_ifnet_arrival_event: can't re-name node ng1_www
lo0: link state changed to UP

Jail start (clean after boot with jails disabled):

[root@ext]# service jail start
Starting jails:ngctl: send msg: File exists
ngctl: send msg: File exists
ngctl: send msg: File exists
ngctl: jail: send msgns0: jng bridge ns0 bridge0 bridge1: failed:
jail: ngctl: wg: jng bridge wg bridge0 bridge1: failedFile exists

send msgjail: : ns1: jng bridge ns1 bridge0 bridge1: failed
File exists
jail: vault: jng bridge vault bridge0 bridge1: failed
jail: numbat: jng bridge numbat bridge0 bridge1: failed
ng0_www
ng1_www
www: created
Comment 1 Kurt Jaeger freebsd_committer 2021-03-08 10:33:30 UTC
See also:

https://github.com/genneko/freebsd-vimage-jails/issues/2
Comment 2 Lutz Donnerhacke freebsd_committer 2021-05-04 16:57:51 UTC
https://github.com/genneko/freebsd-vimage-jails/issues/2 describes the problem in detail: ng_name_node() is called twice and does not recognize the rename to the identical name. This might be solvable.
Comment 3 Lutz Donnerhacke freebsd_committer 2021-05-04 19:29:36 UTC
Please test this patch: https://reviews.freebsd.org/D30110
Comment 4 commit-hook freebsd_committer 2021-06-04 09:22:36 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=0345fd891fe13a191fc0fae9463ea9458bfaff5a

commit 0345fd891fe13a191fc0fae9463ea9458bfaff5a
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-05-04 19:20:39 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-06-04 09:20:19 +0000

    netgraph/ng_base: Renaming a node to the same name is a noop

    Detailed analysis in https://github.com/genneko/freebsd-vimage-jails/issues/2
    brought the problem down to a double call of ng_node_name() before and
    after a vnet move.  Because the name of the node is already known
    (occupied by itself), the second call fails.

    PR:             241954
    Reported by:    Paul Armstrong
    MFC:            1 week
    Differential Revision: https://reviews.freebsd.org/D30110

 sys/netgraph/ng_base.c | 4 ++++
 1 file changed, 4 insertions(+)
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2021-06-05 00:05:32 UTC
^Triage: Re-open pending merges
Comment 6 commit-hook freebsd_committer 2021-06-10 09:31:42 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=37370a6773cde7fcae063aed0a4cc024f95534bd

commit 37370a6773cde7fcae063aed0a4cc024f95534bd
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-05-04 19:20:39 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-06-10 09:29:34 +0000

    netgraph/ng_base: Renaming a node to the same name is a noop

    Detailed analysis in https://github.com/genneko/freebsd-vimage-jails/issues/2
    brought the problem down to a double call of ng_node_name() before and
    after a vnet move.  Because the name of the node is already known
    (occupied by itself), the second call fails.

    PR:             241954
    Reported by:    Paul Armstrong
    Differential Revision: https://reviews.freebsd.org/D30110

    (cherry picked from commit 0345fd891fe13a191fc0fae9463ea9458bfaff5a)

 sys/netgraph/ng_base.c | 4 ++++
 1 file changed, 4 insertions(+)
Comment 7 commit-hook freebsd_committer 2021-06-10 09:34:46 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a0694da5d035ea0e686ad1bae95709e52df0913a

commit a0694da5d035ea0e686ad1bae95709e52df0913a
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-05-04 19:20:39 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-06-10 09:33:19 +0000

    netgraph/ng_base: Renaming a node to the same name is a noop

    Detailed analysis in https://github.com/genneko/freebsd-vimage-jails/issues/2
    brought the problem down to a double call of ng_node_name() before and
    after a vnet move.  Because the name of the node is already known
    (occupied by itself), the second call fails.

    PR:             241954
    Reported by:    Paul Armstrong
    Differential Revision: https://reviews.freebsd.org/D30110

    (cherry picked from commit 0345fd891fe13a191fc0fae9463ea9458bfaff5a)

 sys/netgraph/ng_base.c | 4 ++++
 1 file changed, 4 insertions(+)
Comment 8 commit-hook freebsd_committer 2021-06-10 09:36:47 UTC
A commit in branch stable/11 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b57a593cb2d69a684e4be04e8cea7fe7a7f43b24

commit b57a593cb2d69a684e4be04e8cea7fe7a7f43b24
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-05-04 19:20:39 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-06-10 09:35:21 +0000

    netgraph/ng_base: Renaming a node to the same name is a noop

    Detailed analysis in https://github.com/genneko/freebsd-vimage-jails/issues/2
    brought the problem down to a double call of ng_node_name() before and
    after a vnet move.  Because the name of the node is already known
    (occupied by itself), the second call fails.

    PR:             241954
    Reported by:    Paul Armstrong
    Differential Revision: https://reviews.freebsd.org/D30110

    (cherry picked from commit 0345fd891fe13a191fc0fae9463ea9458bfaff5a)

 sys/netgraph/ng_base.c | 4 ++++
 1 file changed, 4 insertions(+)
Comment 9 commit-hook freebsd_committer 2021-06-15 12:40:45 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=36c7408818c53ae0c1f1aee0250c5d3fe16b06e0

commit 36c7408818c53ae0c1f1aee0250c5d3fe16b06e0
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-06-02 22:29:46 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-06-15 12:31:32 +0000

    tests/netgraph: Inital framework for testing libnetgraph

    Provide a framework of functions to test various netgraph modules.
    Tests contain:
     - creating, renaming, and destroying nodes
     - connecting and removing hooks
     - sending and receiving data
     - sending ASCII messages and receiving binary responses
     - errors can be passed for indiviual inspection or fail the test

    Reviewed by:    kp
    Differential Revision: https://reviews.freebsd.org/D30629
    Differential Revision: https://reviews.freebsd.org/D30657
    Differential Revision: https://reviews.freebsd.org/D30671
    Differential Revision: https://reviews.freebsd.org/D30699

    (cherry picked from commit 24ea1dbf257aa6757f469bcd859f90e9ad851e59)
    (cherry picked from commit 09307dbfb888a98232096c751a96ecb3344aa77c)
    (cherry picked from commit 9021c46603bf29b9700f24b8dce8796b434d7c8f)
    (cherry picked from commit 5554abd9cc9702af30af90925b33c5efff4e7d88)

    Also contains some fixups:
     - indent all files correctly
     - finish factoring out
     - remove debugging code
     - check for renaming issues reported in PR241954

    PR:     241954
    Differential Revision: https://reviews.freebsd.org/D30692
    Differential Revision: https://reviews.freebsd.org/D30714
    Differential Revision: https://reviews.freebsd.org/D30713

    (cherry picked from commit a664ade93972ce617f0888ff79e715dff9cf0f87)
    (cherry picked from commit 0afa9be03937d60cb5aeba64c81e3e2165bd3737)
    (cherry picked from commit 43e4821315c31db067e23564b9bfafb519e77b2b)

 tests/sys/netgraph/Makefile      |   6 +-
 tests/sys/netgraph/basic.c (new) | 191 +++++++++++++++++++++++++++
 tests/sys/netgraph/util.c (new)  | 277 +++++++++++++++++++++++++++++++++++++++
 tests/sys/netgraph/util.h (new)  | 114 ++++++++++++++++
 4 files changed, 587 insertions(+), 1 deletion(-)
Comment 10 commit-hook freebsd_committer 2021-06-16 11:46:10 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2def8906341231cd6b3647756c182f60d114deee

commit 2def8906341231cd6b3647756c182f60d114deee
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-06-02 22:29:46 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-06-16 11:44:15 +0000

    tests/netgraph: Inital framework for testing libnetgraph

    Provide a framework of functions to test various netgraph modules.
    Tests contain:
     - creating, renaming, and destroying nodes
     - connecting and removing hooks
     - sending and receiving data
     - sending ASCII messages and receiving binary responses
     - errors can be passed for indiviual inspection or fail the test

    Reviewed by:    kp
    Differential Revision: https://reviews.freebsd.org/D30629
    Differential Revision: https://reviews.freebsd.org/D30657
    Differential Revision: https://reviews.freebsd.org/D30671
    Differential Revision: https://reviews.freebsd.org/D30699

    (cherry picked from commit 24ea1dbf257aa6757f469bcd859f90e9ad851e59)
    (cherry picked from commit 09307dbfb888a98232096c751a96ecb3344aa77c)
    (cherry picked from commit 9021c46603bf29b9700f24b8dce8796b434d7c8f)
    (cherry picked from commit 5554abd9cc9702af30af90925b33c5efff4e7d88)

    Also contains some fixups:
     - indent all files correctly
     - finish factoring out
     - remove debugging code
     - check for renaming issues reported in PR241954

    PR:     241954
    Differential Revision: https://reviews.freebsd.org/D30692
    Differential Revision: https://reviews.freebsd.org/D30714
    Differential Revision: https://reviews.freebsd.org/D30713

    (cherry picked from commit a664ade93972ce617f0888ff79e715dff9cf0f87)
    (cherry picked from commit 0afa9be03937d60cb5aeba64c81e3e2165bd3737)
    (cherry picked from commit 43e4821315c31db067e23564b9bfafb519e77b2b)

 tests/sys/netgraph/Makefile      |   6 +
 tests/sys/netgraph/basic.c (new) | 191 +++++++++++++++++++++++++++
 tests/sys/netgraph/util.c (new)  | 277 +++++++++++++++++++++++++++++++++++++++
 tests/sys/netgraph/util.h (new)  | 114 ++++++++++++++++
 4 files changed, 588 insertions(+)
Comment 11 commit-hook freebsd_committer 2021-06-16 11:50:13 UTC
A commit in branch stable/11 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4a0c1d0543afd83156b50b3f9b74c79e74cf526f

commit 4a0c1d0543afd83156b50b3f9b74c79e74cf526f
Author:     Lutz Donnerhacke <donner@FreeBSD.org>
AuthorDate: 2021-06-02 22:29:46 +0000
Commit:     Lutz Donnerhacke <donner@FreeBSD.org>
CommitDate: 2021-06-16 11:48:22 +0000

    tests/netgraph: Inital framework for testing libnetgraph

    Provide a framework of functions to test various netgraph modules.
    Tests contain:
     - creating, renaming, and destroying nodes
     - connecting and removing hooks
     - sending and receiving data
     - sending ASCII messages and receiving binary responses
     - errors can be passed for indiviual inspection or fail the test

    Reviewed by:    kp
    Differential Revision: https://reviews.freebsd.org/D30629
    Differential Revision: https://reviews.freebsd.org/D30657
    Differential Revision: https://reviews.freebsd.org/D30671
    Differential Revision: https://reviews.freebsd.org/D30699

    (cherry picked from commit 24ea1dbf257aa6757f469bcd859f90e9ad851e59)
    (cherry picked from commit 09307dbfb888a98232096c751a96ecb3344aa77c)
    (cherry picked from commit 9021c46603bf29b9700f24b8dce8796b434d7c8f)
    (cherry picked from commit 5554abd9cc9702af30af90925b33c5efff4e7d88)

    Also contains some fixups:
     - indent all files correctly
     - finish factoring out
     - remove debugging code
     - check for renaming issues reported in PR241954

    PR:     241954
    Differential Revision: https://reviews.freebsd.org/D30692
    Differential Revision: https://reviews.freebsd.org/D30714
    Differential Revision: https://reviews.freebsd.org/D30713

    (cherry picked from commit a664ade93972ce617f0888ff79e715dff9cf0f87)
    (cherry picked from commit 0afa9be03937d60cb5aeba64c81e3e2165bd3737)
    (cherry picked from commit 43e4821315c31db067e23564b9bfafb519e77b2b)

 tests/sys/netgraph/Makefile      |   6 +
 tests/sys/netgraph/basic.c (new) | 191 +++++++++++++++++++++++++++
 tests/sys/netgraph/util.c (new)  | 277 +++++++++++++++++++++++++++++++++++++++
 tests/sys/netgraph/util.h (new)  | 114 ++++++++++++++++
 4 files changed, 588 insertions(+)