From: Justin Pettit Date: Sun, 30 Mar 2008 08:41:42 +0000 (-0700) Subject: - Fixed endian issue with action type. Thanks, Ben! X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1558120be789c9c133533aaddfbe528de0bb729c;p=openvswitch - Fixed endian issue with action type. Thanks, Ben! - Removed BUG() call that could be maliciously/accidentally triggered by a faulty controller. - Switched from __constant_htons to htons, which seems preferred. --- diff --git a/datapath/forward.c b/datapath/forward.c index 86c694d2..a154393c 100644 --- a/datapath/forward.c +++ b/datapath/forward.c @@ -84,7 +84,7 @@ static void execute_actions(struct datapath *dp, struct sk_buff *skb, prev_port = -1; } - if (likely(a->type == ntohs(OFPAT_OUTPUT))) { + if (likely(a->type == htons(OFPAT_OUTPUT))) { prev_port = ntohs(a->arg.output.port); max_len = ntohs(a->arg.output.max_len); } else { @@ -129,7 +129,7 @@ static void modify_nh(struct sk_buff *skb, uint16_t eth_proto, new = a->arg.nw_addr; - if (a->type == OFPAT_SET_NW_SRC) + if (a->type == htons(OFPAT_SET_NW_SRC)) field = &nh->saddr; else field = &nh->daddr; @@ -157,7 +157,7 @@ static void modify_th(struct sk_buff *skb, uint16_t eth_proto, if (nw_proto == IPPROTO_TCP) { struct tcphdr *th = tcp_hdr(skb); - if (a->type == OFPAT_SET_TP_SRC) + if (a->type == htons(OFPAT_SET_TP_SRC)) field = &th->source; else field = &th->dest; @@ -167,7 +167,7 @@ static void modify_th(struct sk_buff *skb, uint16_t eth_proto, } else if (nw_proto == IPPROTO_UDP) { struct udphdr *th = udp_hdr(skb); - if (a->type == OFPAT_SET_TP_SRC) + if (a->type == htons(OFPAT_SET_TP_SRC)) field = &th->source; else field = &th->dest; @@ -185,7 +185,7 @@ static struct sk_buff *vlan_pull_tag(struct sk_buff *skb) /* Verify we were given a vlan packet */ - if (vh->h_vlan_proto != __constant_htons(ETH_P_8021Q)) + if (vh->h_vlan_proto != htons(ETH_P_8021Q)) return skb; memmove(skb->data + VLAN_HLEN, skb->data, 2 * VLAN_ETH_ALEN); @@ -204,11 +204,11 @@ static struct sk_buff *modify_vlan(struct sk_buff *skb, uint16_t new_id = a->arg.vlan_id; if (new_id != OFP_VLAN_NONE) { - if (key->dl_vlan != __constant_htons(OFP_VLAN_NONE)) { + if (key->dl_vlan != htons(OFP_VLAN_NONE)) { /* Modify vlan id, but maintain other TCI values */ struct vlan_ethhdr *vh = vlan_eth_hdr(skb); vh->h_vlan_TCI = (vh->h_vlan_TCI - & ~(__constant_htons(VLAN_VID_MASK))) | htons(new_id); + & ~(htons(VLAN_VID_MASK))) | htons(new_id); } else { /* Add vlan header */ skb = vlan_put_tag(skb, new_id); @@ -224,7 +224,7 @@ static struct sk_buff *modify_vlan(struct sk_buff *skb, struct sk_buff *execute_setter(struct sk_buff *skb, uint16_t eth_proto, const struct sw_flow_key *key, const struct ofp_action *a) { - switch (a->type) { + switch (ntohs(a->type)) { case OFPAT_SET_DL_VLAN: skb = modify_vlan(skb, key, a); break; @@ -251,7 +251,8 @@ struct sk_buff *execute_setter(struct sk_buff *skb, uint16_t eth_proto, break; default: - BUG(); + if (net_ratelimit()) + printk("execute_setter: unknown action: %d\n", ntohs(a->type)); } return skb; diff --git a/datapath/forward_t.c b/datapath/forward_t.c index 6f923000..c596abb5 100644 --- a/datapath/forward_t.c +++ b/datapath/forward_t.c @@ -46,7 +46,7 @@ static void set_action_data(struct sk_buff *skb, struct sw_flow_key *key, struct ofp_action *a) { if (key != NULL) { - switch(a->type) { + switch(ntohs(a->type)) { case(OFPAT_SET_DL_SRC): memcpy(a->arg.dl_addr, key->dl_src, sizeof key->dl_src); break; @@ -297,7 +297,7 @@ check_packet(struct sk_buff *skb, struct ofp_action *a, struct pkt *p) } if (a != NULL) { - switch(a->type) { + switch(ntohs(a->type)) { case(OFPAT_SET_DL_SRC): if (memcmp(a->arg.dl_addr, eh->h_source, sizeof eh->h_source) != 0) { unit_fail("Source eth addr has not been set"); @@ -399,6 +399,7 @@ void test_l3_l4(void) { struct ofp_action action; + uint16_t a_type; struct sk_buff *skb; struct sw_flow_key key; unsigned int i, j; @@ -423,10 +424,11 @@ test_l3_l4(void) if (unit_failed()) return; - for (action.type = OFPAT_SET_DL_SRC; - action.type <= OFPAT_SET_TP_DST; - action.type++) + for (a_type = OFPAT_SET_DL_SRC; + a_type <= OFPAT_SET_TP_DST; + a_type++) { + action.type = htons(a_type); set_action_data(skb, NULL, &action); for(j = 0; j < 2; j++) { skb = execute_setter(skb, eth_proto, &key, &action); @@ -447,6 +449,9 @@ test_l3_l4(void) if (ret != 0) break; } + + if (ret == 0) + printk("\nL3/L4 actions test passed.\n"); } int @@ -493,7 +498,7 @@ test_vlan(void) eh = eth_hdr(skb); orig_id = eh->h_proto; - action.type = OFPAT_SET_DL_VLAN; + action.type = htons(OFPAT_SET_DL_VLAN); // Add a random vlan tag new_id = (uint16_t) random32() & VLAN_VID_MASK; @@ -564,8 +569,6 @@ test_vlan(void) printk("\nVLAN actions test passed.\n"); return ret; - - } /* diff --git a/datapath/linux-2.4/compat-2.4/include/linux/if_vlan.h b/datapath/linux-2.4/compat-2.4/include/linux/if_vlan.h index 21629460..83163b7a 100644 --- a/datapath/linux-2.4/compat-2.4/include/linux/if_vlan.h +++ b/datapath/linux-2.4/compat-2.4/include/linux/if_vlan.h @@ -40,7 +40,7 @@ static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb, unsigned short t memmove(skb->data, skb->data + VLAN_HLEN, 2 * VLAN_ETH_ALEN); /* first, the ethernet type */ - veth->h_vlan_proto = __constant_htons(ETH_P_8021Q); + veth->h_vlan_proto = htons(ETH_P_8021Q); /* now, the tag */ veth->h_vlan_TCI = htons(tag);