Page MenuHomeFreeBSD
Differential D4004

Yet another if_lagg lladdr/eventhandler fix.
ClosedPublic
Actions

Authored by melifaro on Oct 26 2015, 10:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 7, 2:43 PM
Unknown Object (File)
Tue, Mar 25, 6:54 AM
Unknown Object (File)
Feb 13 2025, 3:33 PM
Unknown Object (File)
Feb 13 2025, 2:08 PM
Unknown Object (File)
Jan 15 2025, 3:07 PM
Unknown Object (File)
Jan 10 2025, 12:16 AM
Unknown Object (File)
Jan 6 2025, 7:58 AM
Unknown Object (File)
Dec 12 2024, 3:23 AM
View All Files
Subscribers
imp

Details

Reviewers
ae
hiren
hrs
glebius
araujo
Group Reviewers
network
Commits
rS290239: Fix lladdr change propagation for on vlans on top of it.
Summary

Recent D3301 change improved thigs with lladdr propagation. However, there are number of outstanding issues that have to be fixed:

  1. lagg does not propagate lladdr change on vlans on top of it.
  2. lagg does not update "primary" interface lladdr when setting mac address manually.
  3. lladdr_event notifications are not always sent (for example, when adding slave port).
  4. Things not addressed in this review
  5. livelock on "./checklagg.sh cxl0 cxl1" due to (devd) "ifconfig lagg0 inet6 disabled" and "ifconfig lagg0 create laggport cxl0" cmds
  6. lagg llq is parsed in LIFO order (not

The ultimate goal is to make lladdr_event be reliably triggered. After that, arp-sending code from if_setlladdr() can be moved arp llhandler and long-standing issue with updating ifaddr lle (kern/145300, kern/152235) can also be addressed.

Actual changelist:

  • Rework lagg_port_lladdr / lagg_port_setlladdr logic: now lagg_port_lladdr() consumers are responsible for specifying how lladdr update should be executed. This is done base on update _type_. There are only two of them: LAGG_LLQTYPE_PHYS (represents physical port, update can be skipped if current lladr is the same, BUT if we're executing from taskqueue we have to do both if_setlladdr() and lladdr notification). LAGG_LLQTYPE_VIRT (represents virtual e.g. lagg interface itself. Update is queue anyway, BUT we avoid calling if_setlladdr() (would trigger down/up) and just perform notification).
  • Add (hopefully useful) comments on various lladdr queueing calls.
  • Simplify "primary" port case in lagg_port_destroy().
  • Add LAGG_UNLOCK_ASSERT() to note lagg_proto_detach() returns unlocked.
  • Added __noinline to some control function to ease dtrace instrumentation.

See long, boring test plan with explanation/testing various scenarios with checklagg.sh script, slightly modified kernel and dtrace script (sources for all are attached).
Most debug was performed with vlans as lagg ports, however I performed similar testing with ixgbe/cxl drivers with similar results.

Very short version:

HEAD (r289939)

P81 HEAD (r289939)
1[21:34:00] Working on:
2 ix0 90:e2:ba:0f:a0:04
3 ix1 90:e2:ba:0f:a0:05
4[21:34:00] Creating lagg lagg0 on top of ix0
5[21:34:00] Testing inherited mac.. OK
6
7[21:34:05] Creating vlan devices (vlan900, vlan901) on lagg0.. DONE
8[21:34:05] Checking if interfaces has been created with proper macs..
9 vlan900 OK
10 vlan901 OK
11[21:34:10] Adding (slave) port ix1 to lagg0.. DONE
12[21:34:10] Checking if slave port mac has changed.. OK
13[21:34:15] Removing primary port from lagg (ix1 is new primary).. DONE
14Sleeping to give physical interface chance to reinit..
15[21:34:17] Checking if all interfaces has updated their macs..
16 ix0 OK
17 ix1 OK
18 lagg0 OK
19 vlan900 FAIL
20 vlan901 FAIL
21[21:34:22] Adding (slave) port ix0 back.. DONE
22[21:34:22] Checking if slave port mac has changed.. OK
23[21:34:27] Trying to change mac manually to 52:54:00:00:00:00.. DONE
24Sleeping to give physical interface chance to reinit..
25[21:34:29] Checking if all interfaces has updated their macs..
26 ix0 OK
27 ix1 FAIL
28 lagg0 OK
29 vlan900 OK
30 vlan901 OK
31[21:34:34] Cleaning up.. DONE
32[21:34:35] Checking original macs..
33 ix0 OK
34 ix1 OK

PATCHED

P82 Patched
1[21:29:45] Working on:
2 ix0 90:e2:ba:0f:a0:04
3 ix1 90:e2:ba:0f:a0:05
4[21:29:45] Creating lagg lagg0 on top of ix0
5[21:29:45] Testing inherited mac.. OK
6[21:29:50] Creating vlan devices (vlan900, vlan901) on lagg0.. DONE
7[21:29:50] Checking if interfaces has been created with proper macs..
8 vlan900 OK
9 vlan901 OK
10[21:29:55] Adding (slave) port ix1 to lagg0.. DONE
11[21:29:55] Checking if slave port mac has changed.. OK
12[21:30:00] Removing primary port from lagg (ix1 is new primary).. DONE
13Sleeping to give physical interface chance to reinit..
14[21:30:02] Checking if all interfaces has updated their macs..
15 ix0 OK
16 ix1 OK
17 lagg0 OK
18 vlan900 OK
19 vlan901 OK
20[21:30:07] Adding (slave) port ix0 back.. DONE
21[21:30:07] Checking if slave port mac has changed.. OK
22[21:30:12] Trying to change mac manually to 52:54:00:00:00:00.. DONE
23Sleeping to give physical interface chance to reinit..
24[21:30:14] Checking if all interfaces has updated their macs..
25 ix0 OK
26 ix1 OK
27 lagg0 OK
28 vlan900 OK
29 vlan901 OK
30[21:30:19] Cleaning up.. DONE
31[21:30:20] Checking original macs..
32 ix0 OK
33 ix1 OK

Pre-D3301 (r288654)

P83 Pre-D3301 (r288654)
117:57 [0] m@pktgen1 s ./checklagg.sh ix0 ix1
2Working on:
3 ix0 90:e2:ba:0f:a0:04
4 ix1 90:e2:ba:0f:a0:05
5Creating lagg lagg0 on top of ix0
6Testing inherited mac.. OK
7Creating vlan devices (vlan900, vlan901) on lagg0.. DONE
8Checking if interfaces has been created with proper macs..
9 vlan900 OK
10 vlan901 OK
11Adding (slave) port ix1 to lagg0.. DONE
12Checking if slave port mac has changed.. OK
13Removing primary port from lagg (ix1 is new primary).. DONE
14Checking if all interfaces has updated their macs..
15 ix0 OK
16 ix1 FAIL
17 lagg0 OK
18 vlan900 FAIL
19 vlan901 FAIL
20Adding (slave) port ix0 back.. DONE
21Checking if slave port mac has changed.. OK
22Trying to change mac manually to 52:54:00:00:00:00.. DONE
23Checking if all interfaces has updated their macs..
24 ix0 OK
25 ix1 FAIL
26 lagg0 OK
27 vlan900 OK
28 vlan901 OK
29Cleaning up.. DONE
30Checking original macs..
31 ix0 FAIL (was 90:e2:ba:0f:a0:04) now 52:54:00:00:00:00. Restoring.. DONE
32 ix1 FAIL (was 90:e2:ba:0f:a0:05) now 90:e2:ba:0f:a0:04. Restoring.. DONE

Test Plan

Scenario:
virtual machine has 2 interfaces (vlan11, vlan12) on top of vtnet0.
We're building lagg on top of there 2 interfaces (see checklagg.sh for exact details).
Kernel is hacked to

  • allow vlans on top of lagg (one-liner)
  • help dtrace probes by explicitly uninlining several functions
  • dummy arp_iflladdr() lladdr_event callback added to ease dtrace handling

Files:
http://static.ipfw.ru/patches/d4004_checklagg.sh
http://static.ipfw.ru/patches/d4004_arp.d
http://static.ipfw.ru/patches/d4004_kernel.diff

checklagg.sh is running along with arp.d script in other console. Their output is merged and some comments are provided.

TESTCASE 1, HEAD (r289939)

P85 Patched
1[22:02:02] Working on:
2 vlan11 52:54:11:11:11:11
3 vlan12 52:54:12:12:12:12
4[22:02:02] Creating lagg lagg0 on top of vlan11
5[22:02:02] Testing inherited mac.. OK
6
7CPU ID FUNCTION:NAME
8## Queing notification request for lagg mac change
9 0 60254 lagg_lladdr:entry Setting 'lagg0' mac to 52:54:11:11:11:11
10 0 60270 lagg_port_lladdr:entry Setting 'lagg0' mac to 52:54:11:11:11:11
11 1 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
12
13## No forced update (hence no down/up), just eventhandler
14 1 17375 arp_iflladdr:entry ARP handler called for 'lagg0' [mac 52:54:11:11:11:11]
15## Ignored set request (same lladdr for physical port)
16 0 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:11:11:11:11
17
18[22:02:07] Creating vlan devices (vlan900, vlan901) on lagg0.. DONE
19[22:02:07] Checking if interfaces has been created with proper macs..
20 vlan900 OK
21 vlan901 OK
22[22:02:12] Adding (slave) port vlan12 to lagg0.. DONE
23[22:02:12] Checking if slave port mac has changed.. OK
24
25## Enqueueing slave port lladdr change
26 0 60270 lagg_port_lladdr:entry Setting 'vlan12' mac to 52:54:11:11:11:11
27 1 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
28
29## Set mac + send notification
30 1 33396 if_setlladdr:entry Setting 'vlan12' mac to 52:54:11:11:11:11
31 1 17375 arp_iflladdr:entry ARP handler called for 'vlan12' [mac 52:54:11:11:11:11]
32
33[22:02:17] Removing primary port from lagg (vlan12 is new primary).. DONE
34[22:02:17] Checking if all interfaces has updated their macs..
35 vlan11 OK
36 vlan12 OK
37 lagg0 OK
38 vlan900 OK
39 vlan901 OK
40
41## Enqueue lladdr change for removed interface (ignored since lladdr was not changed)
42 0 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:11:11:11:11
43## Choose new mac and change lagg lladdr/enqueue change
44 0 60254 lagg_lladdr:entry Setting 'lagg0' mac to 52:54:12:12:12:12
45 0 60270 lagg_port_lladdr:entry Setting 'lagg0' mac to 52:54:12:12:12:12
46 0 60270 lagg_port_lladdr:entry Setting 'vlan12' mac to 52:54:12:12:12:12
47 1 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
48
49## Handling queue backward
50## Set mac + send notification for vlan12
51 1 33396 if_setlladdr:entry Setting 'vlan12' mac to 52:54:12:12:12:12
52 1 17375 arp_iflladdr:entry ARP handler called for 'vlan12' [mac 52:54:12:12:12:12]
53## Executing event handlers for lagg0 (not setting lladdr )
54 1 33396 if_setlladdr:entry Setting 'vlan901' mac to 52:54:12:12:12:12
55 1 33396 if_setlladdr:entry Setting 'vlan900' mac to 52:54:12:12:12:12
56 1 17375 arp_iflladdr:entry ARP handler called for 'lagg0' [mac 52:54:12:12:12:12]
57
58[22:02:22] Adding (slave) port vlan11 back.. DONE
59[22:02:22] Checking if slave port mac has changed.. OK
60
61## Enqueue lladdr change for newly-attached child interface
62 1 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:12:12:12:12
63 0 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
64
65## Set mac + send notification for vlan11
66 0 33396 if_setlladdr:entry Setting 'vlan11' mac to 52:54:12:12:12:12
67 0 17375 arp_iflladdr:entry ARP handler called for 'vlan11' [mac 52:54:12:12:12:12]
68
69[22:02:27] Trying to change mac manually to 52:54:00:00:00:00.. DONE
70[22:02:27] Checking if all interfaces has updated their macs..
71 vlan11 OK
72 vlan12 OK
73 lagg0 OK
74 vlan900 OK
75 vlan901 OK
76
77## Start from setlladdr -> lagg_stop() -> lagg_init()
78 2 33396 if_setlladdr:entry Setting 'lagg0' mac to 52:54:00:00:00:00
79## lagg_init() enqueues change for each port
80 2 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:00:00:00:00
81 2 60270 lagg_port_lladdr:entry Setting 'vlan12' mac to 52:54:00:00:00:00
82## taskqueue handler on cpu0
83 0 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
84
85## eventhnadler notification from if_setlladdr() on cpu2
86 2 33396 if_setlladdr:entry Setting 'vlan901' mac to 52:54:00:00:00:00
87## Set mac + send notification for vlan12
88 0 33396 if_setlladdr:entry Setting 'vlan12' mac to 52:54:00:00:00:00
89 0 17375 arp_iflladdr:entry ARP handler called for 'vlan12' [mac 52:54:00:00:00:00]
90## Set mac + send notification for vlan11
91 0 33396 if_setlladdr:entry Setting 'vlan11' mac to 52:54:00:00:00:00
92 0 17375 arp_iflladdr:entry ARP handler called for 'vlan11' [mac 52:54:00:00:00:00]
93## eventhnadler notification from if_setlladdr() on cpu0
94 2 33396 if_setlladdr:entry Setting 'vlan900' mac to 52:54:00:00:00:00
95 2 17375 arp_iflladdr:entry ARP handler called for 'lagg0' [mac 52:54:00:00:00:00]
96
97[22:02:32] Cleaning up.. DONE
98[22:02:32] Checking original macs..
99 vlan11 OK
100 vlan12 OK
101
102 1 60218 lagg_clone_destroy:entry time to shutdown for 'lagg0'
103
104## Enqueing delete for vlan11 (slave port)
105 1 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:11:11:11:11
106## Takqueue triggered, waiting for lock?
107 0 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
108
109## Enqueing delete for vlan12 (the only remaining master)
110 1 60270 lagg_port_lladdr:entry Setting 'vlan12' mac to 52:54:12:12:12:12
111## lagg_port_destroy(): no ports, set empty lladdr
112 1 60254 lagg_lladdr:entry Setting 'lagg0' mac to 00:00:00:00:00:00
113 1 60270 lagg_port_lladdr:entry Setting 'lagg0' mac to 00:00:00:00:00:00
114## iterating taskq queue backwards
115## Send notification for lagg0
116 0 17375 arp_iflladdr:entry ARP handler called for 'lagg0' [mac 00:00:00:00:00:00]
117## Set mac + send notification for vlan12
118 0 33396 if_setlladdr:entry Setting 'vlan12' mac to 52:54:12:12:12:12
119 0 17375 arp_iflladdr:entry ARP handler called for 'vlan12' [mac 52:54:12:12:12:12]
120## Set mac + send notification for vlan11
121 0 33396 if_setlladdr:entry Setting 'vlan11' mac to 52:54:11:11:11:11
122 0 17375 arp_iflladdr:entry ARP handler called for 'vlan11' [mac 52:54:11:11:11:11]
123## tasqueue_drain() for empty queue?
124 0 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'

TESTCASE 2, PATCHED VERSION

P85 Patched
1[22:02:02] Working on:
2 vlan11 52:54:11:11:11:11
3 vlan12 52:54:12:12:12:12
4[22:02:02] Creating lagg lagg0 on top of vlan11
5[22:02:02] Testing inherited mac.. OK
6
7CPU ID FUNCTION:NAME
8## Queing notification request for lagg mac change
9 0 60254 lagg_lladdr:entry Setting 'lagg0' mac to 52:54:11:11:11:11
10 0 60270 lagg_port_lladdr:entry Setting 'lagg0' mac to 52:54:11:11:11:11
11 1 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
12
13## No forced update (hence no down/up), just eventhandler
14 1 17375 arp_iflladdr:entry ARP handler called for 'lagg0' [mac 52:54:11:11:11:11]
15## Ignored set request (same lladdr for physical port)
16 0 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:11:11:11:11
17
18[22:02:07] Creating vlan devices (vlan900, vlan901) on lagg0.. DONE
19[22:02:07] Checking if interfaces has been created with proper macs..
20 vlan900 OK
21 vlan901 OK
22[22:02:12] Adding (slave) port vlan12 to lagg0.. DONE
23[22:02:12] Checking if slave port mac has changed.. OK
24
25## Enqueueing slave port lladdr change
26 0 60270 lagg_port_lladdr:entry Setting 'vlan12' mac to 52:54:11:11:11:11
27 1 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
28
29## Set mac + send notification
30 1 33396 if_setlladdr:entry Setting 'vlan12' mac to 52:54:11:11:11:11
31 1 17375 arp_iflladdr:entry ARP handler called for 'vlan12' [mac 52:54:11:11:11:11]
32
33[22:02:17] Removing primary port from lagg (vlan12 is new primary).. DONE
34[22:02:17] Checking if all interfaces has updated their macs..
35 vlan11 OK
36 vlan12 OK
37 lagg0 OK
38 vlan900 OK
39 vlan901 OK
40
41## Enqueue lladdr change for removed interface (ignored since lladdr was not changed)
42 0 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:11:11:11:11
43## Choose new mac and change lagg lladdr/enqueue change
44 0 60254 lagg_lladdr:entry Setting 'lagg0' mac to 52:54:12:12:12:12
45 0 60270 lagg_port_lladdr:entry Setting 'lagg0' mac to 52:54:12:12:12:12
46 0 60270 lagg_port_lladdr:entry Setting 'vlan12' mac to 52:54:12:12:12:12
47 1 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
48
49## Handling queue backward
50## Set mac + send notification for vlan12
51 1 33396 if_setlladdr:entry Setting 'vlan12' mac to 52:54:12:12:12:12
52 1 17375 arp_iflladdr:entry ARP handler called for 'vlan12' [mac 52:54:12:12:12:12]
53## Executing event handlers for lagg0 (not setting lladdr )
54 1 33396 if_setlladdr:entry Setting 'vlan901' mac to 52:54:12:12:12:12
55 1 33396 if_setlladdr:entry Setting 'vlan900' mac to 52:54:12:12:12:12
56 1 17375 arp_iflladdr:entry ARP handler called for 'lagg0' [mac 52:54:12:12:12:12]
57
58[22:02:22] Adding (slave) port vlan11 back.. DONE
59[22:02:22] Checking if slave port mac has changed.. OK
60
61## Enqueue lladdr change for newly-attached child interface
62 1 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:12:12:12:12
63 0 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
64
65## Set mac + send notification for vlan11
66 0 33396 if_setlladdr:entry Setting 'vlan11' mac to 52:54:12:12:12:12
67 0 17375 arp_iflladdr:entry ARP handler called for 'vlan11' [mac 52:54:12:12:12:12]
68
69[22:02:27] Trying to change mac manually to 52:54:00:00:00:00.. DONE
70[22:02:27] Checking if all interfaces has updated their macs..
71 vlan11 OK
72 vlan12 OK
73 lagg0 OK
74 vlan900 OK
75 vlan901 OK
76
77## Start from setlladdr -> lagg_stop() -> lagg_init()
78 2 33396 if_setlladdr:entry Setting 'lagg0' mac to 52:54:00:00:00:00
79## lagg_init() enqueues change for each port
80 2 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:00:00:00:00
81 2 60270 lagg_port_lladdr:entry Setting 'vlan12' mac to 52:54:00:00:00:00
82## taskqueue handler on cpu0
83 0 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
84
85## eventhnadler notification from if_setlladdr() on cpu2
86 2 33396 if_setlladdr:entry Setting 'vlan901' mac to 52:54:00:00:00:00
87## Set mac + send notification for vlan12
88 0 33396 if_setlladdr:entry Setting 'vlan12' mac to 52:54:00:00:00:00
89 0 17375 arp_iflladdr:entry ARP handler called for 'vlan12' [mac 52:54:00:00:00:00]
90## Set mac + send notification for vlan11
91 0 33396 if_setlladdr:entry Setting 'vlan11' mac to 52:54:00:00:00:00
92 0 17375 arp_iflladdr:entry ARP handler called for 'vlan11' [mac 52:54:00:00:00:00]
93## eventhnadler notification from if_setlladdr() on cpu0
94 2 33396 if_setlladdr:entry Setting 'vlan900' mac to 52:54:00:00:00:00
95 2 17375 arp_iflladdr:entry ARP handler called for 'lagg0' [mac 52:54:00:00:00:00]
96
97[22:02:32] Cleaning up.. DONE
98[22:02:32] Checking original macs..
99 vlan11 OK
100 vlan12 OK
101
102 1 60218 lagg_clone_destroy:entry time to shutdown for 'lagg0'
103
104## Enqueing delete for vlan11 (slave port)
105 1 60270 lagg_port_lladdr:entry Setting 'vlan11' mac to 52:54:11:11:11:11
106## Takqueue triggered, waiting for lock?
107 0 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'
108
109## Enqueing delete for vlan12 (the only remaining master)
110 1 60270 lagg_port_lladdr:entry Setting 'vlan12' mac to 52:54:12:12:12:12
111## lagg_port_destroy(): no ports, set empty lladdr
112 1 60254 lagg_lladdr:entry Setting 'lagg0' mac to 00:00:00:00:00:00
113 1 60270 lagg_port_lladdr:entry Setting 'lagg0' mac to 00:00:00:00:00:00
114## iterating taskq queue backwards
115## Send notification for lagg0
116 0 17375 arp_iflladdr:entry ARP handler called for 'lagg0' [mac 00:00:00:00:00:00]
117## Set mac + send notification for vlan12
118 0 33396 if_setlladdr:entry Setting 'vlan12' mac to 52:54:12:12:12:12
119 0 17375 arp_iflladdr:entry ARP handler called for 'vlan12' [mac 52:54:12:12:12:12]
120## Set mac + send notification for vlan11
121 0 33396 if_setlladdr:entry Setting 'vlan11' mac to 52:54:11:11:11:11
122 0 17375 arp_iflladdr:entry ARP handler called for 'vlan11' [mac 52:54:11:11:11:11]
123## tasqueue_drain() for empty queue?
124 0 60274 lagg_port_setlladdr:entry Taskqueue handler called for 'lagg0'

files:
F270951
F270953
F270955

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

melifaro updated this revision to Diff 9716.Oct 26 2015, 10:37 PM
melifaro retitled this revision from to Yet another if_lagg lladdr/eventhandler fix..
melifaro updated this object.
melifaro edited the test plan for this revision. (Show Details)
melifaro added reviewers: network, ae, araujo, glebius, hiren, hrs.
Herald added a subscriber: imp. · View Herald TranscriptOct 26 2015, 10:37 PM
melifaro updated this object.Oct 26 2015, 10:56 PM
melifaro edited the test plan for this revision. (Show Details)
melifaro updated this object.Oct 26 2015, 11:01 PM
melifaro updated this object.Oct 26 2015, 11:08 PM
melifaro mentioned this in F270951: arp.d.Oct 26 2015, 11:11 PM
melifaro mentioned this in F270953: checklagg.sh.
melifaro mentioned this in F270955: d4004_kernel.diff.
melifaro edited the test plan for this revision. (Show Details)Oct 26 2015, 11:16 PM
hrs accepted this revision.Oct 27 2015, 5:55 AM
hrs edited edge metadata.

Looks good to me. Thank you for your work!

This revision is now accepted and ready to land.Oct 27 2015, 5:55 AM
melifaro added a child revision: D4019: Use lladdr_event to propagate gratiotus arp..Oct 27 2015, 8:55 PM
melifaro updated this revision to Diff 9866.Nov 1 2015, 7:50 PM
melifaro edited edge metadata.
melifaro edited the test plan for this revision. (Show Details)

Do pre-commit sync and update some comments.

This revision now requires review to proceed.Nov 1 2015, 7:50 PM
Closed by commit rS290239: Fix lladdr change propagation for on vlans on top of it. (authored by melifaro). · Explain WhyNov 1 2015, 7:59 PM
This revision was automatically updated to reflect the committed changes.
melifaro mentioned this in D4102: Add link header precomputation for ethernet/infiniband. Make arp/ndp/bpf/flowtable use it..Dec 13 2015, 9:09 PM

Revision Contents
Changeset List

PathSize
head/
sys/
net/
4 lines
8 lines
129 lines

Diff 9867

View Options

head/sys/net/if.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause
*
* Copyright (c) 2023 Dave Cottlehuber <dch@FreeBSD.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <errno.h>
#include <sys/param.h>
#include <stdio.h>
#include <string.h>
#include <fetch.h>
#include <lua.h>
#include "lauxlib.h"
#include "lfetch.h"
/*
* Minimal implementation of libfetch
*/
static int
lfetch_parse_url(lua_State *L)
{
const char *url = luaL_checkstring(L, 1);
struct url *u;
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
lua_newtable(L);
lua_pushstring(L, u->scheme);
lua_setfield(L, -2, "scheme");
lua_pushstring(L, u->user);
lua_setfield(L, -2, "user");
lua_pushstring(L, u->pwd);
lua_setfield(L, -2, "password");
lua_pushstring(L, u->host);
lua_setfield(L, -2, "host");
/* if port is not explicitly set, it defaults to 0, not scheme */
kevansUnsubmitted
Done
Inline Actions

My gut reaction to this is that we probably shouldn't expose the port at all if it's not explicitly set, just leave it nil in this table to allow for more idiomatic usage where you can simply test the port as truthy before using it.f

kevans: My gut reaction to this is that we probably shouldn't expose the port at all if it's not…
lua_pushinteger(L, u->port);
lua_setfield(L, -2, "port");
/* doc is also known as the query string for http(s) */
lua_pushstring(L, u->doc);
lua_setfield(L, -2, "doc");
fetchFreeURL(u);
return (1);
}
static int
lfetch_get_url(lua_State *L) {
kevansUnsubmitted
Done
Inline Actions

Brace on its own line

kevans: Brace on its own line
const char *url = luaL_checkstring(L, 1);
const char *out = luaL_checkstring(L, 2);
struct url *u;
kevansUnsubmitted
Done
Inline Actions

This should move up above the blank line

kevans: This should move up above the blank line
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
FILE *file = fopen(out, "wb");
kevansUnsubmitted
Done
Inline Actions

IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of this function.

kevans: IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of…
if (file == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to open output file: %s", strerror(errno));
return 2;
}
FILE *fetch = fetchGet(u, "");
if (fetch == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to read from URL: %s", strerror(errno));
return 2;
kevansUnsubmitted
Done
Inline Actions

Leaks file

kevans: Leaks `file`
}
int chunk_size = 4096;
kevansUnsubmitted
Done
Inline Actions

Ditto for these declarations; chunk_size I'd probably just make a #define since we're not going to be altering it

kevans: Ditto for these declarations; chunk_size I'd probably just make a `#define` since we're not…
char buf[chunk_size];
size_t bytes;
while ((bytes = fread(buf, 1, chunk_size, fetch)) > 0) {
if (fwrite(buf, 1, bytes, file) != bytes) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to write to file: %s", strerror(errno));
fclose(file);
kevansUnsubmitted
Done
Inline Actions

Leaks fetch

kevans: Leaks `fetch`
dchAuthorUnsubmitted
Done
Inline Actions

If I fclose(fetch); here, this segfaults:

Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC-Rio

f.get_url("https://invalid.site/", "/tmp/i")

fish: Job 1, '/usr/libexec/flua $argv' terminated by signal SIGSEGV (Address boundary error)

(lldb) thr backtrace all
* thread #1, name = 'flua', stop reason = signal SIGSEGV
  * frame #0: 0x00001252697ad0f9
    frame #1: 0xffadb0abacff9a89
dch: If I `fclose(fetch);` here, this segfaults: Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC…
kevansUnsubmitted
Done
Inline Actions

The context for this one has changed, the comment was originally down in the error path in the loop. This branch looks fine

kevans: The context for this one has changed, the comment was originally down in the error path in the…
return 2;
}
}
fclose(fetch);
fclose(file);
fetchFreeURL(u);
lua_pushboolean(L, 1);
return 1;
}
static const struct
luaL_Reg fetchlib[] = {
{"parse_url", lfetch_parse_url},
{"get_url", lfetch_get_url},
{NULL, NULL}
};
int
luaopen_fetch(lua_State *L)
{
luaL_newlib(L, fetchlib);
return (1);
}
View Options

head/sys/net/if_lagg.h

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause
*
* Copyright (c) 2023 Dave Cottlehuber <dch@FreeBSD.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <errno.h>
#include <sys/param.h>
#include <stdio.h>
#include <string.h>
#include <fetch.h>
#include <lua.h>
#include "lauxlib.h"
#include "lfetch.h"
/*
* Minimal implementation of libfetch
*/
static int
lfetch_parse_url(lua_State *L)
{
const char *url = luaL_checkstring(L, 1);
struct url *u;
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
lua_newtable(L);
lua_pushstring(L, u->scheme);
lua_setfield(L, -2, "scheme");
lua_pushstring(L, u->user);
lua_setfield(L, -2, "user");
lua_pushstring(L, u->pwd);
lua_setfield(L, -2, "password");
lua_pushstring(L, u->host);
lua_setfield(L, -2, "host");
/* if port is not explicitly set, it defaults to 0, not scheme */
kevansUnsubmitted
Done
Inline Actions

My gut reaction to this is that we probably shouldn't expose the port at all if it's not explicitly set, just leave it nil in this table to allow for more idiomatic usage where you can simply test the port as truthy before using it.f

kevans: My gut reaction to this is that we probably shouldn't expose the port at all if it's not…
lua_pushinteger(L, u->port);
lua_setfield(L, -2, "port");
/* doc is also known as the query string for http(s) */
lua_pushstring(L, u->doc);
lua_setfield(L, -2, "doc");
fetchFreeURL(u);
return (1);
}
static int
lfetch_get_url(lua_State *L) {
kevansUnsubmitted
Done
Inline Actions

Brace on its own line

kevans: Brace on its own line
const char *url = luaL_checkstring(L, 1);
const char *out = luaL_checkstring(L, 2);
struct url *u;
kevansUnsubmitted
Done
Inline Actions

This should move up above the blank line

kevans: This should move up above the blank line
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
FILE *file = fopen(out, "wb");
kevansUnsubmitted
Done
Inline Actions

IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of this function.

kevans: IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of…
if (file == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to open output file: %s", strerror(errno));
return 2;
}
FILE *fetch = fetchGet(u, "");
if (fetch == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to read from URL: %s", strerror(errno));
return 2;
kevansUnsubmitted
Done
Inline Actions

Leaks file

kevans: Leaks `file`
}
int chunk_size = 4096;
kevansUnsubmitted
Done
Inline Actions

Ditto for these declarations; chunk_size I'd probably just make a #define since we're not going to be altering it

kevans: Ditto for these declarations; chunk_size I'd probably just make a `#define` since we're not…
char buf[chunk_size];
size_t bytes;
while ((bytes = fread(buf, 1, chunk_size, fetch)) > 0) {
if (fwrite(buf, 1, bytes, file) != bytes) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to write to file: %s", strerror(errno));
fclose(file);
kevansUnsubmitted
Done
Inline Actions

Leaks fetch

kevans: Leaks `fetch`
dchAuthorUnsubmitted
Done
Inline Actions

If I fclose(fetch); here, this segfaults:

Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC-Rio

f.get_url("https://invalid.site/", "/tmp/i")

fish: Job 1, '/usr/libexec/flua $argv' terminated by signal SIGSEGV (Address boundary error)

(lldb) thr backtrace all
* thread #1, name = 'flua', stop reason = signal SIGSEGV
  * frame #0: 0x00001252697ad0f9
    frame #1: 0xffadb0abacff9a89
dch: If I `fclose(fetch);` here, this segfaults: Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC…
kevansUnsubmitted
Done
Inline Actions

The context for this one has changed, the comment was originally down in the error path in the loop. This branch looks fine

kevans: The context for this one has changed, the comment was originally down in the error path in the…
return 2;
}
}
fclose(fetch);
fclose(file);
fetchFreeURL(u);
lua_pushboolean(L, 1);
return 1;
}
static const struct
luaL_Reg fetchlib[] = {
{"parse_url", lfetch_parse_url},
{"get_url", lfetch_get_url},
{NULL, NULL}
};
int
luaopen_fetch(lua_State *L)
{
luaL_newlib(L, fetchlib);
return (1);
}
View Options

