Unicast to the giaddr address when non zero:

[jacobweinstock]

Description

When the DHCP header giaddr is set, we must unicast responses to that address. DHCP RFC section 4.1, page 22:

"If the 'giaddr' field in a DHCP message from a client is non-zero, the server sends any return messages to the 'DHCP server' port on the BOOTP relay agent whose address appears in 'giaddr'."

In addition to not being to spec, sending responses to the direct peer causes that a relay agent that sent a unicast request doesn't receive the expected unicast response if there is more than one relay in the mix. This is observed with the v0.4.2 Tink stack helm chart as we deploy a DHCP relay as part of the stack.

Current Traffic flow for the Helm chart deployment. Traffic from the relay container (inside the tink stack deployment) doesn't unicast responses to the network's relay agent.

                                                                                ▲                                              
                                                                                │                                              
                                                                                │                                              
                                                                            broadcast                                          
                                                                                │                                              
                                                                                │                                              
┌───────────────┐                   ┌───────────────┐                   ┌───────────────┐                     ┌───────────────┐
│    machine    │◀─────broadcast───▶│     relay     │──────unicast─────▶│relay container│◀──────unicast──────▶│     Smee      │
└───────────────┘                   └───────────────┘                   └───────────────┘                     └───────────────┘

Why is this needed

Fixes: #tinkerbell/smee#382

How Has This Been Tested?

Manually testing using the v0.4.2 helm chart.

Traffic will now make it back to the network's relay agent:

                                                          ┌───────────────┐                 
                                             ┌────────────│     Smee      │◀───────┐        
                                             │            └───────────────┘        │        
                                          unicast                               unicast     
                                             │                                     │        
                                             ▼                                     │        
┌───────────────┐                    ┌───────────────┐                     ┌───────────────┐
│    machine    │◀─────broadcast────▶│     relay     │───────unicast──────▶│relay container│
└───────────────┘                    └───────────────┘                     └───────────────┘

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

• updated the documentation and/or roadmap (if required)
• added unit or e2e tests
• provided instructions on how to upgrade

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (b3c8a31) 93.02% compared to head (5c684d3) 92.82%.

❗ Current head 5c684d3 differs from pull request most recent head 07e0e03. Consider uploading reports for the commit 07e0e03 to get more accurate results

Files	Patch %	Lines
handler/reservation/handler.go	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #67      +/-   ##
==========================================
- Coverage   93.02%   92.82%   -0.21%     
==========================================
  Files          11       11              
  Lines         817      822       +5     
==========================================
+ Hits          760      763       +3     
- Misses         37       38       +1     
- Partials       20       21       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.