Page MenuHomeFreeBSD

netgraph/ng_bridge: Replace NG_BRIDGE_MAX_LINKS with unlimited links
ClosedPublic

Authored by lutz_donnerhacke.de on Sep 26 2019, 3:50 PM.

Details

Summary

Remove the compile time limit for number of links a ng_bridge node can handle.

PR:240787

Test Plan
# kldload ng_bridge
# kldload ng_eiface
# ngctl

try some illegal link names first

+ mkpeer . bridge b1 link012345
ngctl: send msg: Invalid argument

create node

+ mkpeer . bridge b1 link12345

try more illegal link names

+ mkpeer . bridge b2 link12345678901234567890
ngctl: send msg: Invalid argument
+ mkpeer . bridge b2 123
ngctl: send msg: Invalid argument
+ mkpeer . bridge b2 link123test
ngctl: send msg: Invalid argument
+ mkpeer . bridge b2 123link
ngctl: send msg: Invalid argument

create node

+ mkpeer . bridge b2 link12345

add interfaces to the bridges

+ mkpeer b1 eiface link0 ether
+ mkpeer b2 eiface link0 ether
+ name b1.link0 if1
+ name b2.link0 if2

increase debuglevel and customize timeouts for loop detection

+ msg b1 setconfig { debugLevel=2 loopTimeout=60 maxStaleness=900 minStableAge=1 }
+ msg b2 setconfig { debugLevel=2 loopTimeout=30 maxStaleness=900 minStableAge=1 }

remove from controlling node

+ rmhook b1 
+ rmhook b2

cross-connect the nodes (incl selfconnect and multiple assignment)

+ connect if1:ether if1:ether link1 link2
ngctl: send msg: Too many levels of symbolic links
+ connect if1:ether if2:ether link1 link2
+ connect if1:ether if2:ether link1 link2
ngctl: send msg: File exists

build a loop

+ connect if1:ether if2:ether link654321 link123456

summary of the structure

+ list -l
  Name: <unnamed>       Type: bridge          ID: 0000002f   Num hooks: 3
  Local hook      Peer name       Peer type    Peer ID         Peer hook
  ----------      ---------       ---------    -------         ---------
  link1           <unnamed>       bridge       00000034        link2
  link654321      <unnamed>       bridge       00000034        link123456
  link0           if1             eiface       00000035        ether

  Name: <unnamed>       Type: bridge          ID: 00000034   Num hooks: 3
  Local hook      Peer name       Peer type    Peer ID         Peer hook
  ----------      ---------       ---------    -------         ---------
  link2           <unnamed>       bridge       0000002f        link1
  link123456      <unnamed>       bridge       0000002f        link654321
  link0           if2             eiface       00000036        ether

  Name: if1             Type: eiface          ID: 00000035   Num hooks: 1
  Local hook      Peer name       Peer type    Peer ID         Peer hook
  ----------      ---------       ---------    -------         ---------
  ether           <unnamed>       bridge       0000002f        link0

  Name: if2             Type: eiface          ID: 00000036   Num hooks: 1
  Local hook      Peer name       Peer type    Peer ID         Peer hook
  ----------      ---------       ---------    -------         ---------
  ether           <unnamed>       bridge       00000034        link0

bring an interface into a usable state

+ msg if1: set 00:01:02:03:04:05

back on the shell, test the connectivity (the eiface nodes may have different names)

# ifconfig ngeth2 up
# ifconfig ngeth3 up
# tcpdump -nei ngeth3 -s0 -vv &
tcpdump: listening on ngeth3, link-type EN10MB (Ethernet), capture size 262144 bytes
# dhclient -d ngeth2
DHCPDISCOVER on ngeth2 to 255.255.255.255 port 67 interval 7
16:27:22.651839 00:01:02:03:04:05 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: (tos 0x10, ttl 128, id 0, offset 0, flags [none], proto UDP (17), length 328)
    0.0.0.0.68 > 255.255.255.255.67: [udp sum ok] BOOTP/DHCP, Request from 00:01:02:03:04:05, length 300, xid 0x836640d6, Flags [none] (0x0000)
          Client-Ethernet-Address 00:01:02:03:04:05
          Vendor-rfc1048 Extensions
            Magic Cookie 0x63825363
            DHCP-Message Option 53, length 1: Discover
            Client-ID Option 61, length 7: ether 00:01:02:03:04:05
            Hostname Option 12, length 6: "a10nsp"
            Parameter-Request Option 55, length 10: 
              Subnet-Mask, BR, Time-Zone, Classless-Static-Route
              Default-Gateway, Domain-Name, Domain-Name-Server, Hostname
              Option 119, MTU
DHCPDISCOVER on ngeth2 to 255.255.255.255 port 67 interval 8
16:27:29.727550 00:01:02:03:04:05 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: (tos 0x10, ttl 128, id 0, offset 0, flags [none], proto UDP (17), length 328)
    0.0.0.0.68 > 255.255.255.255.67: [udp sum ok] BOOTP/DHCP, Request from 00:01:02:03:04:05, length 300, xid 0x836640d6, secs 7, Flags [none] (0x0000)
          Client-Ethernet-Address 00:01:02:03:04:05
          Vendor-rfc1048 Extensions
            Magic Cookie 0x63825363
            DHCP-Message Option 53, length 1: Discover
            Client-ID Option 61, length 7: ether 00:01:02:03:04:05
            Hostname Option 12, length 6: "a10nsp"
            Parameter-Request Option 55, length 10: 
              Subnet-Mask, BR, Time-Zone, Classless-Static-Route
              Default-Gateway, Domain-Name, Domain-Name-Server, Hostname
              Option 119, MTU
^C

show statistics

