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

portfwd: Read bytes ignored on is io.EOF returned from ReadFrom #2970

Closed
nirs opened this issue Dec 3, 2024 · 0 comments · Fixed by #2971
Closed

portfwd: Read bytes ignored on is io.EOF returned from ReadFrom #2970

nirs opened this issue Dec 3, 2024 · 0 comments · Fixed by #2971
Assignees
Milestone

Comments

@nirs
Copy link
Member

nirs commented Dec 3, 2024

Based on PacketCon.ReadFrom docs:

        // ReadFrom reads a packet from the connection,
	// copying the payload into p. It returns the number of
	// bytes copied into p and the return address that
	// was on the packet.
	// It returns the number of bytes read (0 <= n <= len(p))
	// and any error encountered. Callers should always process
	// the n > 0 bytes returned before considering the error err.
	// ReadFrom can be made to time out and return an error after a
	// fixed time limit; see SetDeadline and SetReadDeadline.

https://pkg.go.dev/net#PacketConn

	g.Go(func() error {
		buf := make([]byte, 65507)
		for {
			n, addr, err := conn.ReadFrom(buf)
			if errors.Is(err, io.EOF) {
				return nil
			}
			if err != nil {
				return err
			}
			msg := &api.TunnelMessage{
				Id:            id + "-" + addr.String(),
				Protocol:      "udp",
				GuestAddr:     guestAddr,
				Data:          buf[:n],
				UdpTargetAddr: addr.String(),
			}
			if err := stream.Send(msg); err != nil {
				return err
			}
		}
	})

It seems that we need to do:

  • read
  • send message with the read bytes
  • handle the err - not clear if it can be io.EOF

Originally posted by @tamird in #2969 (comment)

@nirs nirs changed the title portfwd: Read bytes ignored on is io.EOF portfwd: Read bytes ignored on is io.EOF returned from ReadFrom Dec 3, 2024
nirs added a commit to nirs/lima that referenced this issue Dec 3, 2024
PacketCon.ReadFrom documentation says[1]:

> Callers should always process the n > 0 bytes returned before
> considering the error err.

But we handled err first, and dropped the last read bytes. Fixed by
sending a message if n > 0, and handling the error after the send.

stream.Recv() does is not documented, but the trivial implementation
ensure that we get nil message on any error. Add comment to make it more
clear.

[1] https://pkg.go.dev/net#PacketConn

Fixes lima-vm#2970

Signed-off-by: Nir Soffer <nirsof@gmail.com>
@nirs nirs added this to the v1.0.3 milestone Dec 3, 2024
@nirs nirs self-assigned this Dec 3, 2024
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 a pull request may close this issue.

1 participant