control packet support in test::Proxy

[GabrielPerezCSDev]

#772

Summary

This PR adds support for control packet handling in Proxy while maintaining backward compatibility with existing tests.

Changes Implemented

• Extended Proxy class to handle control packets in addition to source and repair packets.
• Added an overloaded constructor to allow optional control packet support while preserving the existing test proxy API.
• Updated write() to use control queue integrated into its current use of source/repair queues to forward control packets.
• Existing tests and functionality is unaffected

Development Workflow Compliance

• Targeted develop branch, per project guidelines.
• Applied code formatting (scons -Q fmt) before submission.
• Preserved backward compatibility with existing tests.

Notes

• Control packet support is now integrated into Proxy, following the same queuing mechanism as source and repair packets.

[gavv]

Thanks for the patch! Some comments.

[gavv]

The big part missing from the PR is bi-directional proxying of the control packets - control packets are sent in two directions, unlike source and repair packets. Actually that's the most important part of this feature (the rest is more straightforward), please read issue text in #772.

[GabrielPerezCSDev]

Sorry for the late reply thank you for the updates. I will get right on it!

[GabrielPerezCSDev]

#772

Overview

Updated and modified the PR to adhere to the requested changes. Please let me know if there are any issues and thank you in advance!

Implementation Details

Constructor Changes

• Added optional control endpoint parameter (NULL when not needed)
• Removed duplicate overloaded constructor
• Updated the constructor to handle setting up the control packets when not NULL

Control Packet Handling

• Implemented bidirectional flow for control packets
  • Sender → Receiver: Similar to source/repair packets
  • Receiver → Sender: New handling with appropriate address adjustments
• Control packets processed separately from FEC block structure
• Control packets are never dropped unlike source packets

Accessor Method

• Added CHECK for NULL in control_endpoint() accessor for early failure detection
• Safe handling in destructor for NULL control endpoint

Testing

All tests have been updated to use the new constructor signature and pass NULL for the control endpoint parameter when not needed.

[gavv]

The part of ctor before netio::NetworkLoop::PortHandle send_port = NULL became quite hard to read, I suggest to reorganize it a bit and group configuration by type, like this:

...

// receiver's source endpoint
roc_protocol source_proto;
int source_port = 0;
CHECK(roc_endpoint_get_protocol(receiver_source_endp, &source_proto) == 0);
CHECK(roc_endpoint_get_port(receiver_source_endp, &source_port) == 0);
CHECK(receiver_source_endp_.set_host_port(address::Family_IPv4, "127.0.0.1",
                                          source_port));

// receiver's repair endpoint
roc_protocol repair_proto;
int repair_port = 0;
CHECK(roc_endpoint_get_protocol(receiver_repair_endp, &repair_proto) == 0);
CHECK(roc_endpoint_get_port(receiver_repair_endp, &repair_port) == 0);
CHECK(receiver_repair_endp_.set_host_port(address::Family_IPv4, "127.0.0.1",
                                          repair_port));

// receiver's control endpoint
roc_protocol control_proto;
int control_port = 0;
if (receiver_control_endp) {
    CHECK(roc_endpoint_get_protocol(receiver_control_endp, &control_proto) == 0);
    CHECK(roc_endpoint_get_port(receiver_control_endp, &control_port) == 0);
    CHECK(receiver_control_endp_.set_host_port(address::Family_IPv4, "127.0.0.1",
                                               control_port));
}

// proxy's receiving addresses
CHECK(recv_source_config_.bind_address.set_host_port(address::Family_IPv4,
                                                     "127.0.0.1", 0));
CHECK(recv_repair_config_.bind_address.set_host_port(address::Family_IPv4,
                                                     "127.0.0.1", 0));
if (receiver_control_endp) {
    CHECK(recv_control_config_.bind_address.set_host_port(address::Family_IPv4,
                                                          "127.0.0.1", 0));
}

// proxy's sending address
CHECK(send_config_.bind_address.set_host_port(address::Family_IPv4, "127.0.0.1",
                                              0));

...

(it's same code, but reordered and with added comments)

[gavv]

Hm, have you tested this, could you describe your testing approach? Or if you haven't, please do.

So we need to proxy control packets in two directions: receiver[control_endpoint] -> proxy -> sender[control_endpoint] and sender[control_endpoint] -> proxy -> receiver[control_endpoint].

In both cases dst_addr is the same - it's proxy.

In the second direction, proxy receives control (feedback) packets from sender on the same address from which proxy sends control packets to sender. But in constructor, I don't see that we create any UDP port that is both sending and receiving?

I guess the easiest way is to make the port that is currently sending (the very first AddUdpPort task) to be also receiving, and match packets by source address. You can find example of setting up bi-directional port in node::Sender::configure (look for StartUdpSend and StartUdpRecv).