Add vlan support to embedded script: #44
Conversation
This allows creating an interface with a vlan tag based off of DHCP option 43, suboption 116. Signed-off-by: Jacob Weinstock <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #44 +/- ##
=======================================
Coverage 97.10% 97.10%
=======================================
Files 4 4
Lines 415 415
=======================================
Hits 403 403
Misses 8 8
Partials 4 4 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -6,4 +6,21 @@ echo Welcome to Neverland! | |||
| # Allow the operator to drop to a shell | |||
| prompt --key 0x02 --timeout 2000 Press Ctrl-B for the iPXE command line... && shell || | |||
|
|
|||
| set vlan-id ${43.116:string} | |||
There was a problem hiding this comment.
I'm confused. If this is the script embedded in our ipxe binaries, at this point I would have expected that DHCP hasn't happened yet from the perspective of ipxe... How would that value have been populated at this point?
Do we need to split out an explicit dhcp from the autoboot part?
There was a problem hiding this comment.
That's a great point. I will add a comment above this value to explain. My understanding of iPXE is that "the original vendor PXE DHCP request [is] available within [the chainloaded] UEFI iPXE [and] BIOS iPXE" -- ref. This bears out in my testing, jfyi.
There was a problem hiding this comment.
Thank you for adding the comment in there!
When chainloading from an original vendor PXE the DHCP options it received are available to the chainloaded binary. This allows us to pull out option 43.116 and use it to set up the vlan interface before autoboot-ing which does another DHCP PXE request. Signed-off-by: Jacob Weinstock <[email protected]>
| @@ -6,4 +6,21 @@ echo Welcome to Neverland! | |||
| # Allow the operator to drop to a shell | |||
| prompt --key 0x02 --timeout 2000 Press Ctrl-B for the iPXE command line... && shell || | |||
|
|
|||
| set vlan-id ${43.116:string} | |||
There was a problem hiding this comment.
Thank you for adding the comment in there!
| :boot-with-vlan | ||
| set idx:int32 0 | ||
| # Find the first interface that is configured | ||
| :loop isset ${net${idx}/mac} && goto loop_done || |
There was a problem hiding this comment.
Did you by any chance test this on a machine with multiple NICs and verify that it behaves correctly booting from all of them?
To my eye I'd expect this to find the first NIC with a valid MAC address.
I wonder if there are any other hints from the prior DHCP request that could be helpful here?
I'm mostly flagging this as a place to look for bugs later. I think we should land this so that further testing can proceed. It's already a big step up.
There was a problem hiding this comment.
another great point! yes, i did test with multiple network interfaces. I found that when the PXE firmware would request PXE it would show trying against, for example, net0, net1, etc but once our iPXE binary was loaded, i found that only net0 was available regardless of the firmware PXE net number. That was my very specific testing.
There was a problem hiding this comment.
lgtm, I trust your testing @jacobweinstock and @nshalman's 👍 too :D
Description
This allows creating an interface with a vlan tag based off of DHCP option 43, suboption 116. This will not change the existing behavior. The vlan interface is only created if DHCP option 43.116 is set. This PR is the start of the implementation of proposal 28.
Why is this needed
Fixes: #
How Has This Been Tested?
I have manually test this.
How are existing users impacted? What migration steps/scripts do we need?
No impact.
Checklist:
I have: