nasa / HDTN

High-rate Delay Tolerant Network (HDTN) Software
https://www1.grc.nasa.gov/space/scan/acs/tech-studies/dtn/
Other
91 stars 22 forks source link

LtpEngine has no way to correlate a session source with its session ID #5

Closed BrianSipos closed 2 years ago

BrianSipos commented 2 years ago

The API for LtpEngine currently has callbacks to provide status on outgoing transfer sessions, all indexed by a Ltp::session_id_t object. But there is no way to correlate any of those session ID with the data which originated the outoging session (from any of the TransmissionRequest... functions). I added an outside synchronization on the user side to guarantee each TransmissionRequest correlates to a single TX session ID, but this will not scale to high TX session rates and is unnecessary wait time.

It would be useful if the TransmissionRequest... functions allowed either the user to define a correlation object (just a object-identity token would do, either a std::shared_ptr<SessionToken>) or if possible you could make use of C++ promise/future infrastructure to have the TransmissionRequest... functions return an std::future<Ltp::session_id_t> which would the be fulfilled in the work thread when the session ID is assigned.

I can prototype this last idea and prove it out if it seems like a good path to take.

briantomko commented 2 years ago

@BrianSipos Is something like this what you are looking for? You can bind whatever user data variable(s) you want to any callback functions. For example, replace:

void TransmissionSessionCompletedCallback(const Ltp::session_id_t & sessionId) {}
ltpUdpEngineSrcPtr->SetTransmissionSessionCompletedCallback(boost::bind(&SenderHelper::TransmissionSessionCompletedCallback, &senderHelper, boost::placeholders::_1));

with:

void TransmissionSessionCompletedCallback(const Ltp::session_id_t & sessionId, int customDataInt, const std::string & customDataString) {}
int myCustomDataInt = 10;
std::string myCustomDataString = "some data";
ltpUdpEngineSrcPtr->SetTransmissionSessionCompletedCallback(boost::bind(&SenderHelper::TransmissionSessionCompletedCallback, &senderHelper, boost::placeholders::_1, myCustomDataInt, myCustomDataString));
BrianSipos commented 2 years ago

That allows a user to associate the data with the peer engine but what I need is to associate data with the specific outgoing session. The example you have doesn't make that data specific to the outgoing transfer session.

In my case each session has an external bundle handle and when I relay the HDTN LTP session completed/canceled I need to refer to the original bundle handle used to initiate the session.

I can even use object identity as a correlator, so if there was some generic base class

class LtpSessionUserData {
public:
  virtual ~LtpSessionUserData() {}
};

with the TransmissionRequest...() and TransmissionSession...Callback_t signatures adapted to add a std::shared_ptr<LtpSessionUserData> I would be able to either derive from that class and include the handle directly or just use the pointer value as a unique key. The important thing is that this pointer is kept with each LtpSessionSender object and able to be present for the callbacks.

briantomko commented 2 years ago

@BrianSipos Would this work? You can bind the shared pointer to all or some of the callbacks.

class MyLtpSessionUserData {
};
boost::shared_ptr<MyLtpSessionUserData> myUserDataPtr = boost::make_shared<MyLtpSessionUserData>();

void TransmissionSessionCompletedCallback(const Ltp::session_id_t & sessionId, boost::shared_ptr<MyLtpSessionUserData> & userDataPtr) {}
void InitialTransmissionCompletedCallback(const Ltp::session_id_t & sessionId, boost::shared_ptr<MyLtpSessionUserData> & userDataPtr) {}
void TransmissionSessionCancelledCallback(const Ltp::session_id_t & sessionId, CANCEL_SEGMENT_REASON_CODES reasonCode, boost::shared_ptr<MyLtpSessionUserData> & userDataPtr) {}

ltpUdpEngineSrcPtr->SetTransmissionSessionCompletedCallback(boost::bind(&TransmissionSessionCompletedCallback, boost::placeholders::_1, myUserDataPtr));
ltpUdpEngineSrcPtr->SetInitialTransmissionCompletedCallback(boost::bind(&InitialTransmissionCompletedCallback, boost::placeholders::_1, myUserDataPtr));
ltpUdpEngineSrcPtr->SetTransmissionSessionCancelledCallback(boost::bind(&TransmissionSessionCancelledCallback, boost::placeholders::_1, boost::placeholders::_2, myUserDataPtr));
BrianSipos commented 2 years ago

