On Wed, Jun 15, 2011 at 09:44:33AM +0300, Mikolaj Golub wrote:
>
> On Tue, 14 Jun 2011 12:23:03 +0300 Kostik Belousov wrote:
>
> KB> I do not understand what then happens for the recvfrom(2) call ?
> KB> Would it get some error, or 0 as return and no data, or something else ?
>
> It will wait for data below in another loop ("Now continue to read any data
> mbufs off of the head...").
>
> Elaborating, I would split soreceive_generic on three logical parts.
>
> In the first (restart) part we block until some data are received and also
> (without the patch) in the case of MSG_WAITALL if the buffer is big enough we
> block until all MSG_WAITALL request is received (actually it will spin in
> "goto restart" loop until some condition becomes invalid).
>
> The second part is some processing of received data and the third part is a
> "while" loop where data is copied to userspace and in the case of MSG_WAITALL
> request if not all data is received to satisfy the request it also waits for
> this data.
>
> My patch removes the condition in the first part in the case of MSG_WAITALL to
> wait for all data if buffer is big enough. We always will wait for the rest of
> data in the third part. It might be not so effective, and this is my first
> concern about the patch (although not big :-).
Now I think that this part of the patch is right.
The loop in the soreceive_generic() would behave as I would expect
it for MSG_WAITALL. It copyout the received data to userspace by
received chunks.I do not understand your note about effectiveness there. > > KB> Also, what is the MT_CONTROL chunk about ? > > When I removed the condition to skip blocking in the first part I started to > observe panic on KASSERT(m->m_type == MT_DATA) for the following scenario > (produced by HAST): > > sender: > > send(4 bytes); /* send protocol name */ > sendmsg(); /* send descriptor (normal data is empty, descriptor in > control data) */ > > receiver: > > recv(127 bytes, MSG_WAITALL); /* recive protocol name */ > recvmsg(); /* recive descriptor */ > > Although the recv() has MSG_WAITALL, it exits after receiving 4 bytes because > the next received data is of different (MT_CONTROL) type. An it panicked when > got control data. > > It is unclear for me why it is not expected to have MT_CONTROL data in that > part. We do have processing of MT_CONTROL above (in the second part) in the > code but I still a have feeling that it is possible to create some scenario to > break this assert without my patch too, but I have failed so far. And this is > my second concern about my patch, big enough, because for now I am not sure > that this is correct. Although I have not observed issues with it so far... I have no idea about this part. > > Also, I am not sure if there is sense to bother with soreceive_generic() at > all. May be it is more perspective to spend time on "maturing" > soreceive_stream(). As I see it is going to be a replacement for > soreceive_generic() for stream sockets. > > -- > Mikolaj Golub
pgpvI7MVDwreM.pgp
Description: PGP signature
