Hi,

On 2015年07月17日 06:40, Ben Pfaff wrote:
> On Thu, Jul 16, 2015 at 02:31:10PM +0900, Minoru TAKAHASHI wrote:
>> Maybe I’ve found a problem on "ofputil_encode_group_desc_request" of 
>> ofp-util.c.
>> According to the OFspec v1.5, the last element of 
>> ofp_group_multipart_request has to be filled with zeros.
>>
>>   /* Body of OFPMP_GROUP_STATS and OFPMP_GROUP_DESC requests. */
>>       struct ofp_group_multipart_request {
>>       uint32_t group_id;  /* All groups if OFPG_ALL. */
>>       uint8_t pad[4];     /* Align to 64 bits. */
>>   };
>>   OFP_ASSERT(sizeof(struct ofp_group_multipart_request) == 8);
>>
>> I’ve checked the source code of "ofputil_encode_group_desc_request", such a 
>> process it could not find.
>>
>>   * ofputil_encode_group_desc_request
>>   https://github.com/openvswitch/ovs/blob/master/lib/ofp-util.c#L7274-L7279
>>
>> I guess it should be write in the same way as follows.
>>
>>   * ofputil_encode_group_stats_request
>>   https://github.com/openvswitch/ovs/blob/master/lib/ofp-util.c#L7207-L7213
>>
>> And, I think "ofputil_encode_port_desc_stats_request" also has a similar 
>> problem.
>>
>>   * ofputil_encode_port_desc_stats_request
>>   https://github.com/openvswitch/ovs/blob/master/lib/ofp-util.c#L4009-L4014
>>
>> Please confirm if it’s correct.
> 
> You're right.
> 
> Will you submit a patch?

For starters, I've created patches that fixes the following problem.

>> I’ve checked the source code of "ofputil_encode_group_desc_request", such a 
>> process it could not find.
>>
>>   * ofputil_encode_group_desc_request
>>   https://github.com/openvswitch/ovs/blob/master/lib/ofp-util.c#L7274-L7279

I would like you to review my patches.

thanks,

> 
> Thanks,
> 
> Ben.
> 
>From 24e6b63add0812c4c7d1c9403d11f621df2ace46 Mon Sep 17 00:00:00 2001
From: Minoru TAKAHASHI <takahashi.mino...@gmail.com>
Date: Fri, 17 Jul 2015 13:22:13 +0900
Subject: [PATCH 1/2] openflow: Add OpenFlow1.5 group desc request.

Signed-off-by: Minoru TAKAHASHI <takahashi.mino...@gmail.com>
---
 include/openflow/openflow-1.5.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 4d77818..6348b9a 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -137,6 +137,12 @@ struct ofp15_group_mod {
 };
 OFP_ASSERT(sizeof(struct ofp15_group_mod) == 16);
 
+struct ofp15_group_desc_request {
+    ovs_be32 group_id;         /* All groups if OFPG_ALL. */
+    uint8_t pad[4];            /* Align to 64 bits. */
+};
+OFP_ASSERT(sizeof(struct ofp15_group_desc_request) == 8);
+
 /* Body of reply to OFPMP_GROUP_DESC request. */
 struct ofp15_group_desc_stats {
     ovs_be16 length;              /* Length of this entry. */
-- 
1.9.1

>From bb7ecda3d1460e234f18e89a57f3d0033a016b7f Mon Sep 17 00:00:00 2001
From: Minoru TAKAHASHI <takahashi.mino...@gmail.com>
Date: Fri, 17 Jul 2015 13:22:37 +0900
Subject: [PATCH 2/2] ofp-util: Fix group desc request encoding.

Signed-off-by: Minoru TAKAHASHI <takahashi.mino...@gmail.com>
---
 lib/ofp-util.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 7e2ee4b..1180c24 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -7258,7 +7258,6 @@ ofputil_encode_group_desc_request(enum ofp_version ofp_version,
                                   uint32_t group_id)
 {
     struct ofpbuf *request;
-    ovs_be32 gid;
 
     switch (ofp_version) {
     case OFP10_VERSION:
@@ -7271,12 +7270,14 @@ ofputil_encode_group_desc_request(enum ofp_version ofp_version,
         request = ofpraw_alloc(OFPRAW_OFPST11_GROUP_DESC_REQUEST,
                                ofp_version, 0);
         break;
-    case OFP15_VERSION:
+    case OFP15_VERSION:{
+        struct ofp15_group_desc_request *req;
         request = ofpraw_alloc(OFPRAW_OFPST15_GROUP_DESC_REQUEST,
                                ofp_version, 0);
-        gid = htonl(group_id);
-        ofpbuf_put(request, &gid, sizeof gid);
+        req = ofpbuf_put_zeros(request, sizeof *req);
+        req->group_id = htonl(group_id);
         break;
+    }
     default:
         OVS_NOT_REACHED();
     }
-- 
1.9.1

_______________________________________________
discuss mailing list
discuss@openvswitch.org
http://openvswitch.org/mailman/listinfo/discuss

Reply via email to