head/sys/net/if_lagg.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause
*
* Copyright (c) 2023 Dave Cottlehuber <dch@FreeBSD.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <errno.h>
#include <sys/param.h>
#include <stdio.h>
#include <string.h>
#include <fetch.h>
#include <lua.h>
#include "lauxlib.h"
#include "lfetch.h"
/*
* Minimal implementation of libfetch
*/
static int
lfetch_parse_url(lua_State *L)
{
const char *url = luaL_checkstring(L, 1);
struct url *u;
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
lua_newtable(L);
lua_pushstring(L, u->scheme);
lua_setfield(L, -2, "scheme");
lua_pushstring(L, u->user);
lua_setfield(L, -2, "user");
lua_pushstring(L, u->pwd);
lua_setfield(L, -2, "password");
lua_pushstring(L, u->host);
lua_setfield(L, -2, "host");
/* if port is not explicitly set, it defaults to 0, not scheme */
kevansUnsubmitted
Done
Inline Actions

My gut reaction to this is that we probably shouldn't expose the port at all if it's not explicitly set, just leave it nil in this table to allow for more idiomatic usage where you can simply test the port as truthy before using it.f

kevans: My gut reaction to this is that we probably shouldn't expose the port at all if it's not…
lua_pushinteger(L, u->port);
lua_setfield(L, -2, "port");
/* doc is also known as the query string for http(s) */
lua_pushstring(L, u->doc);
lua_setfield(L, -2, "doc");
fetchFreeURL(u);
return (1);
}
static int
lfetch_get_url(lua_State *L) {
kevansUnsubmitted
Done
Inline Actions

Brace on its own line

kevans: Brace on its own line
const char *url = luaL_checkstring(L, 1);
const char *out = luaL_checkstring(L, 2);
struct url *u;
kevansUnsubmitted
Done
Inline Actions

This should move up above the blank line

kevans: This should move up above the blank line
u = fetchParseURL(url);
if (u == NULL) {
lua_pushnil(L);
lua_pushstring(L, "Failed to parse URL");
return (2);
}
FILE *file = fopen(out, "wb");
kevansUnsubmitted
Done
Inline Actions

IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of this function.

kevans: IIRC style(9) still doesn't allow declarations mid-block, it would need to be at the top of…
if (file == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to open output file: %s", strerror(errno));
return 2;
}
FILE *fetch = fetchGet(u, "");
if (fetch == NULL) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to read from URL: %s", strerror(errno));
return 2;
kevansUnsubmitted
Done
Inline Actions

Leaks file

kevans: Leaks `file`
}
int chunk_size = 4096;
kevansUnsubmitted
Done
Inline Actions

