On Nov 8, 2011, at 10:56 PM, Jesse Gross wrote:

> I think after this patch we actually do need to update the comment in
> datapath/flow.h on tos (it talks about DSCP).  Also that block of
> comments in misaligned.

Okay.  Updated.

> I also discovered that there is ipv4_change_dsfield() function that
> you could use instead of set_ip_tos() and also does the checksum a
> little more efficiently.

Cool.  I've attached a new incremental for these cleanups to the end of this 
message.

> I just realized that I've been reading code for almost 16 hours today.
> I have no idea if any of these comments still make sense.


Yuck.  Yeah, I think I'm going to leave the last patch until the morning.  
Thanks for the reviews.

--Justin


diff --git a/datapath/actions.c b/datapath/actions.c
index e8b0ce3..3296dee 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -21,6 +21,7 @@
 #include <linux/if_vlan.h>
 #include <net/ip.h>
 #include <net/checksum.h>
+#include <net/dsfield.h>
 
 #include "actions.h"
 #include "checksum.h"
@@ -150,12 +151,6 @@ static void set_ip_addr(struct sk_buff *skb, struct iphdr *
        *addr = new_addr;
 }
 
-static void set_ip_tos(struct sk_buff *skb, struct iphdr *nh, u8 new_tos)
-{
-       csum_replace2(&nh->check, htons(nh->tos), htons(new_tos));
-       nh->tos = new_tos;
-}
-
 static int set_ipv4(struct sk_buff *skb, const struct ovs_key_ipv4 *ipv4_key)
 {
        struct iphdr *nh;
@@ -175,7 +170,7 @@ static int set_ipv4(struct sk_buff *skb, const struct ovs_ke
                set_ip_addr(skb, nh, &nh->daddr, ipv4_key->ipv4_dst);
 
        if (ipv4_key->ipv4_tos != nh->tos)
-               set_ip_tos(skb, nh, ipv4_key->ipv4_tos);
+               ipv4_change_dsfield(nh, 0, ipv4_key->ipv4_tos);
 
        return 0;
 }
diff --git a/datapath/flow.h b/datapath/flow.h
index 77a955f..1a3ba6a 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -44,8 +44,8 @@ struct sw_flow_key {
        } eth;
        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_* flags. */
+               u8     tos;             /* IP ToS. */
+               u8     frag;            /* OVS_FRAG_TYPE_* flags. */
        } ip;
        union {
                struct {

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

Reply via email to