msg: fixup for 2ffacbe (crc configuration in messenger)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

It looks like the following commit introcuded a regression:

commit 2ffacbe6efae05f5c426cae34882b3351a5ccfbe
Author: Casey Bodley <casey@xxxxxxxxxxxx>
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@xxxxxxxxxxxx>
    Signed-off-by: Matt Benjamin <matt@xxxxxxxxxxxx>

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
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux