On Nov 8, 2011, at 8:12 PM, Jesse Gross wrote:
> On Tue, Nov 8, 2011 at 3:57 PM, Justin Pettit <jpet...@nicira.com> wrote:
>> diff --git a/datapath/flow.h b/datapath/flow.h
>> index ba8c66a..f12b11a 100644
>> --- a/datapath/flow.h
>> +++ b/datapath/flow.h
>> @@ -147,7 +143,7 @@ u64 flow_used_time(unsigned long flow_jiffies);
>> * 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 38 2 4 44
>> + * OVS_KEY_ATTR_IPV6 39 1 4 44
>
> I think this is just a mistake since this patch has nothing IPv6
> specific in it but this list is for the userspace interface, which
> this doesn't change.
I don't understand this or your follow-up about it being incorrect before this
patch. Both the IPv4 and IPv6 "ipvX_tos" fields that expand the struct size by
one. Are you stating that it's inconsistent with "lib/odp-util.h"? If so,
then I've updated them to be in sync.
> In userspace there's a fair amount of masking with IP_DSCP_MASK even
> though it is unnecessary because the userspace flow shouldn't contain
> the ECN bits. I'm guessing that it all gets resolved (or becomes
> necessary) in the patch where ECN support is added, so it doesn't
> really matter.
Correct. Or, at least, that's the hope. :-)
> I don't have any further comments beyond Ben's
At the end of the message, I've included an incremental that includes Ben's
suggestions.
--Justin
diff --git a/datapath/flow.c b/datapath/flow.c
index 285006d..5080ee9 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -693,12 +693,12 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct
offset = nh->frag_off & htons(IP_OFFSET);
if (offset) {
- key->ip.frag |= OVS_FRAG_TYPE_LATER;
+ key->ip.frag = OVS_FRAG_TYPE_LATER;
goto out;
}
if (nh->frag_off & htons(IP_MF) ||
skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
- key->ip.frag |= OVS_FRAG_TYPE_FIRST;
+ key->ip.frag = OVS_FRAG_TYPE_FIRST;
/* Transport layer. */
if (key->ip.proto == IPPROTO_TCP) {
@@ -765,7 +765,7 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct sw
if (key->ip.frag == OVS_FRAG_TYPE_LATER)
goto out;
if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
- key->ip.frag |= OVS_FRAG_TYPE_FIRST;
+ key->ip.frag = OVS_FRAG_TYPE_FIRST;
/* Transport layer. */
if (key->ip.proto == NEXTHDR_TCP) {
diff --git a/datapath/flow.h b/datapath/flow.h
index 7e3dcf9..77a955f 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -45,7 +45,7 @@ struct sw_flow_key {
struct {
u8 proto; /* IP protocol or lower 8 bits of ARP op
u8 tos; /* IP ToS DSCP in high 6 bits. */
- u8 frag; /* OVS_FRAG_TYPE_* in low 2 bits. */
+ u8 frag; /* OVS_FRAG_TYPE_* flags. */
} ip;
union {
struct {
diff --git a/lib/flow.c b/lib/flow.c
index bc53bd8..99cef1e 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -371,7 +371,7 @@ flow_extract(struct ofpbuf *packet, uint32_t priority, ovs_b
flow->tos = nh->ip_tos & IP_DSCP_MASK;
if (IP_IS_FRAGMENT(nh->ip_frag_off)) {
- flow->frag |= FLOW_FRAG_ANY;
+ flow->frag = FLOW_FRAG_ANY;
if (nh->ip_frag_off & htons(IP_FRAG_OFF_MASK)) {
flow->frag |= FLOW_FRAG_LATER;
}
diff --git a/lib/flow.h b/lib/flow.h
index 248233e..af634fb 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -71,9 +71,9 @@ struct flow {
uint8_t dl_dst[6]; /* Ethernet destination address. */
uint8_t nw_proto; /* IP protocol or low 8 bits of ARP opcode. */
uint8_t tos; /* IP ToS. */
- uint8_t frag; /* FLOW_FRAG_*. */
uint8_t arp_sha[6]; /* ARP/ND source hardware address. */
uint8_t arp_tha[6]; /* ARP/ND target hardware address. */
+ uint8_t frag; /* FLOW_FRAG_* flags. */
uint8_t reserved[7]; /* Reserved for 64-bit packing. */
};
@@ -81,8 +81,8 @@ struct flow {
* flow", followed by FLOW_PAD_SIZE bytes of padding. */
#define FLOW_SIG_SIZE (109 + FLOW_N_REGS * 4)
#define FLOW_PAD_SIZE 7
-BUILD_ASSERT_DECL(offsetof(struct flow, arp_tha) == FLOW_SIG_SIZE - 6);
-BUILD_ASSERT_DECL(sizeof(((struct flow *)0)->arp_tha) == 6);
+BUILD_ASSERT_DECL(offsetof(struct flow, frag) == FLOW_SIG_SIZE - 1);
+BUILD_ASSERT_DECL(sizeof(((struct flow *)0)->frag) == 1);
BUILD_ASSERT_DECL(sizeof(struct flow) == FLOW_SIG_SIZE + FLOW_PAD_SIZE);
/* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 8a7fbc6..73c3362 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -72,7 +72,7 @@ void format_odp_actions(struct ds *, const struct nlattr *odp_
* 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 38 2 4 44
+ * OVS_KEY_ATTR_IPV6 39 1 4 44
* OVS_KEY_ATTR_ICMPV6 2 2 4 8
* OVS_KEY_ATTR_ND 28 -- 4 32
* -------------------------------------------------
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
