Skip to content
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

Server should respond over same socker UDP socket. #18

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

Conversation

zbleness
Copy link

@zbleness zbleness commented May 3, 2022

When running in docker (--net=bridge) on Ubuntu 22.04
we hit a problem where the TFTP responses were not
routed properly. After some investigation we concluded
that the response got lost because it was coming from a
different (random) host:port.

The patch in question re-uses the server socket for
the UDP response so that reverse NAT treversal is
possible in stricter network topologies.

When running in docker (--net=bridge) on Ubuntu 22.04
we hit a problem where the TFTP responses were not
routed properly. After some investigation we concluded
that the response got lost because it was coming from a
different (random) host:port.

The patch in question re-uses the server socket for
the UDP response so that reverse NAT treversal is
possible in stricter network topologies.
@sirMackk
Copy link
Owner

sirMackk commented Jul 1, 2022

Hi @zbleness , thanks for the PR and sorry for the delay. I'll look this over next week and go about merging/releasing :).

@zbleness
Copy link
Author

zbleness commented Jul 4, 2022

I forgot about this patch as well @sirMackk . Even though it looked promising at start we could not get our e2e tests to work with py3tftp so we ended up switching to ptftplib. I guess what I'm trying to say is that we are not sure whether this patch improves your project or not as we did not follow it through. So please be careful before merging.

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.

2 participants