Hi Hans,

Probably you were looking at code that included this Thuan's patch.

In legacy code, only mdtm_sendto() is called inside the function 
mdtm_frag_and_send().

Regards, Vu

> -----Original Message-----
> From: Hans Nordebäck <hans.nordeb...@ericsson.com>
> Sent: Thursday, April 25, 2019 6:10 PM
> To: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>; Thuan Tran
> <thuan.t...@dektech.com.au>; Minh Hon Chau
> <minh.c...@dektech.com.au>
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 0/1] Review Request for mds: support multicast
> fragmented messages [#3033] V3
> 
> 
> Hi Vu,
> It seems mdtm_mcast_sendto is used in mdtm_frag_and_send, at
> MDS_SENDTYPE_BCAST/BR Hans
> -----Original Message-----
> From: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>
> Sent: den 25 april 2019 12:20
> To: Hans Nordebäck <hans.nordeb...@ericsson.com>; Thuan Tran
> <thuan.t...@dektech.com.au>; Minh Hon Chau
> <minh.c...@dektech.com.au>
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: RE: [PATCH 0/1] Review Request for mds: support multicast
> fragmented messages [#3033] V3
> 
> Hi Hans,
> 
> See my responses inline.
> 
> Regards, Vu
> 
> > -----Original Message-----
> > From: Hans Nordebäck <hans.nordeb...@ericsson.com>
> > Sent: Thursday, April 25, 2019 4:28 PM
> > To: Thuan Tran <thuan.t...@dektech.com.au>; Vu Minh Nguyen
> > <vu.m.ngu...@dektech.com.au>; Minh Hon Chau
> <minh.c...@dektech.com.au>
> > Cc: opensaf-devel@lists.sourceforge.net
> > Subject: Re: [PATCH 0/1] Review Request for mds: support multicast
> > fragmented messages [#3033] V3
> >
> > Hi Vu and Thuan,
> >
> > a few question, is the text in the ticket description correct? E.g it
> > says unicast is used if a multicast message is fragmented, (I think
> > multicast still is used
> >
> > to send the fragments), this is what you mean with 2 different channels?
> > (only one socket is used, BSRsock),
> [Vu] Yes. Unicast is used to send fragmented messages. Here is the current
> logic in case of sending a large package:
> Iterate over destinations { // mcm_pvt_process_svc_bcast_common() @
> mds_c_sndrcv.c
>       1) Fragment the package // mdtm_frag_and_send() @ mds_dt_tipc.c
>       2) Unicast to a specific adest  // mdtm_sendto() @
> mds_dt_tipc.c
>       4) Continue with next adest
> }
> 
> >
> > The problem stated is sending one large multicast message and then
> > several smaller multicast messages, have you checked the
> >
> > fragment re-assembly part of the common code?
> [Vu] Yes. At the receive side, if msg is fragmented, mds will not forward to
> upper layer until all fragmented msgs are collected.
> If the message is not fragmented, mds will transfer the msg to upper right
> away.
> 
> I checked with TIPC guys here, and he said that TIPC does not guarantee the
> order if we send msgs in different channels (unicast vs mcast).
> 
> >
> > /BR Hans
> >
> >
> > On 2019-04-24 13:06, thuan.tran wrote:
> > > Summary: mds: support multicast fragmented messages [#3033] Review
> > > request for Ticket(s): 3033 Peer Reviewer(s): Hans, Minh, Vu Pull
> > > request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected
> > > branch(es): develop Development branch: ticket-3033 Base revision:
> > > 7916ac316e86478c621c8359cf2aca4886288a38
> > > Personal repository: git://git.code.sf.net/u/thuantr/review
> > >
> > > --------------------------------
> > > Impacted area       Impact y/n
> > > --------------------------------
> > >   Docs                    n
> > >   Build system            n
> > >   RPM/packaging           n
> > >   Configuration files     n
> > >   Startup scripts         n
> > >   SAF services            y
> > >   OpenSAF services        n
> > >   Core libraries          n
> > >   Samples                 n
> > >   Tests                   n
> > >   Other                   n
> > >
> > > NOTE: Patch(es) contain lines longer than 80 characers
> > >
> > > Comments (indicate scope for each "y" above):
> > > ---------------------------------------------
> > > N/A
> > >
> > > revision 568f09774f936506f5e05e03813fa572af0fe0d3
> > > Author:   thuan.tran <thuan.t...@dektech.com.au>
> > > Date:     Wed, 24 Apr 2019 17:54:25 +0700
> > >
> > > mds: support multicast fragmented messages [#3033]
> > >
> > > - Sender may send broadcast big messages (> 65K) then small messages
> > > (<
> > 65K).
> > > Current MDS just loop via all destinations to unicast all fragmented
> > messages
> > > to one by one destinations. But sending multicast non-fragment
> > > messages
> > to all
> > > destinations. Therefor, receivers may get messages with incorrect
> > > order, non-fragment messages may come before fragmented messages.
> > > For example, it may lead to OUT OF ORDER for IMMNDs during IMMD
> sync.
> > > - Solution: support send multicast each fragmented messages to avoid
> > > disorder of arrived broadcast messages.
> > >
> > >
> > >
> > > Complete diffstat:
> > > ------------------
> > >   src/mds/mds_c_sndrcv.c |   3 +-
> > >   src/mds/mds_dt_tipc.c  | 104
> > > +++++++++++++++++++-------------------------
> > -----
> > >   2 files changed, 40 insertions(+), 67 deletions(-)
> > >
> > >
> > > Testing Commands:
> > > -----------------
> > > N/A
> > >
> > > Testing, Expected Results:
> > > --------------------------
> > > N/A
> > >
> > > Conditions of Submission:
> > > -------------------------
> > > N/A
> > >
> > > Arch      Built     Started    Linux distro
> > > -------------------------------------------
> > > mips        n          n
> > > mips64      n          n
> > > x86         n          n
> > > x86_64      y          y
> > > powerpc     n          n
> > > powerpc64   n          n
> > >
> > >
> > > Reviewer Checklist:
> > > -------------------
> > > [Submitters: make sure that your review doesn't trigger any
> > > checkmarks!]
> > >
> > >
> > > Your checkin has not passed review because (see checked entries):
> > >
> > > ___ Your RR template is generally incomplete; it has too many blank
> entries
> > >      that need proper data filled in.
> > >
> > > ___ You have failed to nominate the proper persons for review and push.
> > >
> > > ___ Your patches do not have proper short+long header
> > >
> > > ___ You have grammar/spelling in your header that is unacceptable.
> > >
> > > ___ You have exceeded a sensible line length in your
> > headers/comments/text.
> > >
> > > ___ You have failed to put in a proper Trac Ticket # into your commits.
> > >
> > > ___ You have incorrectly put/left internal data in your comments/files
> > >      (i.e. internal bug tracking tool IDs, product names etc)
> > >
> > > ___ You have not given any evidence of testing beyond basic build tests.
> > >      Demonstrate some level of runtime or other sanity testing.
> > >
> > > ___ You have ^M present in some of your files. These have to be
> removed.
> > >
> > > ___ You have needlessly changed whitespace or added whitespace crimes
> > >      like trailing spaces, or spaces before tabs.
> > >
> > > ___ You have mixed real technical changes with whitespace and other
> > >      cosmetic code cleanup changes. These have to be separate commits.
> > >
> > > ___ You need to refactor your submission into logical chunks; there is
> > >      too much content into a single commit.
> > >
> > > ___ You have extraneous garbage in your review (merge commits etc)
> > >
> > > ___ You have giant attachments which should never have been sent;
> > >      Instead you should place your content in a public tree to be pulled.
> > >
> > > ___ You have too many commits attached to an e-mail; resend as
> threaded
> > >      commits, or place in a public tree for a pull.
> > >
> > > ___ You have resent this content multiple times without a clear indication
> > >      of what has changed between each re-send.
> > >
> > > ___ You have failed to adequately and individually address all of the
> > >      comments and change requests that were proposed in the initial
> review.
> > >
> > > ___ You have a misconfigured ~/.gitconfig file (i.e. user.name,
> > > user.email
> > etc)
> > >
> > > ___ Your computer have a badly configured date and time; confusing the
> > >      the threaded patch review.
> > >
> > > ___ Your changes affect IPC mechanism, and you don't present any
> results
> > >      for in-service upgradability test.
> > >
> > > ___ Your changes affect user manual and documentation, your patch
> series
> > >      do not contain the patch that updates the Doxygen manual.
> > >




_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to