Skip to content

BFD: consider re-using UdpSocket in egress thread instead of creating a new one for every packet #655

@taspelund

Description

@taspelund

In mgd/src/bfd_admin.rs, we are creating a new UdpSocket for every iteration of a packet being sent (match rx.recv() pulls out a Result<(IpAddr, packet::Control)>):

fn egress(
    rx: Receiver<(IpAddr, packet::Control)>,
    local: IpAddr,
    src_port: u16,
    dst_port: u16,
    log: Logger,
) {
    spawn(move || {
        loop {
            let (addr, pkt) = match rx.recv() {
                Ok(result) => result,
                Err(e) => {
                    bfd_log!(log, warn, "udp egress channel closed: {e}";
                        "local" => format!("{local}"),
                        "src_port" => format!("{src_port}"),
                        "dst_port" => format!("{dst_port}"),
                        "error" => format!("{e}")
                    );
                    break;
                }
            };

            let sk = match UdpSocket::bind(SocketAddr::new(local, src_port)) {
                Err(e) => {
                    bfd_log!(log, error, "failed to create tx socket: {e}";
                        "local" => format!("{local}"),
                        "src_port" => format!("{src_port}"),
                        "dst_port" => format!("{dst_port}"),
                        "error" => format!("{e}")
                    );
                    continue;
                }
                Ok(sk) => sk,
            };

            let sa = SocketAddr::new(addr, dst_port);
            if let Err(e) = sk.send_to(&pkt.to_bytes(), sa) {
                bfd_log!(log, error, "udp send error: {e}";
                    "local" => format!("{local}"),
                    "src_port" => format!("{src_port}"),
                    "dst_port" => format!("{dst_port}"),
                    "message" => "control",
                    "message_contents" => format!("{pkt}"),
                    "error" => format!("{e}")
                );
            }
        }
    });
}

This requires context switching into the kernel for a syscall on every iteration of the loop, which is less than ideal... especially at the subsecond frequency that BFD packets are sent.

We should really consider creating just a single UdpSocket for the session and creating a new UdpSocket if the old one breaks.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bfdBidirectional Forwarding DetectionmgdMaghemite daemonneeds testingwant
    No fields configured for Enhancement.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions