On Nov 8, 2011, at 8:44 PM, Jesse Gross wrote:
> On Tue, Nov 8, 2011 at 3:57 PM, Justin Pettit <jpet...@nicira.com> wrote:
>>
>> --- a/lib/odp-util.c
>> +++ b/lib/odp-util.c
>> @@ -370,7 +370,7 @@ format_odp_key_attr(const struct nlattr *a, struct ds
>> *ds)
>> ...
>
> I would use # here as well.
Okay. Changed:
-=-=-=-=-=-=-=-=-=-=-
@@ -370,7 +370,7 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
case OVS_KEY_ATTR_IPV4:
ipv4_key = nl_attr_get(a);
ds_put_format(ds, "ipv4(src="IP_FMT",dst="IP_FMT","
- "proto=%"PRId8",tos=%"PRIu8",frag=%s)",
+ "proto=%"PRId8",tos=%#"PRIx8",frag=%s)",
IP_ARGS(&ipv4_key->ipv4_src),
IP_ARGS(&ipv4_key->ipv4_dst),
ipv4_key->ipv4_proto, ipv4_key->ipv4_tos,
@@ -386,7 +386,7 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
inet_ntop(AF_INET6, ipv6_key->ipv6_dst, dst_str, sizeof dst_str);
ds_put_format(ds, "ipv6(src=%s,dst=%s,label=%#"PRIx32",proto=%"PRId8
- ",tos=%"PRIu8",frag=%s)",
+ ",tos=%#"PRIx8",frag=%s)",
src_str, dst_str, ntohl(ipv6_key->ipv6_label),
ipv6_key->ipv6_proto, ipv6_key->ipv6_tos,
ovs_frag_type_to_string(ipv6_key->ipv6_frag));
-=-=-=-=-=-=-=-=-=-=-
> I also notice that we print both the
> individual DSCP/ECN components and the combined ToS in decimal in some
> places as well, which is a little weird.
Yes, this is because OpenFlow's treatment of DSCP/ECN/TOS is weird. I'm trying
to get it straightened out a bit in OpenFlow 1.2.
The way I currently have it working is to have the kernel only understand a
single "tos" element, which contains DSCP and ECN as you'd expect. We print
this in hex, since it's easier to parse out the DSCP bits from the ECN ones.
In userspace, "nw_tos" is the DSCP bits shifted over by two (as if ECN were
just being masked out of the actual TOS field). This is how OpenFlow 1.0 and
1.1 define it, and we've just always printed it in decimal. The new "nw_ecn"
argument has a range of 0 to 3, so we just always print it in decimal.
If you have a better suggestion, let me know. I'm hoping that we can get some
sanity in 1.2, and then we can just adopt that convention in the userspace.
> You also need to update the set_tos action in the userspace datapath.
Good catch. Here's an incremental.
-=-=-=-=-=-=-=-=-=-=-
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b4e788f..d776899 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1086,12 +1086,9 @@ dp_netdev_set_ip_tos(struct ip_header *nh, uint8_t new_to
{
uint8_t *field = &nh->ip_tos;
- /* Set the DSCP bits and preserve the ECN bits. */
- uint8_t new = new_tos | (nh->ip_tos & IP_ECN_MASK);
-
nh->ip_csum = recalc_csum16(nh->ip_csum, htons((uint16_t)*field),
- htons((uint16_t) new));
- *field = new;
+ htons((uint16_t) new_tos));
+ *field = new_tos;
}
static void
-=-=-=-=-=-=-=-=-=-=-
--Justin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
