Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState

[Zhang Chen]

On 05/02/2017 12:53 PM, Jason Wang wrote:
> On 2017年04月28日 17:47, Zhang Chen wrote:
> > Address Jason Wang's comments add vnet header length to SocketReadState.
>
> Instead of saying this, you can add "Suggested-by: Jason Wang 
> <address@hidden>" above your sign-off.

OK.

> > So we change net_fill_rstate() to read
> > struct  {int size; int vnet_hdr_len; const uint8_t buf[];}.
>
> This makes me thinking about the backward compatibility. I think we'd 
> better try to keep it as much as possible. E.g how about pack 
> vnet_hdr_len into higher bits of size?

Do you means split uint32_t size to uint16_t size and uint16_t 
vnet_hdr_len ?
If yes, we also can't keep compatibility with old version.
Old code send a uint32_t size, we read it as uint16_t size is always wrong.

Thanks
Zhang Chen


> Thanks
>
> > Signed-off-by: Zhang Chen <address@hidden>
> > ---
> >   include/net/net.h |  4 +++-
> >   net/net.c         | 24 ++++++++++++++++++++++--
> >   2 files changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/net.h b/include/net/net.h
> > index 402d913..865cb98 100644
> > --- a/include/net/net.h
> > +++ b/include/net/net.h
> > @@ -115,9 +115,11 @@ typedef struct NICState {
> >   } NICState;
> >     struct SocketReadState {
> > -    int state; /* 0 = getting length, 1 = getting data */
> > +    /* 0 = getting length, 1 = getting vnet header length, 2 = 
> > getting data */
> > +    int state;
> >       uint32_t index;
> >       uint32_t packet_len;
> > +    uint32_t vnet_hdr_len;
> >       uint8_t buf[NET_BUFSIZE];
> >       SocketReadStateFinalize *finalize;
> >   };
> > diff --git a/net/net.c b/net/net.c
> > index f69260f..5a6b5ac 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1639,8 +1639,12 @@ int net_fill_rstate(SocketReadState *rs, const 
> > uint8_t *buf, int size)
> >       unsigned int l;
> >         while (size > 0) {
> > -        /* reassemble a packet from the network */
> > -        switch (rs->state) { /* 0 = getting length, 1 = getting data */
> > +        /* Reassemble a packet from the network.
> > +         * 0 = getting length.
> > +         * 1 = getting vnet header length.
> > +         * 2 = getting data.
> > +         */
> > +        switch (rs->state) {
> >           case 0:
> >               l = 4 - rs->index;
> >               if (l > size) {
> > @@ -1658,6 +1662,22 @@ int net_fill_rstate(SocketReadState *rs, const 
> > uint8_t *buf, int size)
> >               }
> >               break;
> >           case 1:
> > +            l = 4 - rs->index;
> > +            if (l > size) {
> > +                l = size;
> > +            }
> > +            memcpy(rs->buf + rs->index, buf, l);
> > +            buf += l;
> > +            size -= l;
> > +            rs->index += l;
> > +            if (rs->index == 4) {
> > +                /* got vnet header length */
> > +                rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
> > +                rs->index = 0;
> > +                rs->state = 2;
> > +            }
> > +            break;
> > +        case 2:
> >               l = rs->packet_len - rs->index;
> >               if (l > size) {
> >                   l = size;
>
>
>
> .

--
Thanks
Zhang Chen