exporter: limit ser2net port range

[benjamin1313]

This allows for specifying a start port and a range up from that to be used when looking for a free port to use with ser2net when creating a networkserialport resource. This can be helpful on networks where access to ports on the network is limited.

Currently this feature is planed for use in a labgrid setup where the exporter and duts are placed on a network separate from the main network the developers are on and there are restrictions on the tcp ports. Being able to specify what port(s) labgrid can use makes it easier to manage and keep track of what ports are open between the two networks.

Checklist

• Documentation for the feature
• Tests for the feature
• PR has been tested
• Man pages have been regenerated

[codecov]

Codecov Report

❌ Patch coverage is 21.42857% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.3%. Comparing base (2daa13d) to head (85b2413).
⚠️ Report is 136 commits behind head on master.

Files with missing lines	Patch %	Lines
labgrid/util/helper.py	23.0%	10 Missing ⚠️
labgrid/remote/exporter.py	0.0%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #1743     +/-   ##
========================================
- Coverage    45.4%   45.3%   -0.1%     
========================================
  Files         172     172             
  Lines       13503   13514     +11     
========================================
+ Hits         6131    6132      +1     
- Misses       7372    7382     +10     

Flag	Coverage Δ
3.10	45.3% <21.4%> (-0.1%)	⬇️
3.11	45.3% <21.4%> (-0.1%)	⬇️
3.12	45.3% <21.4%> (-0.1%)	⬇️
3.13	45.3% <21.4%> (-0.1%)	⬇️
3.9	45.4% <21.4%> (-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.

[Emantor]

I think it's usually more common to have a STARTPORT:ENDPORT definition instead of the PORT:RANGE used here, e.g. 4000:5000 instead of 4000:1000. This might also simplify the code, what do you think?

[Bastian-Krause]

I think this should not be limited to ser2net. If the exporter needs to bind anything else in the future, it could use that range as well. So a more generic name would be good.

I'm not a huge fan of adding more environment variables, a CLI argument for labgrid-exporter would be a better fit. Especially since --isolated exists for similar cases where the exporter/DUT network setup is configured in a special way. So maybe --port-range=4000-5000?

[benjamin1313]

I think it's usually more common to have a STARTPORT:ENDPORT definition instead of the PORT:RANGE used here, e.g. 4000:5000 instead of 4000:1000. This might also simplify the code, what do you think?

Yeah I see what you mean and will look into it.

I think this should not be limited to ser2net. If the exporter needs to bind anything else in the future, it could use that range as well. So a more generic name would be good.

Do you mean a global range for everything else that also needs a port?
Where I'm currently working on a Labgrid setup I'm not in charge of the network and I think giving Labgrid a range of ports to use for whatever is going to be a hard sell to the IT department in charge of the network.
Being able specify which ports are needed for what tool, service etc. is preferred.

[Bastian-Krause]

Letting the exporter bind something is a rather special case. Apart from USBSerialPort/RawSerialPort exports running ser2net, I suppose that would only happen for a new kind of resource (if ever). As long as you don't export such a resource, no new binds will happen.

That being said, maybe it makes more sense (in such a regulated environment) to run ser2net outside labgrid and only export NetworkSerialPorts? If you're worried about misconfigurations, you could even configure your labgrid-exporter.service with SocketBindDeny=any.

[jluebbe]

Please keep in mind that without allocating a new port on acquire, you have the risk that an out of date client connects to the port it knows even if it no longer has a lock.

[jluebbe]

I don't think we should add a ser2net-specific override. Also, we need to make reasonably sure that a new port is chosen when the the resource is acquired again. So: a global port range of at least 1000 ports, used automatically for all calls of get_free_port().

@benjamin1313 Would you be interested in reworking this PR accordingly? If not, I'd close it for now. @Emantor @Bastian-Krause, what do you think?

[benjamin1313]

@jluebbe yes i will look into it.

Is is currently understood what needs to be done is.
use --port-range=4000-5000 instead of environment variable.
use start and stop port instead of start and number of ports to be used.
other resources calling get_free_port() should also use these ports.
and ´get_free_port()´ should choose a port at random in the range given in order to avoid the risk an out of date client connects to the port?

[AiyionPrime]

@benjamin1313 I'm interested in a solution as well and have a few hours to spare on this topic.

I think I understand the requirements and will open a new PR this weekend if you don't object?

[AiyionPrime]

I don't think we should add a ser2net-specific override. [...]

Originally posted by @jluebbe in #1743 (comment)

Looking at it, the strictest approach to not be (ser2net-)specific would be to not implement this feature at all and instead reduce the ephemeral port range to the desired limits in order for the regular bind(('', 0)) to pick it up.

I'll give it a shot tomorrow and find whether this more radical approach limits the systems usability.

[benjamin1313]

@benjamin1313 I'm interested in a solution as well and have a few hours to spare on this topic.
I think I understand the requirements and will open a new PR this weekend if you don't object?

i don't.