How to determine interface from within handler?

[rgooch]

I'm using the ListenAndServe method to register my handler, so the same handler will be used for responding for all interfaces. How do I determine (from within the handler) which interface a DHCP packet was received on? I looked at the documentation and this information does not seems to be exposed.

[krolaw]

You need to create your own Conn utilising "golang.org/x/net/ipv4". Look at conn/filter.go for inspiration. It detects which interface the packet came from and ignores it if it doesn't match what it's looking for.

[rgooch]

That creates other problems. I have a mix or real EtherNet interfaces and bridges (which don't have an IP address). If I don't specify the host address to bind to (i.e. default to 0.0.0.0), I can only create a listening packet for the first interface I bound to. I can't specify different IP addresses to bind to because I have more interfaces (bridges) than IP addresses.

[krolaw]

You still want to bind to 0.0.0.0, you just use "golang.org/x/net/ipv4" to detect what interface the packet came from, like conn/NewUDP4FilterListener does, and set laddr param to 0.0.0.0. I had to use this technique, when I couldn't bind to given interface.

[rgooch]

Are you sure? I'm currently using ListenAndServeIf and I get failures binding to port 67. Yeah, I know it's deprecated, but it was the quick path forward to try per-interface serving.

[krolaw]

Sorry I meant: 0.0.0.0:67. Pretty sure, used the technique in a router type box...

[rgooch]

I'm thinking you must have done something different than calling ListenAndServeIf for each interface. Did you perhaps call net.ListenPacket once and then re-use that for serving on each interface?
It would be nice if the package provided convenience code to make this easier. Ideally, ListenAndServe would do all the magic to ensure that reply packets are returned on the same interface as the request packets were received on.

[krolaw]

This is exactly why ListenAndServeIf was deprecated. Use a conn with Serve. The best case is to use conn/NewUDP4BoundListener for each interface you have. You can use the same handler if appropriate. If you can't use NewUDP4BoundListener (i.e. not linux), you'll need to create your own conn, probably based on NewUDP4FilterListener - where you would likely bind to 0.0.0.0:67, unless you have some specialised syscalls for your OS.

[krolaw]

I was using something like NewUDP4FilterListener, before NewUDP4BoundListener was developed on a router like pc. One of the largest challenges is that go's net lib doesn't support binding on a given interface in broadcast mode - otherwise ListenAndServeIf would have been able to do exactly what you want. Alas, that's not to be.

[rgooch]

In my case, my main concern is to ensure that reply packets go back to the requestor. I'm giving up for now having the ServeDHCP method know the interface name, as this would require a goroutine for each interface calling Serve with a different Handler and a bound listener for each interface. This seems a bit heavyweight. What I've gotten working is to use a single listener and have a map of interface indices which are OK. Here are some code extracts in case you're curious:

type serveIfConn struct {
	ifIndices map[int]struct{}
	conn      *ipv4.PacketConn
	cm        *ipv4.ControlMessage
}

	if len(interfaceNames) < 1 {
		logger.Debugln(0, "Starting DHCP server on all broadcast interfaces")
		if interfaces, err := net.Interfaces(); err != nil {
			return nil, err
		} else {
			for _, iface := range interfaces {
				if iface.Flags&net.FlagBroadcast != 0 {
					interfaceNames = append(interfaceNames, iface.Name)
				}
			}
		}
	} else {
		logger.Debugln(0, "Starting DHCP server on interfaces: "+
			strings.Join(interfaceNames, ","))
	}
	serveConn := &serveIfConn{
		ifIndices: make(map[int]struct{}, len(interfaceNames)),
	}
	for _, interfaceName := range interfaceNames {
		if iface, err := net.InterfaceByName(interfaceName); err != nil {
			return nil, err
		} else {
			serveConn.ifIndices[iface.Index] = struct{}{}
		}
	}
	listener, err := net.ListenPacket("udp4", ":67")
	if err != nil {
		return nil, err
	}
	pktConn := ipv4.NewPacketConn(listener)
	if err := pktConn.SetControlMessage(ipv4.FlagInterface, true); err != nil {
		listener.Close()
		return nil, err
	}
	serveConn.conn = pktConn
	go func() {
		if err := dhcp.Serve(serveConn, dhcpServer); err != nil {
			logger.Println(err)
		}
	}()

func (s *serveIfConn) ReadFrom(b []byte) (n int, addr net.Addr, err error) {
	for {
		n, s.cm, addr, err = s.conn.ReadFrom(b)
		if err != nil || s.cm == nil {
			break
		}
		if _, ok := s.ifIndices[s.cm.IfIndex]; ok {
			break
		}
	}
	return
}

func (s *serveIfConn) WriteTo(b []byte, addr net.Addr) (n int, err error) {
	s.cm.Src = nil
	return s.conn.WriteTo(b, s.cm, addr)
}

[rgooch]

It would be nice if Serve would support a ServeConn with an alternate reading method which returns a blob of data that can be passed to an alternate method for Handler.

[rgooch]

Another approach would be to define a new ExtendedHandler interface which adds a ServeExtendedDHCP method. That method would receive an extra parameter which is a pointer to a struct with information such as the interface index. ListenAndServe would do something similar to what I'm doing now: create a single listener, wrap it and then use it. This would ensure - with no code changes by users - that reply packets are sent to the correct interface. Users would have the option of also getting the interface index in their ExtendedHandler.
Would you accept a pull request that does something like this?

[krolaw]

No, but feel free to fork. Currently, this can be accomplished by creating a link between your conn and your handler. Ideally, I think one interface per goroutine is cleaner as goroutines are designed to be cheap (go web servers have 1000s).

[rgooch]

I was hoping to avoid having to fork, as it means I'd have to maintain the fork, the feature sets would diverge and there would be fragmentation. I think there's a quality of implementation argument to at least ensure that ListenAndServe will send responses back on the right interface.

[krolaw]

Actually, you don't need to fork, you could create a new pkg with just your ExtendedHandler in it and have it import dhcp4. That way you only need to maintain your code, and it benefits from changes in the dhcp4, rather than you have to keep pulling through.

As a maintainer of such a project, you will get to choose whether changes sent in by users are appropriate for your project. You may need to disappoint some people because you do not believe in their approach. You might be right, you might be wrong; but someone has to choose and that person is you because you maintain the project.

If you do go ahead, please add a link of your project to this issue.

[krolaw]

Closing as issue is directly related to deprecated code.