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

Move to libpldm APIs for Allocating Instance IDs and change Transport APIs #213

Open
wants to merge 2 commits into
base: 1110
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion include/pldm_fw.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <libpldm/instance-id.h>
#include <libpldm/transport.h>
#include <libpldm/transport/af-mctp.h>
#include <libpldm/transport/mctp-demux.h>
#include <stdint.h>

Expand Down Expand Up @@ -71,7 +72,13 @@ class PldmFramework
/** pldm transport instance */
pldm_transport* pldmTransport = nullptr;

pldm_transport_mctp_demux* mctpDemux = nullptr;
union TransportImpl
{
pldm_transport_mctp_demux* mctpDemux;
pldm_transport_af_mctp* afMctp;
};

TransportImpl impl;

// Constants required for PLDM packet.
static constexpr auto phypTerminusID = (types::Byte)208;
Expand Down Expand Up @@ -135,6 +142,11 @@ class PldmFramework
*/
int openMctpDemuxTransport();

/** @brief Opens the MCTP AF_MCTP for sending and receiving messages.
*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is AF_MCTP? what it does? and why it is required here?
can you please add those details in doxygen.

*/
int openAfMctpTransport();

/** @brief Close the PLDM file */
void pldmClose();

Expand Down
6 changes: 6 additions & 0 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ systemd_system_unit_dir = systemd.get_variable('systemdsystemunitdir')

service_file = 'service_files/com.ibm.panel.service'

if(get_option('transport-implementation')) == 'mctp-demux'
add_project_arguments('-DPLDM_TRANSPORT_WITH_MCTP_DEMUX', language : 'cpp')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please add a comment here on what does the options DPLDM_TRANSPORT_WITH_MCTP_DEMUX and DPLDM_TRANSPORT_WITH_AF_MCTP do?

else
add_project_arguments('-DPLDM_TRANSPORT_WITH_AF_MCTP', language: 'cpp')
endif

if cxx.has_header('poll.h')
add_project_arguments('-DPLDM_HAS_POLL=1', language: 'cpp')
endif
Expand Down
6 changes: 6 additions & 0 deletions meson_options.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
option('tests', type: 'feature', value: 'enabled', description: 'Build tests.',)
option('system-vpd-dependency', type: 'feature', description: 'Enable/disable system vpd dependency.', value: 'disabled')
option(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, which one needs to be used and based on what criteria is it decided?

'transport-implementation',
type: 'combo',
choices: ['mctp-demux', 'af-mctp'],
description: 'transport via af-mctp or mctp-demux'
)
62 changes: 56 additions & 6 deletions src/pldm_fw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,16 @@ int PldmFramework::openPldm()
return fd;
}

#if defined(PLDM_TRANSPORT_WITH_MCTP_DEMUX)
fd = openMctpDemuxTransport();
#elif defined(PLDM_TRANSPORT_WITH_AF_MCTP)
fd = openAfMctpTransport();
#else
std::cerr << "open: No valid transport defined!" << std::endl;
throw FunctionFailure("Required host dump action via PLDM is not allowed: "
"No valid transport defined!");
#endif

if (fd < 0)
{
auto e = errno;
Expand All @@ -72,9 +81,10 @@ int PldmFramework::openPldm()
return fd;
}

int PldmFramework::openMctpDemuxTransport()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have created the line 75 in your first commit and edited it in second commit(in the same PR). can you please check if these changes between two commits are required?

[[maybe_unused]] int PldmFramework::openMctpDemuxTransport()
{
int rc = pldm_transport_mctp_demux_init(&mctpDemux);
impl.mctpDemux = nullptr;
int rc = pldm_transport_mctp_demux_init(&impl.mctpDemux);
if (rc)
{
std::cerr << "openMctpDemuxTransport: Failed to init MCTP demux "
Expand All @@ -83,7 +93,7 @@ int PldmFramework::openMctpDemuxTransport()
throw FunctionFailure("Failed to init MCTP demux transport");
}

rc = pldm_transport_mctp_demux_map_tid(mctpDemux, tid, tid);
rc = pldm_transport_mctp_demux_map_tid(impl.mctpDemux, tid, tid);
if (rc)
{
std::cerr << "openMctpDemuxTransport: Failed to setup tid to eid "
Expand All @@ -92,7 +102,7 @@ int PldmFramework::openMctpDemuxTransport()
pldmClose();
throw FunctionFailure("Failed to setup tid to eid mapping");
}
pldmTransport = pldm_transport_mctp_demux_core(mctpDemux);
pldmTransport = pldm_transport_mctp_demux_core(impl.mctpDemux);

struct pollfd pollfd;
rc = pldm_transport_mctp_demux_init_pollfd(pldmTransport, &pollfd);
Expand All @@ -106,10 +116,50 @@ int PldmFramework::openMctpDemuxTransport()
return pollfd.fd;
}

[[maybe_unused]] int PldmFramework::openAfMctpTransport()
{
impl.afMctp = nullptr;
int rc = pldm_transport_af_mctp_init(&impl.afMctp);
if (rc)
{
std::cerr
<< "openAfMctpTransport: Failed to init AF MCTP transport. rc = "
<< rc << std::endl;
throw FunctionFailure("Failed to init AF MCTP transport");
}

rc = pldm_transport_af_mctp_map_tid(impl.afMctp, tid, tid);
if (rc)
{
std::cerr
<< "openAfMctpTransport: Failed to setup tid to eid mapping. rc = "
<< rc << std::endl;
pldmClose();
throw FunctionFailure("Failed to setup tid to eid mapping");
}
pldmTransport = pldm_transport_af_mctp_core(impl.afMctp);

struct pollfd pollfd;
rc = pldm_transport_af_mctp_init_pollfd(pldmTransport, &pollfd);
if (rc)
{
std::cerr << "openAfMctpTransport: Failed to get pollfd. rc = " << rc
<< std::endl;
pldmClose();
throw FunctionFailure("Failed to get pollfd");
}
return pollfd.fd;
}

void PldmFramework::pldmClose()
{
pldm_transport_mctp_demux_destroy(mctpDemux);
mctpDemux = nullptr;
#if defined(PLDM_TRANSPORT_WITH_MCTP_DEMUX)
pldm_transport_mctp_demux_destroy(impl.mctpDemux);
impl.mctpDemux = nullptr;
#elif defined(PLDM_TRANSPORT_WITH_AF_MCTP)
pldm_transport_af_mctp_destroy(impl.afMctp);
impl.afMctp = nullptr;
#endif
pldmTransport = nullptr;
}

Expand Down