# ngctl
+ msg if1:ether getstats 1
Rec'd response "getstats" (4) from "[2f]:":
Args:   { recvOctets=342 recvPackets=1 recvBroadcast=1 loopDrops=3 loopDetects=1 }
+ msg if1:ether getstats 654321
Rec'd response "getstats" (4) from "[2f]:":
Args:   { xmitOctets=1026 xmitPackets=3 xmitBroadcasts=3 }
+ msg if1:ether getstats 0
Rec'd response "getstats" (4) from "[2f]:":
Args:   { recvOctets=1026 recvPackets=3 recvBroadcast=3 }
+ msg if2:ether getstats 2
Rec'd response "getstats" (4) from "[34]:":
Args:   { recvOctets=342 recvPackets=1 recvBroadcast=1 loopDrops=3 loopDetects=1 }
+ msg if2:ether getstats 123456
Rec'd response "getstats" (4) from "[34]:":
Args:   { recvOctets=1026 recvPackets=3 recvBroadcast=3 }
+ msg if2:ether getstats 0
Rec'd response "getstats" (4) from "[34]:":
Args:   { xmitOctets=1026 xmitPackets=3 xmitBroadcasts=3 }

try illegal queries

+ msg if2:ether getstats 1
ngctl: send msg: Socket is not connected

show forwarding table (the new interface)

+ msg if2:ether gettable
Rec'd response "gettable" (7) from "[2f]:":
Args:   { numHosts=1 hosts=[ { addr=00:01:02:03:04:05 hook="link654321" age=30 staleness=30 } ] }
+ msg if1:ether gettable
Rec'd response "gettable" (7) from "[34]:":
Args:   { numHosts=1 hosts=[ { addr=00:01:02:03:04:05 hook="link0" age=37 staleness=37 } ] }

clear state of a single side of an hook, should not affect the peer

+ msg if2:ether getclrstats 2
Rec'd response "getclrstats" (6) from "[34]:":
Args:   { recvOctets=342 recvPackets=1 recvBroadcast=1 loopDrops=3 loopDetects=1 }
+ msg if2:ether getstats 2
Rec'd response "getstats" (4) from "[34]:":
Args:   {}
+ msg if1:ether getstats 1
Rec'd response "getstats" (4) from "[2f]:":
Args:   { recvOctets=342 recvPackets=1 recvBroadcast=1 loopDrops=3 loopDetects=1 }

bring the test network down

+ shutdown if1:ether
+ shutdown if2:ether
+ shutdown if1:
+ shutdown if2:
tcpdump: pcap_loop: The interface went down
2 packets captured
2 packets received by filter
0 packets dropped by kernel
[1]  + Exit 1                        tcpdump -nei ngeth3 -s0 -vv

unload the module and check for memory leaks

# kldunload ng_bridge
# dmesg | tail
ngeth2: link state changed to UP
ngeth3: link state changed to UP
ngeth3: promiscuous mode enabled
ng_bridge: [2f]: loopback detected on link1
ng_bridge: [34]: loopback detected on link2
ng_bridge: [34]: restoring looped back link2
ng_bridge: [2f]: restoring looped back link1
ngeth2: link state changed to DOWN
ngeth3: link state changed to DOWN
ngeth3: promiscuous mode disabled

Nothing.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

share/man/man4/ng_bridge.4
109 ↗(On Diff #62600)

There is no ipfw functionality. It's completely unclear which configuration would be needed ...

sys/netgraph/ng_bridge.c
530 ↗(On Diff #62600)

NG_NODE_FOREACH_HOOK requires an external function and a single external data pointer

685 ↗(On Diff #62600)

All this code is moved to the external function ng_bridge_send_ctx

798 ↗(On Diff #62600)

NG_HOOK_PRIVATE always exists

sys/netgraph/ng_bridge.h
48 ↗(On Diff #62600)

Removing the ipfw config and modifying the gettable output requires a new cookie.

118 ↗(On Diff #62600)

Internal pointer "link" is useless outside.
It's more interesting to know on which hook the host is located.

bcr accepted this revision as: manpages.Sep 26 2019, 5:20 PM
bcr added a subscriber: bcr.

OK from manpages.

lutz_donnerhacke.de edited the test plan for this revision. (Show Details)Sep 27 2019, 9:07 PM
lutz_donnerhacke.de edited the test plan for this revision. (Show Details)
lutz_donnerhacke.de edited the summary of this revision. (Show Details)Sep 30 2019, 7:06 AM
glebius accepted this revision.Sep 30 2019, 8:45 PM
glebius added a subscriber: glebius.
glebius added inline comments.
sys/netgraph/ng_bridge.c
348 ↗(On Diff #62600)

Typo.

361 ↗(On Diff #62600)

In general (there could be exclusions) nodes run newhook method in context of syscall, thus sleeping for memory is okay.

981 ↗(On Diff #62600)

Braces around return value.

sys/netgraph/ng_bridge.h
122 ↗(On Diff #62600)

The project moves from historic BSD u_intXX_t types to C99 uintXX_t. Please use the latter in the new code.

This revision is now accepted and ready to land.Sep 30 2019, 8:45 PM

What is the next step?
Should I replace the patch by a new one in order to correct the remaining issues?

sys/netgraph/ng_bridge.h
122 ↗(On Diff #62600)

I'd stay with the old type for this patch.
Afterwards I'd rework the whole file to new types.
That would keep the patches self-contained and don't result in inconsistent types for the whole file.
Okay?

Fix spelling and delayed allocation

This revision now requires review to proceed.Oct 1 2019, 8:15 AM
lutz_donnerhacke.de marked 3 inline comments as done.Oct 1 2019, 8:18 AM
markj added a subscriber: markj.Oct 1 2019, 2:23 PM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 3 2019, 2:33 AM
This revision was automatically updated to reflect the committed changes.