Re: [devel] [PATCH 1/1] mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095]

2019-10-01 Thread Minh Hon Chau

Hi Vu,

See comments below.

Thanks

Minh

On 1/10/19 8:34 pm, Nguyen Minh Vu wrote:

Hi Minh,

Ack with minor comments. Thanks.

Regards, Vu

On 10/1/19 12:49 PM, Minh Chau wrote:

In the scenario of recovery from split-brain, where both
active director services may suffer mds message loss due
to lost-contact tipc link. If MDS_TIPC_FCTRL_ENABLED is
set, the out-of-order message will be dropped, and there
is no mechanism to trigger the retransmission from receiver
side at this moment (the retransmission is only triggered
from sender as result of TIPC_ERR_OVERLOAD).

In reception of disordered message, the receiver can send
not-acknowledgement to notify the sender for retransmission.
Therefore, the sender can trigger retransmisison in the same
way as receiving TIPC_ERR_OVERLOAD.

This patch adds Nack message for retransmission of disordered
message detected from receiver side.
---
  src/mds/mds_c_api.c  |  2 +-
  src/mds/mds_dt_common.c  |  2 +-
  src/mds/mds_tipc_fctrl_intf.cc   | 19 ++-
  src/mds/mds_tipc_fctrl_msg.cc    | 33 
+

  src/mds/mds_tipc_fctrl_msg.h | 22 ++
  src/mds/mds_tipc_fctrl_portid.cc | 18 +-
  src/mds/mds_tipc_fctrl_portid.h  |  1 +
  7 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index c41c8dd..132555b 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -4196,7 +4196,7 @@ void mds_mcm_msg_loss(MDS_SVC_HDL 
local_svc_hdl, MDS_DEST rem_adest,

    /* Check whether the msg loss is enabled or not */
  if (true != local_svc_info->i_msg_loss_indication) {
-    m_MDS_LOG_INFO(" MSG loss not enbaled mds_mcm_msg_loss\n");
+    m_MDS_LOG_NOTIFY("MSG loss is not enabled mds_mcm_msg_loss\n");
  return;
  }
  diff --git a/src/mds/mds_dt_common.c b/src/mds/mds_dt_common.c
index 66652af..de13883 100644
--- a/src/mds/mds_dt_common.c
+++ b/src/mds/mds_dt_common.c
@@ -972,7 +972,7 @@ uint32_t mds_tmr_mailbox_processing(void)
  .vdest_id);
  break;
  case MDS_REASSEMBLY_TMR:
