diff mbox

[V4,05/12] net/net.c: Add vnet_hdr support in SocketReadState

Message ID 1494553288-30764-6-git-send-email-zhangchen.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang Chen May 12, 2017, 1:41 a.m. UTC
Address Jason Wang's comments add vnet header length to SocketReadState.
We add a flag to dicide whether net_fill_rstate() to read
struct  {int size; int vnet_hdr_len; const uint8_t buf[];} or not.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 include/net/net.h   |  9 +++++++--
 net/colo-compare.c  |  4 ++--
 net/filter-mirror.c |  2 +-
 net/net.c           | 36 ++++++++++++++++++++++++++++++++----
 net/socket.c        |  2 +-
 5 files changed, 43 insertions(+), 10 deletions(-)

Comments

Jason Wang May 15, 2017, 4:02 a.m. UTC | #1
On 2017年05月12日 09:41, Zhang Chen wrote:
> Address Jason Wang's comments add vnet header length to SocketReadState.

Better to use "Suggested-by" instead of this :).

> We add a flag to dicide whether net_fill_rstate() to read
> struct  {int size; int vnet_hdr_len; const uint8_t buf[];} or not.

Few words to explain the format is better than e.g a struct.

>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   include/net/net.h   |  9 +++++++--
>   net/colo-compare.c  |  4 ++--
>   net/filter-mirror.c |  2 +-
>   net/net.c           | 36 ++++++++++++++++++++++++++++++++----
>   net/socket.c        |  2 +-
>   5 files changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 70edfc0..0763636 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -113,14 +113,19 @@ 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;
>   };
>   
> -int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size);
> +int net_fill_rstate(SocketReadState *rs,
> +                    const uint8_t *buf,
> +                    int size,
> +                    bool vnet_hdr);

Why not just move vnet_hdr to SocketReadState?

Thanks

>   char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>   NetClientState *qemu_find_netdev(const char *id);
>   int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 4ab80b1..332f57e 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -530,7 +530,7 @@ static void compare_pri_chr_in(void *opaque, const uint8_t *buf, int size)
>       CompareState *s = COLO_COMPARE(opaque);
>       int ret;
>   
> -    ret = net_fill_rstate(&s->pri_rs, buf, size);
> +    ret = net_fill_rstate(&s->pri_rs, buf, size, false);
>       if (ret == -1) {
>           qemu_chr_fe_set_handlers(&s->chr_pri_in, NULL, NULL, NULL,
>                                    NULL, NULL, true);
> @@ -547,7 +547,7 @@ static void compare_sec_chr_in(void *opaque, const uint8_t *buf, int size)
>       CompareState *s = COLO_COMPARE(opaque);
>       int ret;
>   
> -    ret = net_fill_rstate(&s->sec_rs, buf, size);
> +    ret = net_fill_rstate(&s->sec_rs, buf, size, false);
>       if (ret == -1) {
>           qemu_chr_fe_set_handlers(&s->chr_sec_in, NULL, NULL, NULL,
>                                    NULL, NULL, true);
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index a65853c..4649416 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -134,7 +134,7 @@ static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
>       MirrorState *s = FILTER_REDIRECTOR(nf);
>       int ret;
>   
> -    ret = net_fill_rstate(&s->rs, buf, size);
> +    ret = net_fill_rstate(&s->rs, buf, size, s->vnet_hdr);
>   
>       if (ret == -1) {
>           qemu_chr_fe_set_handlers(&s->chr_in, NULL, NULL, NULL,
> diff --git a/net/net.c b/net/net.c
> index a00a0c9..a9c97cf 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1618,13 +1618,20 @@ void net_socket_rs_init(SocketReadState *rs,
>    * 0: success
>    * -1: error occurs
>    */
> -int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
> +int net_fill_rstate(SocketReadState *rs,
> +                    const uint8_t *buf,
> +                    int size,
> +                    bool vnet_hdr)
>   {
>       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) {
> @@ -1638,10 +1645,31 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
>                   /* got length */
>                   rs->packet_len = ntohl(*(uint32_t *)rs->buf);
>                   rs->index = 0;
> -                rs->state = 1;
> +                if (vnet_hdr) {
> +                    rs->state = 1;
> +                } else {
> +                    rs->state = 2;
> +                    rs->vnet_hdr_len = 0;
> +                }
>               }
>               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;
> diff --git a/net/socket.c b/net/socket.c
> index b8c931e..4e58eff 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -182,7 +182,7 @@ static void net_socket_send(void *opaque)
>       }
>       buf = buf1;
>   
> -    ret = net_fill_rstate(&s->rs, buf, size);
> +    ret = net_fill_rstate(&s->rs, buf, size, false);
>   
>       if (ret == -1) {
>           goto eoc;
Zhang Chen May 15, 2017, 7:49 a.m. UTC | #2
On 05/15/2017 12:02 PM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, Zhang Chen wrote:
>> Address Jason Wang's comments add vnet header length to SocketReadState.
>
> Better to use "Suggested-by" instead of this :).

I got it~

>
>> We add a flag to dicide whether net_fill_rstate() to read
>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];} or not.
>
> Few words to explain the format is better than e.g a struct.

OK, will fix it in next version.

>
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   include/net/net.h   |  9 +++++++--
>>   net/colo-compare.c  |  4 ++--
>>   net/filter-mirror.c |  2 +-
>>   net/net.c           | 36 ++++++++++++++++++++++++++++++++----
>>   net/socket.c        |  2 +-
>>   5 files changed, 43 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 70edfc0..0763636 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -113,14 +113,19 @@ 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;
>>   };
>>   -int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int 
>> size);
>> +int net_fill_rstate(SocketReadState *rs,
>> +                    const uint8_t *buf,
>> +                    int size,
>> +                    bool vnet_hdr);
>
> Why not just move vnet_hdr to SocketReadState?

