[2/2] libceph: just wait for more data to be available on the socket

From: Xiubo Li <xiubli@redhat.com>

The messages from ceph maybe split into multiple socket packages
and we just need to wait for all the data to be availiable on the
sokcet.

URL: https://tracker.ceph.com/issues/63586
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
 net/ceph/messenger_v1.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

On Fri, Dec 8, 2023 at 5:34 AM <xiubli@redhat.com> wrote:
>
> From: Xiubo Li <xiubli@redhat.com>
>
> The messages from ceph maybe split into multiple socket packages
> and we just need to wait for all the data to be availiable on the
> sokcet.
>
> URL: https://tracker.ceph.com/issues/63586
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  net/ceph/messenger_v1.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/net/ceph/messenger_v1.c b/net/ceph/messenger_v1.c
> index f9a50d7f0d20..aff81fef932f 100644
> --- a/net/ceph/messenger_v1.c
> +++ b/net/ceph/messenger_v1.c
> @@ -1160,15 +1160,17 @@ static int read_partial_message(struct ceph_connection *con)
>         /* header */
>         size = sizeof(con->v1.in_hdr);
>         end = size;
> -       ret = read_partial(con, end, size, &con->v1.in_hdr);
> -       if (ret <= 0)
> -               return ret;
> +       if (con->v1.in_base_pos < end) {
> +               ret = read_partial(con, end, size, &con->v1.in_hdr);
> +               if (ret <= 0)
> +                       return ret;
>
> -       crc = crc32c(0, &con->v1.in_hdr, offsetof(struct ceph_msg_header, crc));
> -       if (cpu_to_le32(crc) != con->v1.in_hdr.crc) {
> -               pr_err("read_partial_message bad hdr crc %u != expected %u\n",
> -                      crc, con->v1.in_hdr.crc);
> -               return -EBADMSG;
> +               crc = crc32c(0, &con->v1.in_hdr, offsetof(struct ceph_msg_header, crc));
> +               if (cpu_to_le32(crc) != con->v1.in_hdr.crc) {
> +                       pr_err("read_partial_message bad hdr crc %u != expected %u\n",
> +                              crc, con->v1.in_hdr.crc);
> +                       return -EBADMSG;
> +               }
>         }
>
>         front_len = le32_to_cpu(con->v1.in_hdr.front_len);
> --
> 2.43.0
>

Hi Xiubo,

This doesn't seem right to me.  read_partial() is supposed to be called
unconditionally.  On a short read (i.e. when it's unable to fill the
destination buffer -- in this case the header), it returns 0 and the
stack is supposed to unroll all the way up to ceph_con_workfn().

If the destination buffer is already filled, read_partial() does
nothing and returns 1.  Recomputing the header crc in case
read_partial_message() is called repeatedly shouldn't be an issue
because con->v1.in_hdr shouldn't be modified in the interim.  If it
gets modified, it's a bug.

It might help if you provide a step-by-step breakdown of the scenario
that you are trying to address in the commit message.

Thanks,

                Ilya