-    m_MDS_LOG_DBG(
+    m_MDS_LOG_ERR(
  "MDTM: Tmr Mailbox Processing:Reassemble Tmr 
Hdl=0x%08x",

  mbx_evt_info->info.tmr_info_hdl);
  mdtm_process_reassem_timer_event(
diff --git a/src/mds/mds_tipc_fctrl_intf.cc 
b/src/mds/mds_tipc_fctrl_intf.cc

index 2366672..65f1849 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -38,6 +38,7 @@ using mds::Timer;
  using mds::DataMessage;
  using mds::ChunkAck;
  using mds::HeaderMessage;
+using mds::Nack;
    namespace {
  // flow control enabled/disabled
@@ -142,7 +143,8 @@ uint32_t process_flow_event(const Event& evt) {
  if (evt.type_ == Event::Type::kEvtSendChunkAck) {
    portid->SendChunkAck(evt.fseq_, evt.svc_id_, evt.chunk_size_);
  }
-    if (evt.type_ == Event::Type::kEvtDropData) {
+    if (evt.type_ == Event::Type::kEvtDropData ||
+    evt.type_ == Event::Type::kEvtRcvNack) {
    portid->ReceiveNack(evt.mseq_, evt.mfrag_,
    evt.fseq_);
  }
@@ -464,6 +466,21 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t 
*buffer, uint16_t len,

  // skip this data msg
  return NCSCC_RC_FAILURE;
    }
+  if (header.msg_type_ == Nack::kNackMsgType) {
+    // receive nack message
+    Nack nack;
+    nack.Decode(buffer);
+    // send to the event thread
+    if (m_NCS_IPC_SEND(_events,
+    new Event(Event::Type::kEvtRcvNack, id, nack.svc_id_,
+    header.mseq_, header.mfrag_, nack.nacked_fseq_),
+    NCS_IPC_PRIORITY_HIGH) != NCSCC_RC_SUCCESS) {
+  m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events\n");
+    }
+    // return NCSCC_RC_FAILURE, so the tipc receiving thread 
(legacy) will

+    // skip this data msg
+    return NCSCC_RC_FAILURE;
+  }
  } else {
    // receive data message
    DataMessage data;
diff --git a/src/mds/mds_tipc_fctrl_msg.cc 
b/src/mds/mds_tipc_fctrl_msg.cc

index 064d977..f85568c 100644
--- a/src/mds/mds_tipc_fctrl_msg.cc
+++ b/src/mds/mds_tipc_fctrl_msg.cc
@@ -139,4 +139,37 @@ void ChunkAck::Decode(uint8_t *msg) {
    chunk_size_ = ncs_decode_16bit();
  }
  +
+Nack::Nack(uint16_t svc_id, uint16_t fseq):
+    svc_id_(svc_id), nacked_fseq_(fseq) {
+  msg_type_ = kNackMsgType;
+}
+
+void Nack::Encode(uint8_t *msg) {
+  uint8_t *ptr;
+  // encode protocol identifier
+  ptr = [Nack::FieldIndex::kProtocolIdentifier];
+  ncs_encode_32bit(, MDS_PROT_FCTRL_ID);
+  // encode message type
+  ptr = [Nack::FieldIndex::kFlowControlMessageType];
+  ncs_encode_8bit(, kNackMsgType);
+  // encode service id
+  ptr = [Nack::FieldIndex::kServiceId];
+  ncs_encode_16bit(, svc_id_);
+  // encode flow control sequence number
+  ptr = [Nack::FieldIndex::kFlowControlSequenceNumber];
+ 

Re: [devel] [PATCH 1/1] mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095]

2019-10-01 Thread Nguyen Minh Vu

Hi Minh,

Ack with minor comments. Thanks.

Regards, Vu

On 10/1/19 12:49 PM, Minh Chau wrote:

In the scenario of recovery from split-brain, where both
active director services may suffer mds message loss due
to lost-contact tipc link. If MDS_TIPC_FCTRL_ENABLED is
set, the out-of-order message will be dropped, and there
is no mechanism to trigger the retransmission from receiver
side at this moment (the retransmission is only triggered
from sender as result of TIPC_ERR_OVERLOAD).

In reception of disordered message, the receiver can send
not-acknowledgement to notify the sender for retransmission.
Therefore, the sender can trigger retransmisison in the same
way as receiving TIPC_ERR_OVERLOAD.

This patch adds Nack message for retransmission of disordered
message detected from receiver side.
---
  src/mds/mds_c_api.c  |  2 +-
  src/mds/mds_dt_common.c  |  2 +-
  src/mds/mds_tipc_fctrl_intf.cc   | 19 ++-
  src/mds/mds_tipc_fctrl_msg.cc| 33 +
  src/mds/mds_tipc_fctrl_msg.h | 22 ++
  src/mds/mds_tipc_fctrl_portid.cc | 18 +-
  src/mds/mds_tipc_fctrl_portid.h  |  1 +
  7 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index c41c8dd..132555b 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -4196,7 +4196,7 @@ void mds_mcm_msg_loss(MDS_SVC_HDL local_svc_hdl, MDS_DEST 
rem_adest,
  
  	/* Check whether the msg loss is enabled or not */

if (true != local_svc_info->i_msg_loss_indication) {
-   m_MDS_LOG_INFO(" MSG loss not enbaled mds_mcm_msg_loss\n");
+   m_MDS_LOG_NOTIFY("MSG loss is not enabled mds_mcm_msg_loss\n");
return;
}
  
diff --git a/src/mds/mds_dt_common.c b/src/mds/mds_dt_common.c

index 66652af..de13883 100644
--- a/src/mds/mds_dt_common.c
+++ b/src/mds/mds_dt_common.c
@@ -972,7 +972,7 @@ uint32_t mds_tmr_mailbox_processing(void)
.vdest_id);
break;
case MDS_REASSEMBLY_TMR:
-   m_MDS_LOG_DBG(
+   m_MDS_LOG_ERR(
"MDTM: Tmr Mailbox Processing:Reassemble Tmr 
Hdl=0x%08x",
mbx_evt_info->info.tmr_info_hdl);
mdtm_process_reassem_timer_event(
diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc
index 2366672..65f1849 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -38,6 +38,7 @@ using mds::Timer;
  using mds::DataMessage;
  using mds::ChunkAck;
  using mds::HeaderMessage;
+using mds::Nack;
  
  namespace {

  // flow control enabled/disabled
@@ -142,7 +143,8 @@ uint32_t process_flow_event(const Event& evt) {
  if (evt.type_ == Event::Type::kEvtSendChunkAck) {
portid->SendChunkAck(evt.fseq_, evt.svc_id_, evt.chunk_size_);
  }
-if (evt.type_ == Event::Type::kEvtDropData) {
+if (evt.type_ == Event::Type::kEvtDropData ||
+evt.type_ == Event::Type::kEvtRcvNack) {
portid->ReceiveNack(evt.mseq_, evt.mfrag_,
evt.fseq_);
  }
@@ -464,6 +466,21 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t *buffer, uint16_t 
len,
  // skip this data msg
  return NCSCC_RC_FAILURE;
}
+  if (header.msg_type_ == Nack::kNackMsgType) {
+// receive nack message
+Nack nack;
+nack.Decode(buffer);
+// send to the event thread
+if (m_NCS_IPC_SEND(_events,
+new Event(Event::Type::kEvtRcvNack, id, nack.svc_id_,
+header.mseq_, header.mfrag_, nack.nacked_fseq_),
+NCS_IPC_PRIORITY_HIGH) != NCSCC_RC_SUCCESS) {
+  m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events\n");
+}
+// return NCSCC_RC_FAILURE, so the tipc receiving thread (legacy) will
+// skip this data msg
+return NCSCC_RC_FAILURE;
+  }
  } else {
// receive data message
DataMessage data;
diff --git a/src/mds/mds_tipc_fctrl_msg.cc b/src/mds/mds_tipc_fctrl_msg.cc
index 064d977..f85568c 100644
--- a/src/mds/mds_tipc_fctrl_msg.cc
+++ b/src/mds/mds_tipc_fctrl_msg.cc
@@ -139,4 +139,37 @@ void ChunkAck::Decode(uint8_t *msg) {
chunk_size_ = ncs_decode_16bit();
  }
  
+

+Nack::Nack(uint16_t svc_id, uint16_t fseq):
+svc_id_(svc_id), nacked_fseq_(fseq) {
+  msg_type_ = kNackMsgType;
+}
+
+void Nack::Encode(uint8_t *msg) {
+  uint8_t *ptr;
+  // encode protocol identifier
+  ptr = [Nack::FieldIndex::kProtocolIdentifier];
+  ncs_encode_32bit(, MDS_PROT_FCTRL_ID);
+  // encode message type
+  ptr = [Nack::FieldIndex::kFlowControlMessageType];
+  ncs_encode_8bit(, kNackMsgType);
+  // encode service id
+  ptr = [Nack::FieldIndex::kServiceId];
+  ncs_encode_16bit(, svc_id_);
+  // encode flow control sequence number
+  ptr = 

[devel] [PATCH 1/1] mds: Add Nack message for MDS_TIPC_FCTRL_ENABLED [#3095]

2019-09-30 Thread Minh Chau
In the scenario of recovery from split-brain, where both
active director services may suffer mds message loss due
to lost-contact tipc link. If MDS_TIPC_FCTRL_ENABLED is
set, the out-of-order message will be dropped, and there
is no mechanism to trigger the retransmission from receiver
side at this moment (the retransmission is only triggered
from sender as result of TIPC_ERR_OVERLOAD).

In reception of disordered message, the receiver can send
not-acknowledgement to notify the sender for retransmission.
Therefore, the sender can trigger retransmisison in the same
way as receiving TIPC_ERR_OVERLOAD.

This patch adds Nack message for retransmission of disordered
message detected from receiver side.
---
 src/mds/mds_c_api.c  |  2 +-
 src/mds/mds_dt_common.c  |  2 +-
 src/mds/mds_tipc_fctrl_intf.cc   | 19 ++-
 src/mds/mds_tipc_fctrl_msg.cc| 33 +
 src/mds/mds_tipc_fctrl_msg.h | 22 ++
 src/mds/mds_tipc_fctrl_portid.cc | 18 +-
 src/mds/mds_tipc_fctrl_portid.h  |  1 +
 7 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/src/mds/mds_c_api.c b/src/mds/mds_c_api.c
index c41c8dd..132555b 100644
--- a/src/mds/mds_c_api.c
+++ b/src/mds/mds_c_api.c
@@ -4196,7 +4196,7 @@ void mds_mcm_msg_loss(MDS_SVC_HDL local_svc_hdl, MDS_DEST 
rem_adest,
 
/* Check whether the msg loss is enabled or not */
if (true != local_svc_info->i_msg_loss_indication) {
-   m_MDS_LOG_INFO(" MSG loss not enbaled mds_mcm_msg_loss\n");
+   m_MDS_LOG_NOTIFY("MSG loss is not enabled mds_mcm_msg_loss\n");
return;
}
 
diff --git a/src/mds/mds_dt_common.c b/src/mds/mds_dt_common.c
index 66652af..de13883 100644
--- a/src/mds/mds_dt_common.c
+++ b/src/mds/mds_dt_common.c
@@ -972,7 +972,7 @@ uint32_t mds_tmr_mailbox_processing(void)
.vdest_id);
break;
case MDS_REASSEMBLY_TMR:
-   m_MDS_LOG_DBG(
+   m_MDS_LOG_ERR(
"MDTM: Tmr Mailbox Processing:Reassemble 
Tmr Hdl=0x%08x",
mbx_evt_info->info.tmr_info_hdl);
mdtm_process_reassem_timer_event(
diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc
index 2366672..65f1849 100644
--- a/src/mds/mds_tipc_fctrl_intf.cc
+++ b/src/mds/mds_tipc_fctrl_intf.cc
@@ -38,6 +38,7 @@ using mds::Timer;
 using mds::DataMessage;
 using mds::ChunkAck;
 using mds::HeaderMessage;
+using mds::Nack;
 
 namespace {
 // flow control enabled/disabled
@@ -142,7 +143,8 @@ uint32_t process_flow_event(const Event& evt) {
 if (evt.type_ == Event::Type::kEvtSendChunkAck) {
   portid->SendChunkAck(evt.fseq_, evt.svc_id_, evt.chunk_size_);
 }
-if (evt.type_ == Event::Type::kEvtDropData) {
+if (evt.type_ == Event::Type::kEvtDropData ||
+evt.type_ == Event::Type::kEvtRcvNack) {
   portid->ReceiveNack(evt.mseq_, evt.mfrag_,
   evt.fseq_);
 }
@@ -464,6 +466,21 @@ uint32_t mds_tipc_fctrl_rcv_data(uint8_t *buffer, uint16_t 
len,
 // skip this data msg
 return NCSCC_RC_FAILURE;
   }
+  if (header.msg_type_ == Nack::kNackMsgType) {
+// receive nack message
+Nack nack;
+nack.Decode(buffer);
+// send to the event thread
+if (m_NCS_IPC_SEND(_events,
+new Event(Event::Type::kEvtRcvNack, id, nack.svc_id_,
+header.mseq_, header.mfrag_, nack.nacked_fseq_),
+NCS_IPC_PRIORITY_HIGH) != NCSCC_RC_SUCCESS) {
+  m_MDS_LOG_ERR("FCTRL: Failed to send msg to mbx_events\n");
+}
+// return NCSCC_RC_FAILURE, so the tipc receiving thread (legacy) will
+// skip this data msg
+return NCSCC_RC_FAILURE;
+  }
 } else {
   // receive data message
   DataMessage data;
diff --git a/src/mds/mds_tipc_fctrl_msg.cc b/src/mds/mds_tipc_fctrl_msg.cc
index 064d977..f85568c 100644
--- a/src/mds/mds_tipc_fctrl_msg.cc
+++ b/src/mds/mds_tipc_fctrl_msg.cc
@@ -139,4 +139,37 @@ void ChunkAck::Decode(uint8_t *msg) {
   chunk_size_ = ncs_decode_16bit();
 }
 
+
+Nack::Nack(uint16_t svc_id, uint16_t fseq):
+svc_id_(svc_id), nacked_fseq_(fseq) {
+  msg_type_ = kNackMsgType;
+}
+
+void Nack::Encode(uint8_t *msg) {
+  uint8_t *ptr;
+  // encode protocol identifier
+  ptr = [Nack::FieldIndex::kProtocolIdentifier];
+  ncs_encode_32bit(, MDS_PROT_FCTRL_ID);
+  // encode message type
+  ptr = [Nack::FieldIndex::kFlowControlMessageType];
+  ncs_encode_8bit(, kNackMsgType);
+  // encode service id
+  ptr = [Nack::FieldIndex::kServiceId];
+  ncs_encode_16bit(, svc_id_);
+  // encode flow control sequence number
+  ptr = [Nack::FieldIndex::kFlowControlSequenceNumber];
+  ncs_encode_16bit(, nacked_fseq_);
+}
+
+void Nack::Decode(uint8_t *msg) {
+