On Sat, Feb 20, 2016 at 7:44 PM, Alex Elder <elder@xxxxxxxx> wrote:
> +On 02/20/2016 10:45 AM, Ilya Dryomov wrote:
>> ceph_msg_footer is 21 bytes long, while ceph_msg_footer_old is only 13.
>> Don't skip too much when CEPH_FEATURE_MSG_AUTH isn't negotiated.
>
> This looks good, but I have a few suggestions.
>
> You could done some factoring in prepare_write_message_footer()
> to use the new function for setting iov_len and updating
> out_kvec_bytes.
I considered it, but we'd still need the if on MSG_AUTH bit, so
I decided it wasn't worth it. TBH I'm more inclined to rip this
old_footer stuff entirely - it's supported since v0.55, that's
pre-bobtail...
>
> Consider my suggestions, but otherwise I believe this
> is correct.
>
> Reviewed-by: Alex Elder <elder@xxxxxxxxxx>
>
>> Cc: stable@xxxxxxxxxxxxxxx # 3.19+
>> Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx>
>> ---
>> net/ceph/messenger.c | 17 ++++++++++-------
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
>> index fec20819a5ea..dd7c1b7f932b 100644
>> --- a/net/ceph/messenger.c
>> +++ b/net/ceph/messenger.c
>> @@ -2284,6 +2284,13 @@ static int read_partial_msg_data(struct ceph_connection *con)
>> return 1; /* must return > 0 to indicate success */
>> }
>>
>> +static size_t sizeof_footer(struct ceph_connection *con)
>
> I understand why this is named this way. But it's supplying a
> connection as argument, but a footer is a message attribute.
> So I might call this con_msg_footer_size().
The *size* of footer is a connection attribute though ;)
Thanks,
Ilya
--
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
