All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mykola Golub <mgolub@mirantis.com>
To: ceph-devel@vger.kernel.org
Cc: Casey Bodley <casey@linuxbox.com>,
	Matt Benjamin <matt@cohortfs.com>, Sage Weil <sage@redhat.com>
Subject: msg: fixup for 2ffacbe (crc configuration in messenger)
Date: Fri, 23 Jan 2015 14:50:46 +0200	[thread overview]
Message-ID: <20150123125045.GA12920@gmail.com> (raw)

Hi,

It looks like the following commit introcuded a regression:

commit 2ffacbe6efae05f5c426cae34882b3351a5ccfbe
Author: Casey Bodley <casey@linuxbox.com>
Date:   Wed Sep 3 12:29:17 2014 -0400

    msg: crc configuration in messenger
    
    Add new header_crc and data_crc configuration booleans, and use
    them consistently to govern whether CRC is performed in the
    Message encode, decode, and transit paths.
    
    Remove ms_nocrc, changes per Sage.
    Mimimally adapt AsyncMessenger for crcflags.
    
    Signed-off-by: Casey Bodley <casey@linuxbox.com>
    Signed-off-by: Matt Benjamin <matt@cohortfs.com>

I faced problems with forwarding messages from older monitors: after
changing the signature of Message::encode(), not all calls were
updated, as a result route messages sent by a new monitor have crc for
data not calculated, while the old monitor expects it calculated.

Also, in Message::encode() crc for data is calculated when MSG_CRC_DATA
is set, but in decode_message(), Pipe::read/write_message() crc for
data is calculated when MSG_CRC_HEADER is set. This looks wrong to me.

Please rewiew the fixup:

https://github.com/ceph/ceph/pull/3473

Also, the behavior of Pipe::read_message/write_message() has been
changed: previously the method always calculated crc, now they
calculate it only if crc is enabled in the config. This means crc can
not be disabled if there are monitors of older version in the
cluster. I suppose we can't fix this, but it might worth documenting
in the release report?

-- 
Mykola Golub

                 reply	other threads:[~2015-01-23 12:51 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150123125045.GA12920@gmail.com \
    --to=mgolub@mirantis.com \
    --cc=casey@linuxbox.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=matt@cohortfs.com \
    --cc=sage@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.