From: Justin Pettit Date: Thu, 25 Sep 2008 22:00:23 +0000 (-0700) Subject: Modify VLAN actions to support setting both VID and priority. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=06e22ff0fe709ec119fea34c555e9411a4cce0d9;p=openvswitch Modify VLAN actions to support setting both VID and priority. Stripping VLANs is now done through the OFPAT_STRIP_VLAN action (i.e., you don't specify a magic value in the generirc VLAN action). Also, it is now possible to modify the priority bits associated with the VLAN tag through the OFPAT_SET_VLAN_PCP action. The OFPAT_SET_DL_VLAN has been renamed to OFPAT_SET_VLAN_VID. --- diff --git a/datapath/datapath.h b/datapath/datapath.h index 621906b8..f5225e1b 100644 --- a/datapath/datapath.h +++ b/datapath/datapath.h @@ -28,7 +28,9 @@ /* Actions supported by this implementation. */ #define OFP_SUPPORTED_ACTIONS ( (1 << OFPAT_OUTPUT) \ - | (1 << OFPAT_SET_DL_VLAN) \ + | (1 << OFPAT_SET_VLAN_VID) \ + | (1 << OFPAT_SET_VLAN_PCP) \ + | (1 << OFPAT_STRIP_VLAN) \ | (1 << OFPAT_SET_DL_SRC) \ | (1 << OFPAT_SET_DL_DST) \ | (1 << OFPAT_SET_NW_SRC) \ diff --git a/datapath/forward.c b/datapath/forward.c index ddae5d78..db5e751e 100644 --- a/datapath/forward.c +++ b/datapath/forward.c @@ -90,7 +90,7 @@ static int do_output(struct datapath *dp, struct sk_buff *skb, size_t max_len, } void execute_actions(struct datapath *dp, struct sk_buff *skb, - const struct sw_flow_key *key, + struct sw_flow_key *key, const struct ofp_action *actions, int n_actions, int ignore_no_fwd) { @@ -235,41 +235,53 @@ static struct sk_buff *vlan_pull_tag(struct sk_buff *skb) return skb; } -static struct sk_buff *modify_vlan(struct sk_buff *skb, - const struct sw_flow_key *key, const struct ofp_action *a) +static struct sk_buff *modify_vlan_tci(struct sk_buff *skb, + struct sw_flow_key *key, uint16_t tci, uint16_t mask) { - uint16_t new_id = ntohs(a->arg.vlan_id); - - if (new_id != 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 - & ~(htons(VLAN_VID_MASK))) | a->arg.vlan_id; - } else { - /* Add vlan header */ - - /* xxx The vlan_put_tag function, doesn't seem to work - * xxx reliably when it attempts to use the hardware-accelerated - * xxx version. We'll directly use the software version - * xxx until the problem can be diagnosed. - */ - skb = __vlan_put_tag(skb, new_id); - } + struct vlan_ethhdr *vh = vlan_eth_hdr(skb); + + if (key->dl_vlan != htons(OFP_VLAN_NONE)) { + /* Modify vlan id, but maintain other TCI values */ + vh->h_vlan_TCI = (vh->h_vlan_TCI & ~(htons(mask))) | htons(tci); } else { - /* Remove an existing vlan header if it exists */ - vlan_pull_tag(skb); + /* Add vlan header */ + + /* xxx The vlan_put_tag function, doesn't seem to work + * xxx reliably when it attempts to use the hardware-accelerated + * xxx version. We'll directly use the software version + * xxx until the problem can be diagnosed. + */ + skb = __vlan_put_tag(skb, tci); + vh = vlan_eth_hdr(skb); } + key->dl_vlan = vh->h_vlan_TCI & htons(VLAN_VID_MASK); return skb; } +/* Mask for the priority bits in a vlan header. The kernel doesn't + * define this like it does for VID. */ +#define VLAN_PCP_MASK 0xe000 + struct sk_buff *execute_setter(struct sk_buff *skb, uint16_t eth_proto, - const struct sw_flow_key *key, const struct ofp_action *a) + struct sw_flow_key *key, const struct ofp_action *a) { switch (ntohs(a->type)) { - case OFPAT_SET_DL_VLAN: - skb = modify_vlan(skb, key, a); + case OFPAT_SET_VLAN_VID: { + uint16_t tci = ntohs(a->arg.vlan_vid); + skb = modify_vlan_tci(skb, key, tci, VLAN_VID_MASK); + break; + } + + case OFPAT_SET_VLAN_PCP: { + uint16_t tci = (uint16_t)a->arg.vlan_pcp << 13; + skb = modify_vlan_tci(skb, key, tci, VLAN_PCP_MASK); + break; + } + + case OFPAT_STRIP_VLAN: + vlan_pull_tag(skb); + key->dl_vlan = htons(OFP_VLAN_NONE); break; case OFPAT_SET_DL_SRC: { diff --git a/datapath/forward.h b/datapath/forward.h index 73152f9f..fc2bc607 100644 --- a/datapath/forward.h +++ b/datapath/forward.h @@ -35,10 +35,10 @@ void fwd_discard_all(void); void fwd_exit(void); void execute_actions(struct datapath *, struct sk_buff *, - const struct sw_flow_key *, - const struct ofp_action *, int n_actions, - int ignore_no_fwd); + struct sw_flow_key *, + const struct ofp_action *, int n_actions, + int ignore_no_fwd); struct sk_buff *execute_setter(struct sk_buff *, uint16_t, - const struct sw_flow_key *, const struct ofp_action *); + struct sw_flow_key *, const struct ofp_action *); #endif /* forward.h */ diff --git a/datapath/forward_t.c b/datapath/forward_t.c index c596abb5..da5de50c 100644 --- a/datapath/forward_t.c +++ b/datapath/forward_t.c @@ -498,11 +498,11 @@ test_vlan(void) eh = eth_hdr(skb); orig_id = eh->h_proto; - action.type = htons(OFPAT_SET_DL_VLAN); + action.type = htons(OFPAT_SET_VLAN_VID); // Add a random vlan tag new_id = (uint16_t) random32() & VLAN_VID_MASK; - action.arg.vlan_id = new_id; + action.arg.vlan_vid = new_id; skb = execute_setter(skb, eth_proto, &key, &action); vh = vlan_eth_hdr(skb); if (ntohs(vh->h_vlan_TCI) != new_id) { @@ -521,7 +521,7 @@ test_vlan(void) // Modify the tag new_id = (uint16_t) random32() & VLAN_VID_MASK; - action.arg.vlan_id = new_id; + action.arg.vlan_vid = new_id; skb = execute_setter(skb, eth_proto, &key, &action); vh = vlan_eth_hdr(skb); if (ntohs(vh->h_vlan_TCI) != new_id) { @@ -539,7 +539,7 @@ test_vlan(void) #endif // Remove the tag - action.arg.vlan_id = OFP_VLAN_NONE; + action.type = htons(OFPAT_STRIP_VLAN); skb = execute_setter(skb, eth_proto, &key, &action); eh = eth_hdr(skb); diff --git a/include/openflow.h b/include/openflow.h index f63ceff3..f33c6dbe 100644 --- a/include/openflow.h +++ b/include/openflow.h @@ -63,7 +63,7 @@ /* The most significant bit being set in the version field indicates an * experimental OpenFlow version. */ -#define OFP_VERSION 0x93 +#define OFP_VERSION 0x94 #define OFP_MAX_TABLE_NAME_LEN 32 #define OFP_MAX_PORT_NAME_LEN 16 @@ -323,14 +323,16 @@ struct ofp_packet_in { OFP_ASSERT(sizeof(struct ofp_packet_in) == 20); enum ofp_action_type { - OFPAT_OUTPUT, /* Output to switch port. */ - OFPAT_SET_DL_VLAN, /* VLAN. */ - OFPAT_SET_DL_SRC, /* Ethernet source address. */ - OFPAT_SET_DL_DST, /* Ethernet destination address. */ - OFPAT_SET_NW_SRC, /* IP source address. */ - OFPAT_SET_NW_DST, /* IP destination address. */ - OFPAT_SET_TP_SRC, /* TCP/UDP source port. */ - OFPAT_SET_TP_DST /* TCP/UDP destination port. */ + OFPAT_OUTPUT, /* Output to switch port. */ + OFPAT_SET_VLAN_VID, /* Set the 802.1q VLAN id. */ + OFPAT_SET_VLAN_PCP, /* Set the 802.1q priority. */ + OFPAT_STRIP_VLAN, /* Strip the 802.1q header. */ + OFPAT_SET_DL_SRC, /* Ethernet source address. */ + OFPAT_SET_DL_DST, /* Ethernet destination address. */ + OFPAT_SET_NW_SRC, /* IP source address. */ + OFPAT_SET_NW_DST, /* IP destination address. */ + OFPAT_SET_TP_SRC, /* TCP/UDP source port. */ + OFPAT_SET_TP_DST /* TCP/UDP destination port. */ }; /* An output action sends packets out 'port'. When the 'port' is the @@ -342,18 +344,12 @@ struct ofp_action_output { }; OFP_ASSERT(sizeof(struct ofp_action_output) == 4); -/* The VLAN id is 12-bits, so we'll use the entire 16 bits to indicate - * special conditions. All ones is used to indicate that no VLAN id was - * set, or if used as an action, that the VLAN header should be - * stripped. - */ -#define OFP_VLAN_NONE 0xffff - struct ofp_action { uint16_t type; /* One of OFPAT_* */ union { struct ofp_action_output output; /* OFPAT_OUTPUT: output struct. */ - uint16_t vlan_id; /* OFPAT_SET_DL_VLAN: VLAN id. */ + uint8_t vlan_pcp; /* OFPAT_SET_VLAN_PCP: priority. */ + uint16_t vlan_vid; /* OFPAT_SET_VLAN_VID: VLAN id. */ uint8_t dl_addr[OFP_ETH_ALEN]; /* OFPAT_SET_DL_SRC/DST */ uint32_t nw_addr OFP_PACKED; /* OFPAT_SET_NW_SRC/DST */ uint16_t tp; /* OFPAT_SET_TP_SRC/DST */ @@ -423,6 +419,11 @@ enum ofp_flow_wildcards { */ #define OFP_DL_TYPE_NOT_ETH_TYPE 0x05ff +/* The VLAN id is 12-bits, so we can use the entire 16 bits to indicate + * special conditions. All ones indicates that no VLAN id was set. + */ +#define OFP_VLAN_NONE 0xffff + /* Fields to match against flows */ struct ofp_match { uint32_t wildcards; /* Wildcard fields. */ diff --git a/include/packets.h b/include/packets.h index 83b71849..e7681ad4 100644 --- a/include/packets.h +++ b/include/packets.h @@ -138,7 +138,8 @@ struct llc_snap_header { } __attribute__((packed)); BUILD_ASSERT_DECL(LLC_SNAP_HEADER_LEN == sizeof(struct llc_snap_header)); -#define VLAN_VID 0x0fff +#define VLAN_VID_MASK 0x0fff +#define VLAN_PCP_MASK 0xe000 #define VLAN_HEADER_LEN 4 struct vlan_header { diff --git a/lib/flow.c b/lib/flow.c index c0f07a2f..ebe9050f 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -144,7 +144,7 @@ flow_extract(struct ofpbuf *packet, uint16_t in_port, struct flow *flow) struct vlan_header *vh = pull_vlan(&b); if (vh) { flow->dl_type = vh->vlan_next_type; - flow->dl_vlan = vh->vlan_tci & htons(VLAN_VID); + flow->dl_vlan = vh->vlan_tci & htons(VLAN_VID_MASK); } } memcpy(flow->dl_src, eth->eth_src, ETH_ADDR_LEN); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index bd064f78..a3f5d635 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -266,13 +266,16 @@ ofp_print_action(struct ds *string, const struct ofp_action *a) } break; - case OFPAT_SET_DL_VLAN: - ds_put_cstr(string, "mod_vlan:"); - if (ntohs(a->arg.vlan_id) == OFP_VLAN_NONE) { - ds_put_cstr(string, "strip"); - } else { - ds_put_format(string, "%"PRIu16, ntohs(a->arg.vlan_id)); - } + case OFPAT_SET_VLAN_VID: + ds_put_format(string, "mod_vlan_vid:%"PRIu16, ntohs(a->arg.vlan_vid)); + break; + + case OFPAT_SET_VLAN_PCP: + ds_put_format(string, "mod_vlan_pcp:%"PRIu8, a->arg.vlan_pcp); + break; + + case OFPAT_STRIP_VLAN: + ds_put_cstr(string, "strip_vlan"); break; case OFPAT_SET_DL_SRC: diff --git a/switch/datapath.c b/switch/datapath.c index 1c8e1342..8358bf21 100644 --- a/switch/datapath.c +++ b/switch/datapath.c @@ -71,7 +71,9 @@ extern char serial_num; /* Actions supported by this implementation. */ #define OFP_SUPPORTED_ACTIONS ( (1 << OFPAT_OUTPUT) \ - | (1 << OFPAT_SET_DL_VLAN) \ + | (1 << OFPAT_SET_VLAN_VID) \ + | (1 << OFPAT_SET_VLAN_PCP) \ + | (1 << OFPAT_STRIP_VLAN) \ | (1 << OFPAT_SET_DL_SRC) \ | (1 << OFPAT_SET_DL_DST) \ | (1 << OFPAT_SET_NW_SRC) \ @@ -153,11 +155,12 @@ static int update_port_status(struct sw_port *p); static void send_port_status(struct sw_port *p, uint8_t status); static void del_switch_port(struct sw_port *p); static void execute_actions(struct datapath *, struct ofpbuf *, - int in_port, const struct sw_flow_key *, + int in_port, struct sw_flow_key *, const struct ofp_action *, int n_actions, bool ignore_no_fwd); -static void modify_vlan(struct ofpbuf *buffer, const struct sw_flow_key *key, - const struct ofp_action *a); +static void modify_vlan_tci(struct ofpbuf *buffer, struct sw_flow_key *key, + uint16_t tci, uint16_t mask); +static void strip_vlan(struct ofpbuf *buffer); static void modify_nh(struct ofpbuf *buffer, uint16_t eth_proto, uint8_t nw_proto, const struct ofp_action *a); static void modify_th(struct ofpbuf *buffer, uint16_t eth_proto, @@ -902,7 +905,7 @@ do_output(struct datapath *dp, struct ofpbuf *buffer, int in_port, static void execute_actions(struct datapath *dp, struct ofpbuf *buffer, - int in_port, const struct sw_flow_key *key, + int in_port, struct sw_flow_key *key, const struct ofp_action *actions, int n_actions, bool ignore_no_fwd) { @@ -934,8 +937,21 @@ execute_actions(struct datapath *dp, struct ofpbuf *buffer, max_len = ntohs(a->arg.output.max_len); break; - case OFPAT_SET_DL_VLAN: - modify_vlan(buffer, key, a); + case OFPAT_SET_VLAN_VID: { + uint16_t tci = ntohs(a->arg.vlan_vid); + modify_vlan_tci(buffer, key, tci, VLAN_VID_MASK); + break; + } + + case OFPAT_SET_VLAN_PCP: { + uint16_t tci = (uint16_t)a->arg.vlan_pcp << 13; + modify_vlan_tci(buffer, key, tci, VLAN_PCP_MASK); + break; + } + + case OFPAT_STRIP_VLAN: + strip_vlan(buffer); + key->flow.dl_vlan = htons(OFP_VLAN_NONE); break; case OFPAT_SET_DL_SRC: @@ -1014,48 +1030,55 @@ static void modify_th(struct ofpbuf *buffer, uint16_t eth_proto, } } +/* Modify vlan tag control information (TCI). Only sets the TCI bits + * indicated by 'mask'. If no vlan tag is present, one is added. + */ static void -modify_vlan(struct ofpbuf *buffer, - const struct sw_flow_key *key, const struct ofp_action *a) +modify_vlan_tci(struct ofpbuf *buffer, struct sw_flow_key *key, + uint16_t tci, uint16_t mask) { - uint16_t new_id = a->arg.vlan_id; struct vlan_eth_header *veh; - if (new_id != htons(OFP_VLAN_NONE)) { - if (key->flow.dl_vlan != htons(OFP_VLAN_NONE)) { - /* Modify vlan id, but maintain other TCI values */ - veh = buffer->l2; - veh->veth_tci &= ~htons(VLAN_VID); - veh->veth_tci |= new_id; - } else { - /* Insert new vlan id. */ - struct eth_header *eh = buffer->l2; - struct vlan_eth_header tmp; - memcpy(tmp.veth_dst, eh->eth_dst, ETH_ADDR_LEN); - memcpy(tmp.veth_src, eh->eth_src, ETH_ADDR_LEN); - tmp.veth_type = htons(ETH_TYPE_VLAN); - tmp.veth_tci = new_id; - tmp.veth_next_type = eh->eth_type; - - veh = ofpbuf_push_uninit(buffer, VLAN_HEADER_LEN); - memcpy(veh, &tmp, sizeof tmp); - buffer->l2 = (char*)buffer->l2 - VLAN_HEADER_LEN; - } - } else { - /* Remove an existing vlan header if it exists */ + if (key->flow.dl_vlan != htons(OFP_VLAN_NONE)) { + /* Modify vlan id, but maintain other TCI values */ veh = buffer->l2; - if (veh->veth_type == htons(ETH_TYPE_VLAN)) { - struct eth_header tmp; - - memcpy(tmp.eth_dst, veh->veth_dst, ETH_ADDR_LEN); - memcpy(tmp.eth_src, veh->veth_src, ETH_ADDR_LEN); - tmp.eth_type = veh->veth_next_type; - - buffer->size -= VLAN_HEADER_LEN; - buffer->data = (char*)buffer->data + VLAN_HEADER_LEN; - buffer->l2 = (char*)buffer->l2 + VLAN_HEADER_LEN; - memcpy(buffer->data, &tmp, sizeof tmp); - } + veh->veth_tci &= ~htons(mask); + veh->veth_tci |= htons(tci); + } else { + /* Insert new vlan id. */ + struct eth_header *eh = buffer->l2; + struct vlan_eth_header tmp; + memcpy(tmp.veth_dst, eh->eth_dst, ETH_ADDR_LEN); + memcpy(tmp.veth_src, eh->eth_src, ETH_ADDR_LEN); + tmp.veth_type = htons(ETH_TYPE_VLAN); + tmp.veth_tci = htons(tci); + tmp.veth_next_type = eh->eth_type; + + veh = ofpbuf_push_uninit(buffer, VLAN_HEADER_LEN); + memcpy(veh, &tmp, sizeof tmp); + buffer->l2 = (char*)buffer->l2 - VLAN_HEADER_LEN; + } + + key->flow.dl_vlan = veh->veth_tci & htons(VLAN_VID_MASK); +} + +/* Remove an existing vlan header if it exists. */ +static void +strip_vlan(struct ofpbuf *buffer) +{ + struct vlan_eth_header *veh = buffer->l2; + + if (veh->veth_type == htons(ETH_TYPE_VLAN)) { + struct eth_header tmp; + + memcpy(tmp.eth_dst, veh->veth_dst, ETH_ADDR_LEN); + memcpy(tmp.eth_src, veh->veth_src, ETH_ADDR_LEN); + tmp.eth_type = veh->veth_next_type; + + buffer->size -= VLAN_HEADER_LEN; + buffer->data = (char*)buffer->data + VLAN_HEADER_LEN; + buffer->l2 = (char*)buffer->l2 + VLAN_HEADER_LEN; + memcpy(buffer->data, &tmp, sizeof tmp); } } diff --git a/utilities/dpctl.8 b/utilities/dpctl.8 index 014b2b1b..c31ad04a 100644 --- a/utilities/dpctl.8 +++ b/utilities/dpctl.8 @@ -342,12 +342,20 @@ Outputs the packet on the ``local port,'' which corresponds to the \fBof\fIn\fR network device (see \fBCONTACTING THE CONTROLLER\fR in \fBsecchan\fR(8) for information on the \fBof\fIn\fR network device). -.IP \fBmod_vlan\fR:\fIvlan_id\fR -Modifies the VLAN tag on a packet. If \fIvlan_id\fR is a number, then -the VLAN tag is added or modified as necessary to match the value -specified. If \fIvlan_id\fR is \fBSTRIP\fR, then the VLAN tag is -stripped from the packet if one is present. (This action is not -implemented by all OpenFlow switches.) +.IP \fBmod_vlan_vid\fR:\fIvlan_vid\fR +Modifies the VLAN id on a packet. The VLAN tag is added or modified +as necessary to match the value specified. If the VLAN tag is added, +a priority of zero is used (see the \fBmod_vlan_pcp\fR action to set +this). + +.IP \fBmod_vlan_pcp\fR:\fIvlan_pcp\fR +Modifies the VLAN priority on a packet. The VLAN tag is added or modified +as necessary to match the value specified. Valid values are between 0 +(lowest) and 7 (highest). If the VLAN tag is added, a vid of zero is used +(see the \fBmod_vlan_vid\fR action to set this). + +.IP \fBstrip_vlan\fR +Strips the VLAN tag from a packet if it is present. .RE .IP diff --git a/utilities/dpctl.c b/utilities/dpctl.c index 4a781c0e..dc0d9cee 100644 --- a/utilities/dpctl.c +++ b/utilities/dpctl.c @@ -586,14 +586,14 @@ str_to_action(char *str, struct ofp_action *action, int *n_actions) arg++; } - if (!strcasecmp(act, "mod_vlan")) { - action[i].type = htons(OFPAT_SET_DL_VLAN); - - if (!strcasecmp(arg, "strip")) { - action[i].arg.vlan_id = htons(OFP_VLAN_NONE); - } else { - action[i].arg.vlan_id = htons(str_to_int(arg)); - } + if (!strcasecmp(act, "mod_vlan_vid")) { + action[i].type = htons(OFPAT_SET_VLAN_VID); + action[i].arg.vlan_vid = htons(str_to_int(arg)); + } else if (!strcasecmp(act, "mod_vlan_pcp")) { + action[i].type = htons(OFPAT_SET_VLAN_PCP); + action[i].arg.vlan_pcp = str_to_int(arg); + } else if (!strcasecmp(act, "strip_vlan")) { + action[i].type = htons(OFPAT_STRIP_VLAN); } else if (!strcasecmp(act, "output")) { port = str_to_int(arg); } else if (!strcasecmp(act, "TABLE")) {