No, it's still the same problem. I would like to pipeline LTP TX sessions and currently cannot because of the session ID correlation. For a toy example, imagine the three TX sessions created by:

LtpUdpEngine *txEng = ...;
auto reqA = boost::make_shared<LtpEngine::transmission_request_t>();
auto reqB = boost::make_shared<LtpEngine::transmission_request_t>();
auto reqC = boost::make_shared<LtpEngine::transmission_request_t>();
...
txEng->TransmissionRequest_ThreadSafe(std::move(reqA));
txEng->TransmissionRequest_ThreadSafe(std::move(reqB));
txEng->TransmissionRequest_ThreadSafe(std::move(reqC));

Because of the background thread operation I don't have anything to correlate, for example, reqA data with anything visible to the ...Transmission... callbacks. Those callbacks only see a Session ID, which is randomly generated after the other thread starts processing each request.

There may be some guarantee of order of InitialTransmissionCompletedCallback invocation based on the order of TransmissionRequest... calls, but it's not documented and not obvious if there is a guarantee of this. If this is the case, then I can satisfy my need by keeping a queue of "requested but not yet initiated transmission" handles.

briantomko commented 2 years ago

You are correct. I will add a user data object to the transmission_request_t

briantomko commented 2 years ago

This issue has been resolved with merge of 73-ltp-transmission-request-user-data into master

Here's how I've tested it in TestLtpUdpEngine.cpp. Right now there's a mix of boost::shared_ptr and std::shared_ptr. Eventually that will all get moved to std::shared_ptr:

struct MyTransmissionUserData : public LtpTransmissionRequestUserData {
    MyTransmissionUserData() : m_data(0) {}
    MyTransmissionUserData(const unsigned int data) : m_data(data) {}
    virtual ~MyTransmissionUserData() {}

    unsigned int m_data;
};

void TransmissionSessionCompletedCallback(const Ltp::session_id_t & sessionId, std::shared_ptr<LtpTransmissionRequestUserData> & userDataPtr) {
    if(MyTransmissionUserData * myUserData = dynamic_cast<MyTransmissionUserData*>(userDataPtr.get())) { //could also use reinterpret_cast if certain of type

    }
}
void InitialTransmissionCompletedCallback(const Ltp::session_id_t & sessionId, std::shared_ptr<LtpTransmissionRequestUserData> & userDataPtr) {}
void TransmissionSessionCancelledCallback(const Ltp::session_id_t & sessionId, CANCEL_SEGMENT_REASON_CODES reasonCode, std::shared_ptr<LtpTransmissionRequestUserData> & userDataPtr) {} 

//add extra placeholder boost::placeholders::_2 for the user data if your function has the userDataPtr parameter
//only need the second parameter `this` or class pointer to `bind` if and only if it is a class member function
ltpUdpEngineSrcPtr->SetTransmissionSessionCompletedCallback(boost::bind(&Test::TransmissionSessionCompletedCallback, this, boost::placeholders::_1, boost::placeholders::_2));
ltpUdpEngineSrcPtr->SetInitialTransmissionCompletedCallback(boost::bind(&Test::InitialTransmissionCompletedCallback, this, boost::placeholders::_1, boost::placeholders::_2));

//add extra placeholder boost::placeholders::_3 for the user data if your function has the userDataPtr parameter
ltpUdpEngineSrcPtr->SetTransmissionSessionCancelledCallback(boost::bind(&Test::TransmissionSessionCancelledCallback, this, boost::placeholders::_1, boost::placeholders::_2, boost::placeholders::_3));

boost::shared_ptr<LtpEngine::transmission_request_t> tReq = boost::make_shared<LtpEngine::transmission_request_t>();
tReq->destinationClientServiceId = CLIENT_SERVICE_ID_DEST;
tReq->destinationLtpEngineId = ENGINE_ID_DEST;
tReq->clientServiceDataToSend = std::vector<uint8_t>(DESIRED_RED_DATA_TO_SEND.data(), DESIRED_RED_DATA_TO_SEND.data() + DESIRED_RED_DATA_TO_SEND.size()); //copy
tReq->lengthOfRedPart = DESIRED_RED_DATA_TO_SEND.size();
std::shared_ptr<MyTransmissionUserData> myUserData = std::make_shared<MyTransmissionUserData>(123);
tReq->userDataPtr = myUserData; //keep a local copy of the pointer (or optionally std::move it)
ltpUdpEngineSrcPtr->TransmissionRequest_ThreadSafe(std::move(tReq));