Re: [devel] [PATCH 1/1] mds: Add Intro message [#3090]
Hi bro.Minh,
ACK from me.
Best Regards,
ThuanTr
-Original Message-
From: Minh Hon Chau
Sent: Tuesday, October 15, 2019 8:54 AM
To: hans.nordeb...@ericsson.com; vu.m.ngu...@dektech.com.au;
gary@dektech.com.au; thuan.t...@dektech.com.au
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] mds: Add Intro message [#3090]
Hi,
The counters reset will be removed in ReceiveIntro().
Thanks
Minh
On 15/10/19 12:50 pm, Minh Chau wrote:
> mds relies on data message sent from the peer to determine whether the
> MDS_TIPC_FCTRL_ENABLED is set. The data message may not be sent right
> after TIPC_PUBLISHED event, which can cause the tx probation timer
> timeout.
>
> This patch add Intro message, which is sent right after the
> TIPC_PUBLISHED to help mds determine the flow control supported at the
> peer earlier.
> ---
> src/mds/mds_main.c | 2 +-
> src/mds/mds_tipc_fctrl_intf.cc | 27 ++
> src/mds/mds_tipc_fctrl_msg.cc| 11 +
> src/mds/mds_tipc_fctrl_msg.h | 18 +++
> src/mds/mds_tipc_fctrl_portid.cc | 49
> ++--
> src/mds/mds_tipc_fctrl_portid.h | 2 ++
> 6 files changed, 96 insertions(+), 13 deletions(-)
>
> diff --git a/src/mds/mds_main.c b/src/mds/mds_main.c index
> 8c9b1f1..c7d2f7b 100644
> --- a/src/mds/mds_main.c
> +++ b/src/mds/mds_main.c
> @@ -408,7 +408,7 @@ uint32_t mds_lib_req(NCS_LIB_REQ_INFO *req)
> if (tipc_mcast_enabled != false)
> tipc_mcast_enabled = true;
>
> - m_MDS_LOG_DBG(
> + m_MDS_LOG_NOTIFY(
> "MDS: TIPC_MCAST_ENABLED: %d Set argument
> \n",
> tipc_mcast_enabled);
> }
> diff --git a/src/mds/mds_tipc_fctrl_intf.cc
> b/src/mds/mds_tipc_fctrl_intf.cc index 6271890..b803bfe 100644
> --- a/src/mds/mds_tipc_fctrl_intf.cc
> +++ b/src/mds/mds_tipc_fctrl_intf.cc
> @@ -39,6 +39,7 @@ using mds::DataMessage;
> using mds::ChunkAck;
> using mds::HeaderMessage;
> using mds::Nack;
> +using mds::Intro;
>
> namespace {
> // flow control enabled/disabled
> @@ -124,12 +125,20 @@ uint32_t process_flow_event(const Event& evt) {
> uint32_t rc = NCSCC_RC_SUCCESS;
> TipcPortId *portid = portid_lookup(evt.id_);
> if (portid == nullptr) {
> +// the null portid normally should not happen; however because the
> +// tipc_cb.Dsock and tipc_cb.BSRsock are separated; the data message
> +// sent from BSRsock may come before reception of TIPC_PUBLISHED
> if (evt.type_ == Event::Type::kEvtRcvData) {
> portid = new TipcPortId(evt.id_, data_sock_fd,
> kChunkAckSize, sock_buf_size);
> portid_map[TipcPortId::GetUniqueId(evt.id_)] = portid;
> rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
> evt.fseq_, evt.svc_id_);
> +} else if (evt.type_ == Event::Type::kEvtRcvIntro) {
> + portid = new TipcPortId(evt.id_, data_sock_fd,
> + kChunkAckSize, sock_buf_size);
> + portid_map[TipcPortId::GetUniqueId(evt.id_)] = portid;
> + portid->ReceiveIntro();
> } else {
> m_MDS_LOG_ERR("FCTRL: [me] <-- [node:%x, ref:%u], "
> "RcvEvt[evt:%d], Error[PortId not found]", @@ -151,6
> +160,9 @@ uint32_t process_flow_event(const Event& evt) {
> portid->ReceiveNack(evt.mseq_, evt.mfrag_,
> evt.fseq_);
> }
> +if (evt.type_ == Event::Type::kEvtRcvIntro) {
> + portid->ReceiveIntro();
> +}
> }
> return rc;
> }
> @@ -489,6 +501,16 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t *buffer,
> uint16_t len,
> m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events,
> Error[%s]",
> strerror(errno));
> }
> + } else if (header.msg_type_ == Intro::kIntroMsgType) {
> +// no need to decode intro message
> +// the decoding intro message type is done in header decoding
> +// send to the event thread
> +if (m_NCS_IPC_SEND(_events,
> +new Event(Event::Type::kEvtRcvIntro, id, 0, 0, 0, 0),
> +NCS_IPC_PRIORITY_HIGH) != NCSCC_RC_SUCCESS) {
> + m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events, Error[%s]",
> + strerror(errno));
> +}
> } else {
> m_MDS_LOG_ERR("FCTRL: [me] <-- [node:%x, ref:%u], "
> "[msg_type:%u], Error[not supported message type]", @@
> -516,6 +538,11 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t *buffer, uint16_t
> len,
> portid_map_mutex.unlock();
> return rc;
> }
> + } else {
> +m_MDS_LOG_DBG("FCTRL: [me] <-- [node:%x, ref:%u], "
> +"Receive non-flow-control data message, "
> +"header.pro_ver:%u",
> +id.node, id.ref, header.pro_ver_);
> }
> return NCSCC_RC_SUCCESS;
> }
> diff --git
Re: [devel] [PATCH 1/1] mds: Add Intro message [#3090]
Hi,
The counters reset will be removed in ReceiveIntro().
Thanks
Minh
On 15/10/19 12:50 pm, Minh Chau wrote:
mds relies on data message sent from the peer to determine
whether the MDS_TIPC_FCTRL_ENABLED is set. The data message
may not be sent right after TIPC_PUBLISHED event, which can
cause the tx probation timer timeout.
This patch add Intro message, which is sent right after the
TIPC_PUBLISHED to help mds determine the flow control supported
at the peer earlier.
---
src/mds/mds_main.c | 2 +-
src/mds/mds_tipc_fctrl_intf.cc | 27 ++
src/mds/mds_tipc_fctrl_msg.cc| 11 +
src/mds/mds_tipc_fctrl_msg.h | 18 +++
src/mds/mds_tipc_fctrl_portid.cc | 49 ++--
src/mds/mds_tipc_fctrl_portid.h | 2 ++
6 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/src/mds/mds_main.c b/src/mds/mds_main.c
index 8c9b1f1..c7d2f7b 100644
--- a/src/mds/mds_main.c
+++ b/src/mds/mds_main.c
@@ -408,7 +408,7 @@ uint32_t mds_lib_req(NCS_LIB_REQ_INFO *req)
if (tipc_mcast_enabled != false)
tipc_mcast_enabled = true;
-m_MDS_LOG_DBG(
+ m_MDS_LOG_NOTIFY(
"MDS: TIPC_MCAST_ENABLED: %d Set argument
\n",
tipc_mcast_enabled);
}
diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc
index 6271890..b803bfe 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -39,6 +39,7 @@ using mds::DataMessage;
using mds::ChunkAck;
using mds::HeaderMessage;
using mds::Nack;
+using mds::Intro;
namespace {
// flow control enabled/disabled
@@ -124,12 +125,20 @@ uint32_t process_flow_event(const Event& evt) {
uint32_t rc = NCSCC_RC_SUCCESS;
TipcPortId *portid = portid_lookup(evt.id_);
if (portid == nullptr) {
+// the null portid normally should not happen; however because the
+// tipc_cb.Dsock and tipc_cb.BSRsock are separated; the data message
+// sent from BSRsock may come before reception of TIPC_PUBLISHED
if (evt.type_ == Event::Type::kEvtRcvData) {
portid = new TipcPortId(evt.id_, data_sock_fd,
kChunkAckSize, sock_buf_size);
portid_map[TipcPortId::GetUniqueId(evt.id_)] = portid;
rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
evt.fseq_, evt.svc_id_);
+} else if (evt.type_ == Event::Type::kEvtRcvIntro) {
+ portid = new TipcPortId(evt.id_, data_sock_fd,
+ kChunkAckSize, sock_buf_size);
+ portid_map[TipcPortId::GetUniqueId(evt.id_)] = portid;
+ portid->ReceiveIntro();
} else {
m_MDS_LOG_ERR("FCTRL: [me] <-- [node:%x, ref:%u], "
"RcvEvt[evt:%d], Error[PortId not found]",
@@ -151,6 +160,9 @@ uint32_t process_flow_event(const Event& evt) {
portid->ReceiveNack(evt.mseq_, evt.mfrag_,
evt.fseq_);
}
+if (evt.type_ == Event::Type::kEvtRcvIntro) {
+ portid->ReceiveIntro();
+}
}
return rc;
}
@@ -489,6 +501,16 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t *buffer, uint16_t
len,
m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events, Error[%s]",
strerror(errno));
}
+ } else if (header.msg_type_ == Intro::kIntroMsgType) {
+// no need to decode intro message
+// the decoding intro message type is done in header decoding
+// send to the event thread
+if (m_NCS_IPC_SEND(_events,
+new Event(Event::Type::kEvtRcvIntro, id, 0, 0, 0, 0),
+NCS_IPC_PRIORITY_HIGH) != NCSCC_RC_SUCCESS) {
+ m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events, Error[%s]",
+ strerror(errno));
+}
} else {
m_MDS_LOG_ERR("FCTRL: [me] <-- [node:%x, ref:%u], "
"[msg_type:%u], Error[not supported message type]",
@@ -516,6 +538,11 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t *buffer, uint16_t
len,
portid_map_mutex.unlock();
return rc;
}
+ } else {
+m_MDS_LOG_DBG("FCTRL: [me] <-- [node:%x, ref:%u], "
+"Receive non-flow-control data message, "
+"header.pro_ver:%u",
+id.node, id.ref, header.pro_ver_);
}
return NCSCC_RC_SUCCESS;
}
diff --git a/src/mds/mds_tipc_fctrl_msg.cc b/src/mds/mds_tipc_fctrl_msg.cc
index 932120f..180dcb6 100644
--- a/src/mds/mds_tipc_fctrl_msg.cc
+++ b/src/mds/mds_tipc_fctrl_msg.cc
@@ -178,4 +178,15 @@ void Nack::Decode(uint8_t *msg) {
nacked_fseq_ = ncs_decode_16bit();
}
+
+void Intro::Encode(uint8_t *msg) {
+ uint8_t *ptr;
+ // encode protocol identifier
+ ptr = [Intro::FieldIndex::kProtocolIdentifier];
+ ncs_encode_32bit(, MDS_PROT_FCTRL_ID);
+ // encode message type
+ ptr = [Intro::FieldIndex::kFlowControlMessageType];
+ ncs_encode_8bit(, kIntroMsgType);
+}
+
} // end
[devel] [PATCH 1/1] mds: Add Intro message [#3090]
mds relies on data message sent from the peer to determine
whether the MDS_TIPC_FCTRL_ENABLED is set. The data message
may not be sent right after TIPC_PUBLISHED event, which can
cause the tx probation timer timeout.
This patch add Intro message, which is sent right after the
TIPC_PUBLISHED to help mds determine the flow control supported
at the peer earlier.
---
src/mds/mds_main.c | 2 +-
src/mds/mds_tipc_fctrl_intf.cc | 27 ++
src/mds/mds_tipc_fctrl_msg.cc| 11 +
src/mds/mds_tipc_fctrl_msg.h | 18 +++
src/mds/mds_tipc_fctrl_portid.cc | 49 ++--
src/mds/mds_tipc_fctrl_portid.h | 2 ++
6 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/src/mds/mds_main.c b/src/mds/mds_main.c
index 8c9b1f1..c7d2f7b 100644
--- a/src/mds/mds_main.c
+++ b/src/mds/mds_main.c
@@ -408,7 +408,7 @@ uint32_t mds_lib_req(NCS_LIB_REQ_INFO *req)
if (tipc_mcast_enabled != false)
tipc_mcast_enabled = true;
- m_MDS_LOG_DBG(
+ m_MDS_LOG_NOTIFY(
"MDS: TIPC_MCAST_ENABLED: %d Set argument
\n",
tipc_mcast_enabled);
}
diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc
index 6271890..b803bfe 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -39,6 +39,7 @@ using mds::DataMessage;
using mds::ChunkAck;
using mds::HeaderMessage;
using mds::Nack;
+using mds::Intro;
namespace {
// flow control enabled/disabled
@@ -124,12 +125,20 @@ uint32_t process_flow_event(const Event& evt) {
uint32_t rc = NCSCC_RC_SUCCESS;
TipcPortId *portid = portid_lookup(evt.id_);
if (portid == nullptr) {
+// the null portid normally should not happen; however because the
+// tipc_cb.Dsock and tipc_cb.BSRsock are separated; the data message
+// sent from BSRsock may come before reception of TIPC_PUBLISHED
if (evt.type_ == Event::Type::kEvtRcvData) {
portid = new TipcPortId(evt.id_, data_sock_fd,
kChunkAckSize, sock_buf_size);
portid_map[TipcPortId::GetUniqueId(evt.id_)] = portid;
rc = portid->ReceiveData(evt.mseq_, evt.mfrag_,
evt.fseq_, evt.svc_id_);
+} else if (evt.type_ == Event::Type::kEvtRcvIntro) {
+ portid = new TipcPortId(evt.id_, data_sock_fd,
+ kChunkAckSize, sock_buf_size);
+ portid_map[TipcPortId::GetUniqueId(evt.id_)] = portid;
+ portid->ReceiveIntro();
} else {
m_MDS_LOG_ERR("FCTRL: [me] <-- [node:%x, ref:%u], "
"RcvEvt[evt:%d], Error[PortId not found]",
@@ -151,6 +160,9 @@ uint32_t process_flow_event(const Event& evt) {
portid->ReceiveNack(evt.mseq_, evt.mfrag_,
evt.fseq_);
}
+if (evt.type_ == Event::Type::kEvtRcvIntro) {
+ portid->ReceiveIntro();
+}
}
return rc;
}
@@ -489,6 +501,16 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t *buffer, uint16_t
len,
m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events, Error[%s]",
strerror(errno));
}
+ } else if (header.msg_type_ == Intro::kIntroMsgType) {
+// no need to decode intro message
+// the decoding intro message type is done in header decoding
+// send to the event thread
+if (m_NCS_IPC_SEND(_events,
+new Event(Event::Type::kEvtRcvIntro, id, 0, 0, 0, 0),
+NCS_IPC_PRIORITY_HIGH) != NCSCC_RC_SUCCESS) {
+ m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events, Error[%s]",
+ strerror(errno));
+}
} else {
m_MDS_LOG_ERR("FCTRL: [me] <-- [node:%x, ref:%u], "
"[msg_type:%u], Error[not supported message type]",
@@ -516,6 +538,11 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t *buffer, uint16_t
len,
portid_map_mutex.unlock();
return rc;
}
+ } else {
+m_MDS_LOG_DBG("FCTRL: [me] <-- [node:%x, ref:%u], "
+"Receive non-flow-control data message, "
+"header.pro_ver:%u",
+id.node, id.ref, header.pro_ver_);
}
return NCSCC_RC_SUCCESS;
}
diff --git a/src/mds/mds_tipc_fctrl_msg.cc b/src/mds/mds_tipc_fctrl_msg.cc
index 932120f..180dcb6 100644
--- a/src/mds/mds_tipc_fctrl_msg.cc
+++ b/src/mds/mds_tipc_fctrl_msg.cc
@@ -178,4 +178,15 @@ void Nack::Decode(uint8_t *msg) {
nacked_fseq_ = ncs_decode_16bit();
}
+
+void Intro::Encode(uint8_t *msg) {
+ uint8_t *ptr;
+ // encode protocol identifier
+ ptr = [Intro::FieldIndex::kProtocolIdentifier];
+ ncs_encode_32bit(, MDS_PROT_FCTRL_ID);
+ // encode message type
+ ptr = [Intro::FieldIndex::kFlowControlMessageType];
+ ncs_encode_8bit(, kIntroMsgType);
+}
+
} // end namespace mds
diff --git a/src/mds/mds_tipc_fctrl_msg.h b/src/mds/mds_tipc_fctrl_msg.h
index e1db200..3e45fa6 100644
---