Ditto for these declarations; chunk_size I'd probably just make a #define since we're not going to be altering it

kevans: Ditto for these declarations; chunk_size I'd probably just make a `#define` since we're not…
char buf[chunk_size];
size_t bytes;
while ((bytes = fread(buf, 1, chunk_size, fetch)) > 0) {
if (fwrite(buf, 1, bytes, file) != bytes) {
lua_pushnil(L);
lua_pushfstring(L, "Failed to write to file: %s", strerror(errno));
fclose(file);
kevansUnsubmitted
Done
Inline Actions

Leaks fetch

kevans: Leaks `fetch`
dchAuthorUnsubmitted
Done
Inline Actions

If I fclose(fetch); here, this segfaults:

Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC-Rio

f.get_url("https://invalid.site/", "/tmp/i")

fish: Job 1, '/usr/libexec/flua $argv' terminated by signal SIGSEGV (Address boundary error)

(lldb) thr backtrace all
* thread #1, name = 'flua', stop reason = signal SIGSEGV
  * frame #0: 0x00001252697ad0f9
    frame #1: 0xffadb0abacff9a89
dch: If I `fclose(fetch);` here, this segfaults: Lua 5.4.4 Copyright (C) 1994-2022 Lua.org, PUC…
kevansUnsubmitted
Done
Inline Actions

The context for this one has changed, the comment was originally down in the error path in the loop. This branch looks fine

kevans: The context for this one has changed, the comment was originally down in the error path in the…
return 2;
}
}
fclose(fetch);
fclose(file);
fetchFreeURL(u);
lua_pushboolean(L, 1);
return 1;
}
static const struct
luaL_Reg fetchlib[] = {
{"parse_url", lfetch_parse_url},
{"get_url", lfetch_get_url},
{NULL, NULL}
};
int
luaopen_fetch(lua_State *L)
{
luaL_newlib(L, fetchlib);
return (1);
}