Good idea.

Thanks
Zhang Chen

>
> Thanks
>
>>   char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>>   NetClientState *qemu_find_netdev(const char *id);
>>   int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 4ab80b1..332f57e 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -530,7 +530,7 @@ static void compare_pri_chr_in(void *opaque, 
>> const uint8_t *buf, int size)
>>       CompareState *s = COLO_COMPARE(opaque);
>>       int ret;
>>   -    ret = net_fill_rstate(&s->pri_rs, buf, size);
>> +    ret = net_fill_rstate(&s->pri_rs, buf, size, false);
>>       if (ret == -1) {
>>           qemu_chr_fe_set_handlers(&s->chr_pri_in, NULL, NULL, NULL,
>>                                    NULL, NULL, true);
>> @@ -547,7 +547,7 @@ static void compare_sec_chr_in(void *opaque, 
>> const uint8_t *buf, int size)
>>       CompareState *s = COLO_COMPARE(opaque);
>>       int ret;
>>   -    ret = net_fill_rstate(&s->sec_rs, buf, size);
>> +    ret = net_fill_rstate(&s->sec_rs, buf, size, false);
>>       if (ret == -1) {
>>           qemu_chr_fe_set_handlers(&s->chr_sec_in, NULL, NULL, NULL,
>>                                    NULL, NULL, true);
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index a65853c..4649416 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -134,7 +134,7 @@ static void redirector_chr_read(void *opaque, 
>> const uint8_t *buf, int size)
>>       MirrorState *s = FILTER_REDIRECTOR(nf);
>>       int ret;
>>   -    ret = net_fill_rstate(&s->rs, buf, size);
>> +    ret = net_fill_rstate(&s->rs, buf, size, s->vnet_hdr);
>>         if (ret == -1) {
>>           qemu_chr_fe_set_handlers(&s->chr_in, NULL, NULL, NULL,
>> diff --git a/net/net.c b/net/net.c
>> index a00a0c9..a9c97cf 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1618,13 +1618,20 @@ void net_socket_rs_init(SocketReadState *rs,
>>    * 0: success
>>    * -1: error occurs
>>    */
>> -int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
>> +int net_fill_rstate(SocketReadState *rs,
>> +                    const uint8_t *buf,
>> +                    int size,
>> +                    bool vnet_hdr)
>>   {
>>       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) {
>> @@ -1638,10 +1645,31 @@ int net_fill_rstate(SocketReadState *rs, 
>> const uint8_t *buf, int size)
>>                   /* got length */
>>                   rs->packet_len = ntohl(*(uint32_t *)rs->buf);
>>                   rs->index = 0;
>> -                rs->state = 1;
>> +                if (vnet_hdr) {
>> +                    rs->state = 1;
>> +                } else {
>> +                    rs->state = 2;
>> +                    rs->vnet_hdr_len = 0;
>> +                }
>>               }
>>               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;
>> diff --git a/net/socket.c b/net/socket.c
>> index b8c931e..4e58eff 100644
>> --- a/net/socket.c
>> +++ b/net/socket.c
>> @@ -182,7 +182,7 @@ static void net_socket_send(void *opaque)
>>       }
>>       buf = buf1;
>>   -    ret = net_fill_rstate(&s->rs, buf, size);
>> +    ret = net_fill_rstate(&s->rs, buf, size, false);
>>         if (ret == -1) {
>>           goto eoc;
>
>
>
> .
>
diff mbox

Patch

diff --git a/include/net/net.h b/include/net/net.h
index 70edfc0..0763636 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -113,14 +113,19 @@  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;
 };
 
-int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size);
+int net_fill_rstate(SocketReadState *rs,
+                    const uint8_t *buf,
+                    int size,
+                    bool vnet_hdr);
 char *qemu_mac_strdup_printf(const uint8_t *macaddr);
 NetClientState *qemu_find_netdev(const char *id);
 int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 4ab80b1..332f57e 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -530,7 +530,7 @@  static void compare_pri_chr_in(void *opaque, const uint8_t *buf, int size)
     CompareState *s = COLO_COMPARE(opaque);
     int ret;
 
-    ret = net_fill_rstate(&s->pri_rs, buf, size);
+    ret = net_fill_rstate(&s->pri_rs, buf, size, false);
     if (ret == -1) {
         qemu_chr_fe_set_handlers(&s->chr_pri_in, NULL, NULL, NULL,
                                  NULL, NULL, true);
@@ -547,7 +547,7 @@  static void compare_sec_chr_in(void *opaque, const uint8_t *buf, int size)
     CompareState *s = COLO_COMPARE(opaque);
     int ret;
 
-    ret = net_fill_rstate(&s->sec_rs, buf, size);
+    ret = net_fill_rstate(&s->sec_rs, buf, size, false);
     if (ret == -1) {
         qemu_chr_fe_set_handlers(&s->chr_sec_in, NULL, NULL, NULL,
                                  NULL, NULL, true);
diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index a65853c..4649416 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -134,7 +134,7 @@  static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
     MirrorState *s = FILTER_REDIRECTOR(nf);
     int ret;
 
-    ret = net_fill_rstate(&s->rs, buf, size);
+    ret = net_fill_rstate(&s->rs, buf, size, s->vnet_hdr);
 
     if (ret == -1) {
         qemu_chr_fe_set_handlers(&s->chr_in, NULL, NULL, NULL,
diff --git a/net/net.c b/net/net.c
index a00a0c9..a9c97cf 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1618,13 +1618,20 @@  void net_socket_rs_init(SocketReadState *rs,
  * 0: success
  * -1: error occurs
  */
-int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
+int net_fill_rstate(SocketReadState *rs,
+                    const uint8_t *buf,
+                    int size,
+                    bool vnet_hdr)
 {
     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) {
@@ -1638,10 +1645,31 @@  int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
                 /* got length */
                 rs->packet_len = ntohl(*(uint32_t *)rs->buf);
                 rs->index = 0;
-                rs->state = 1;
+                if (vnet_hdr) {
+                    rs->state = 1;
+                } else {
+                    rs->state = 2;
+                    rs->vnet_hdr_len = 0;
+                }
             }
             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;
diff --git a/net/socket.c b/net/socket.c
index b8c931e..4e58eff 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -182,7 +182,7 @@  static void net_socket_send(void *opaque)
     }
     buf = buf1;
 
-    ret = net_fill_rstate(&s->rs, buf, size);
+    ret = net_fill_rstate(&s->rs, buf, size, false);
 
     if (ret == -1) {
         goto eoc;