Skip to content

tftp: add external_ip setting #1678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aparcar
Copy link
Contributor

@aparcar aparcar commented Jul 2, 2025

Just as with the external (path) setting, it's helpful for the DUT to know from which IP the server offers firmware.

Description

Checklist

  • Documentation for the feature
  • Tests for the feature
  • The arguments and description in doc/configuration.rst have been updated
  • Add a section on how to use the feature to doc/usage.rst
  • Add a section on how to use the feature to doc/development.rst
  • PR has been tested
  • Man pages have been regenerated

Just as with the `external` (path) setting, it's helpful for the DUT to know
from which IP the server offers firmware.

Signed-off-by: Paul Spooren <[email protected]>
Copy link

codecov bot commented Jul 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.8%. Comparing base (d6d20d7) to head (e73d676).
Report is 19 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1678   +/-   ##
======================================
  Coverage    55.7%   55.8%           
======================================
  Files         170     170           
  Lines       13393   13395    +2     
======================================
+ Hits         7473    7475    +2     
  Misses       5920    5920           
Flag Coverage Δ
3.10 55.8% <100.0%> (+<0.1%) ⬆️
3.11 55.8% <100.0%> (+<0.1%) ⬆️
3.12 55.8% <100.0%> (+<0.1%) ⬆️
3.13 55.7% <100.0%> (+<0.1%) ⬆️
3.9 55.8% <100.0%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@aparcar aparcar changed the title WIP tftp: add external_ip setting tftp: add external_ip setting Jul 2, 2025
@aparcar aparcar marked this pull request as ready for review July 2, 2025 13:20
@aparcar
Copy link
Contributor Author

aparcar commented Jul 2, 2025

I'm running this already in my lab setup and it seems to work just fine.

@aparcar
Copy link
Contributor Author

aparcar commented Jul 2, 2025

@Bastian-Krause
Copy link
Member

Why can't the DUT simply use the host attribute instead?

@Bastian-Krause Bastian-Krause added the needs author info Requires more information from the PR/Issue author label Jul 4, 2025
@aparcar
Copy link
Contributor Author

aparcar commented Jul 5, 2025

Using a proxied setup, the host value is something like hostname-foobar, which doesn't help the tftp DUT at all. The specified IP is instead handed over via a setenv call in a custom strategy

@aparcar
Copy link
Contributor Author

aparcar commented Jul 16, 2025

@Bastian-Krause any further comments? My code looks like this right now and works just fine

elif status == Status.uboot:
            self.transition(Status.off)
            self.target.activate(self.tftp)
            self.target.activate(self.console)

            staged_file = self.tftp.stage(self.target.env.config.get_image_path("root"))
            tftp_server_ip = self.target.get_resource(
                RemoteTFTPProvider, wait_avail=False
            ).external_ip

            self.power.cycle()
            # interrupt uboot

            self.uboot.init_commands = (
                f"setenv bootfile {staged_file}",
            ) + self.uboot.init_commands

            if tftp_server_ip:
                tftp_dut_ip = ipaddress.ip_address(tftp_server_ip) + 1
                self.uboot.init_commands = (
                    f"setenv serverip {tftp_server_ip}",
                    f"setenv ipaddr {tftp_dut_ip}",
                ) + self.uboot.init_commands

            self.target.activate(self.uboot)

@Emantor Emantor removed the needs author info Requires more information from the PR/Issue author label Jul 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants