On Fri, Dec 8, 2023 at 5:34 AM <xiubli@xxxxxxxxxx> wrote: > > From: Xiubo Li <xiubli@xxxxxxxxxx> > > 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@xxxxxxxxxx> > --- > 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
