On Nov 8, 2011, at 5:12 PM, Jesse Gross wrote:

> I got some sparse errors:
> /home/jesse/openvswitch/datapath/linux/flow.c:205:40: warning:
> restricted __be32 degrades to integer
> /home/jesse/openvswitch/datapath/linux/flow.c:205:25: warning:
> incorrect type in assignment (different base types)
> /home/jesse/openvswitch/datapath/linux/flow.c:205:25:    expected
> restricted __be32 [usertype] label
> /home/jesse/openvswitch/datapath/linux/flow.c:205:25:    got unsigned int
> 
> I think it's just an incorrect cast though, not an actual problem.

Thanks.  I cast it to "__be32".

>> @@ -73,6 +73,7 @@ struct sw_flow_key {
>> ...
>> +                       __be32 label;
> 
> Can you add a comment next to this?

Sure.

>> @@ -322,6 +322,7 @@ struct ovs_key_ipv4 {
>>  struct ovs_key_ipv6 {
>> ...
>> +       __be32 ipv6_label;
> 
> Can you add a comment as to how the 20 bits are placed?

No problem.

> We also need to update ODPUTIL_FLOW_KEY_BYTES in lib/odp-util.h.

Good catch.  Thanks.

>> @@ -385,10 +385,10 @@ format_odp_key_attr(const struct nlattr *a, struct ds 
>> *ds)
>> ...
>> +        ds_put_format(ds, 
>> "ipv6(src=%s,dst=%s,label=0x%"PRIx32",proto=%"PRId8
> 
> I think the more canonical form would be to use # instead of 0x explicitly.


That's my preferred form, too.  That file mostly uses "0x", but I went ahead 
and changed this instance to "#".  We should probably be consistent, regardless.

An incremental is at the end of this message.

--Justin


-=-=-=-=-=-=-=-=-=-


diff --git a/datapath/flow.c b/datapath/flow.c
index cd088ac..a7832a4 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -202,7 +202,7 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow
 
        key->ip.proto = NEXTHDR_NONE;
        key->ip.tos_frag = ipv6_get_dsfield(nh) & ~INET_ECN_MASK;
-       key->ipv6.label = *(u32 *)nh & htonl(IPV6_FLOWINFO_FLOWLABEL);
+       key->ipv6.label = *(__be32 *)nh & htonl(IPV6_FLOWINFO_FLOWLABEL);
        ipv6_addr_copy(&key->ipv6.addr.src, &nh->saddr);
        ipv6_addr_copy(&key->ipv6.addr.dst, &nh->daddr);
 
diff --git a/datapath/flow.h b/datapath/flow.h
index ba8c66a..0471cfc 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -73,7 +73,7 @@ struct sw_flow_key {
                                struct in6_addr src;    /* IPv6 source address. 
                                struct in6_addr dst;    /* IPv6 destination addr
                        } addr;
-                       __be32 label;
+                       __be32 label;           /* IPv6 flow label. */
                        struct {
                                __be16 src;             /* TCP/UDP source port. 
                                __be16 dst;             /* TCP/UDP destination p
diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index c2b4285..62668a6 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -322,7 +322,7 @@ struct ovs_key_ipv4 {
 struct ovs_key_ipv6 {
        __be32 ipv6_src[4];
        __be32 ipv6_dst[4];
-       __be32 ipv6_label;
+       __be32 ipv6_label;      /* 20-bits in least-significant bits. */
        __u8   ipv6_proto;
        __u8   ipv6_tos;
        __u8   ipv6_frag;       /* One of OVS_FRAG_TYPE_*. */
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 4562d09..51cd001 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -385,7 +385,7 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
         inet_ntop(AF_INET6, ipv6_key->ipv6_src, src_str, sizeof src_str);
         inet_ntop(AF_INET6, ipv6_key->ipv6_dst, dst_str, sizeof dst_str);
 
-        ds_put_format(ds, "ipv6(src=%s,dst=%s,label=0x%"PRIx32",proto=%"PRId8
+        ds_put_format(ds, "ipv6(src=%s,dst=%s,label=%#"PRIx32",proto=%"PRId8
                       ",tos=%"PRIu8",frag=%s)",
                       src_str, dst_str, ntohl(ipv6_key->ipv6_label),
                       ipv6_key->ipv6_proto, ipv6_key->ipv6_tos,
diff --git a/lib/odp-util.h b/lib/odp-util.h
index c916abf..8a7fbc6 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -72,13 +72,13 @@ void format_odp_actions(struct ds *, const struct nlattr *od
  *  OVS_KEY_ATTR_ETHERNET     12    --     4     16
  *  OVS_KEY_ATTR_8021Q         4    --     4      8
  *  OVS_KEY_ATTR_ETHERTYPE     2     2     4      8
- *  OVS_KEY_ATTR_IPV6         34     2     4     40
+ *  OVS_KEY_ATTR_IPV6         38     2     4     44
  *  OVS_KEY_ATTR_ICMPV6        2     2     4      8
  *  OVS_KEY_ATTR_ND           28    --     4     32
  *  -------------------------------------------------
- *  total                                       140
+ *  total                                       144
  */
-#define ODPUTIL_FLOW_KEY_BYTES 140
+#define ODPUTIL_FLOW_KEY_BYTES 144
 
 /* A buffer with sufficient size and alignment to hold an nlattr-formatted flow
  * key.  An array of "struct nlattr" might not, in theory, be sufficiently

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

Reply via email to