From: Justin Pettit Date: Thu, 14 Aug 2008 07:02:14 +0000 (-0700) Subject: Switch to new packet-out format and add OFPP_IN_PORT. X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=0226bbc7422d5844147e879e64a6defda0cbddd3;p=sliver-openvswitch.git Switch to new packet-out format and add OFPP_IN_PORT. The original packet-out format allowed multiple actions to be specified for buffered packets, but only a destination port for messages including data. This change makes packet-out more consistent by allowing multiple actions regardless of how the packet is stored. This change also disallows sending packets through the incoming port without explicitly using the OFPP_IN_PORT virtual port. --- diff --git a/datapath/datapath.c b/datapath/datapath.c index 1729730c3..2b40e3caa 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -530,19 +530,39 @@ int dp_set_origin(struct datapath *dp, uint16_t in_port, return -ENOENT; } +static int xmit_skb(struct sk_buff *skb) +{ + int len = skb->len; + if (packet_length(skb) > skb->dev->mtu) { + printk("dropped over-mtu packet: %d > %d\n", + packet_length(skb), skb->dev->mtu); + kfree_skb(skb); + return -E2BIG; + } + + dev_queue_xmit(skb); + + return len; +} + /* Takes ownership of 'skb' and transmits it to 'out_port' on 'dp'. */ int dp_output_port(struct datapath *dp, struct sk_buff *skb, int out_port) { BUG_ON(!skb); - if (out_port == OFPP_FLOOD) - return output_all(dp, skb, 1); - else if (out_port == OFPP_ALL) - return output_all(dp, skb, 0); - else if (out_port == OFPP_CONTROLLER) - return dp_output_control(dp, skb, fwd_save_skb(skb), 0, - OFPR_ACTION); - else if (out_port == OFPP_TABLE) { + switch (out_port){ + case OFPP_IN_PORT: + /* Send it out the port it came in on, which is already set in + * the skb. */ + if (!skb->dev) { + if (net_ratelimit()) + printk("skb device not set forwarding to in_port\n"); + kfree(skb); + return -ESRCH; + } + return xmit_skb(skb); + + case OFPP_TABLE: { struct net_bridge_port *p = skb->dev->br_port; int retval; retval = run_flow_through_tables(dp->chain, skb, @@ -550,25 +570,40 @@ int dp_output_port(struct datapath *dp, struct sk_buff *skb, int out_port) if (retval) kfree_skb(skb); return retval; - } else if (out_port == OFPP_LOCAL) { + } + + case OFPP_FLOOD: + return output_all(dp, skb, 1); + + case OFPP_ALL: + return output_all(dp, skb, 0); + + case OFPP_CONTROLLER: + return dp_output_control(dp, skb, fwd_save_skb(skb), 0, + OFPR_ACTION); + + case OFPP_LOCAL: { struct net_device *dev = dp->netdev; return dev ? dp_dev_recv(dev, skb) : -ESRCH; - } else if (out_port >= 0 && out_port < OFPP_MAX) { + } + + case 0 ... OFPP_MAX-1: { struct net_bridge_port *p = dp->ports[out_port]; - int len = skb->len; if (p == NULL) goto bad_port; - skb->dev = p->dev; - if (packet_length(skb) > skb->dev->mtu) { - printk("dropped over-mtu packet: %d > %d\n", - packet_length(skb), skb->dev->mtu); + if (p->dev == skb->dev) { + /* To send to the input port, must use OFPP_IN_PORT */ kfree_skb(skb); - return -E2BIG; + if (net_ratelimit()) + printk("can't directly forward to input port\n"); + return -EINVAL; } + skb->dev = p->dev; + return xmit_skb(skb); + } - dev_queue_xmit(skb); - - return len; + default: + goto bad_port; } bad_port: diff --git a/datapath/forward.c b/datapath/forward.c index fedcca83d..9717fcd0a 100644 --- a/datapath/forward.c +++ b/datapath/forward.c @@ -328,9 +328,18 @@ recv_packet_out(struct sw_chain *chain, const struct sender *sender, struct sk_buff *skb; struct vlan_ethhdr *mac; int nh_ofs; + struct sw_flow_key key; + int n_actions = ntohs(opo->n_actions); + int act_len = n_actions * sizeof opo->actions[0]; + + if (act_len > (ntohs(opo->header.length) - sizeof *opo)) { + if (net_ratelimit()) + printk("message too short for number of actions\n"); + return -EINVAL; + } if (ntohl(opo->buffer_id) == (uint32_t) -1) { - int data_len = ntohs(opo->header.length) - sizeof *opo; + int data_len = ntohs(opo->header.length) - sizeof *opo - act_len; /* FIXME: there is likely a way to reuse the data in msg. */ skb = alloc_skb(data_len, GFP_ATOMIC); @@ -340,8 +349,7 @@ recv_packet_out(struct sw_chain *chain, const struct sender *sender, /* FIXME? We don't reserve NET_IP_ALIGN or NET_SKB_PAD since * we're just transmitting this raw without examining anything * at those layers. */ - memcpy(skb_put(skb, data_len), opo->u.data, data_len); - dp_set_origin(chain->dp, ntohs(opo->in_port), skb); + memcpy(skb_put(skb, data_len), &opo->actions[n_actions], data_len); skb_set_mac_header(skb, 0); mac = vlan_eth_hdr(skb); @@ -350,22 +358,17 @@ recv_packet_out(struct sw_chain *chain, const struct sender *sender, else nh_ofs = sizeof(struct vlan_ethhdr); skb_set_network_header(skb, nh_ofs); - - dp_output_port(chain->dp, skb, ntohs(opo->out_port)); } else { - struct sw_flow_key key; - int n_acts; - skb = retrieve_skb(ntohl(opo->buffer_id)); if (!skb) return -ESRCH; - dp_set_origin(chain->dp, ntohs(opo->in_port), skb); - - n_acts = (ntohs(opo->header.length) - sizeof *opo) - / sizeof *opo->u.actions; - flow_extract(skb, ntohs(opo->in_port), &key); - execute_actions(chain->dp, skb, &key, opo->u.actions, n_acts); } + + dp_set_origin(chain->dp, ntohs(opo->in_port), skb); + + flow_extract(skb, ntohs(opo->in_port), &key); + execute_actions(chain->dp, skb, &key, opo->actions, n_actions); + return 0; } @@ -399,27 +402,29 @@ add_flow(struct sw_chain *chain, const struct ofp_flow_mod *ofm) { int error = -ENOMEM; int i; - int n_acts; + int n_actions; struct sw_flow *flow; /* To prevent loops, make sure there's no action to send to the * OFP_TABLE virtual port. */ - n_acts = (ntohs(ofm->header.length) - sizeof *ofm) / sizeof *ofm->actions; - for (i=0; iheader.length) - sizeof *ofm) + / sizeof *ofm->actions; + for (i=0; iactions[i]; if (a->type == htons(OFPAT_OUTPUT) && (a->arg.output.port == htons(OFPP_TABLE) - || a->arg.output.port == htons(OFPP_NONE))) { + || a->arg.output.port == htons(OFPP_NONE) + || a->arg.output.port == ofm->match.in_port)) { /* xxx Send fancy new error message? */ goto error; } } /* Allocate memory. */ - flow = flow_alloc(n_acts, GFP_ATOMIC); + flow = flow_alloc(n_actions, GFP_ATOMIC); if (flow == NULL) goto error; @@ -429,12 +434,12 @@ add_flow(struct sw_chain *chain, const struct ofp_flow_mod *ofm) flow->idle_timeout = ntohs(ofm->idle_timeout); flow->hard_timeout = ntohs(ofm->hard_timeout); flow->used = jiffies; - flow->n_actions = n_acts; + flow->n_actions = n_actions; flow->init_time = jiffies; flow->byte_count = 0; flow->packet_count = 0; spin_lock_init(&flow->lock); - memcpy(flow->actions, ofm->actions, n_acts * sizeof *flow->actions); + memcpy(flow->actions, ofm->actions, n_actions * sizeof *flow->actions); /* Act. */ error = chain_insert(chain, flow); @@ -447,8 +452,7 @@ add_flow(struct sw_chain *chain, const struct ofp_flow_mod *ofm) struct sw_flow_key key; flow_used(flow, skb); flow_extract(skb, ntohs(ofm->match.in_port), &key); - execute_actions(chain->dp, skb, &key, - ofm->actions, n_acts); + execute_actions(chain->dp, skb, &key, ofm->actions, n_actions); } else error = -ESRCH; diff --git a/include/openflow.h b/include/openflow.h index 0f99c7a0b..e5d600fac 100644 --- a/include/openflow.h +++ b/include/openflow.h @@ -68,7 +68,7 @@ /* The most significant bit being set in the version field indicates an * experimental OpenFlow version. */ -#define OFP_VERSION 0x86 +#define OFP_VERSION 0x87 #define OFP_MAX_TABLE_NAME_LEN 32 #define OFP_MAX_PORT_NAME_LEN 16 @@ -84,10 +84,13 @@ enum ofp_port { OFPP_MAX = 0x100, /* Fake output "ports". */ + OFPP_IN_PORT = 0xfff8, /* Send the packet out the input port. This + virtual port must be explicitly used + in order to send back out of the input + port. */ OFPP_TABLE = 0xfff9, /* Perform actions in flow table. - * NB: This can only be the destination - * port for packet-out messages. - */ + NB: This can only be the destination + port for packet-out messages. */ OFPP_NORMAL = 0xfffa, /* Process with normal L2/L3 switching. */ OFPP_FLOOD = 0xfffb, /* All physical ports except input port and those disabled by STP. */ @@ -304,13 +307,13 @@ OFP_ASSERT(sizeof(struct ofp_action) == 8); /* Send packet (controller -> datapath). */ struct ofp_packet_out { struct ofp_header header; - uint32_t buffer_id; /* ID assigned by datapath (-1 if none). */ - uint16_t in_port; /* Packet's input port (OFPP_NONE if none). */ - uint16_t out_port; /* Output port (if buffer_id == -1). */ - union { - struct ofp_action actions[0]; /* buffer_id != -1 */ - uint8_t data[0]; /* buffer_id == -1 */ - } u; + uint32_t buffer_id; /* ID assigned by datapath (-1 if none). */ + uint16_t in_port; /* Packet's input port (OFPP_NONE if none). */ + uint16_t n_actions; /* Number of actions. */ + struct ofp_action actions[0]; /* Actions. */ + /* uint8_t data[0]; */ /* Packet data. The length is inferred + from the length field in the header. + (Only meaningful if buffer_id == -1.) */ }; OFP_ASSERT(sizeof(struct ofp_packet_out) == 16); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index cb95f1fe9..7e46999a5 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -186,6 +186,9 @@ static void ofp_print_port_name(struct ds *string, uint16_t port) { const char *name; switch (port) { + case OFPP_IN_PORT: + name = "IN_PORT"; + break; case OFPP_TABLE: name = "TABLE"; break; @@ -307,23 +310,31 @@ static void ofp_packet_out(struct ds *string, const void *oh, size_t len, int verbosity) { const struct ofp_packet_out *opo = oh; + int n_actions = ntohs(opo->n_actions); + int act_len = n_actions * sizeof opo->actions[0]; ds_put_cstr(string, " in_port="); ofp_print_port_name(string, ntohs(opo->in_port)); + ds_put_format(string, " n_actions=%d ", n_actions); + if (act_len > (ntohs(opo->header.length) - sizeof *opo)) { + ds_put_format(string, "***packet too short for number of actions***\n"); + return; + } + ofp_print_actions(string, opo->actions, act_len); + if (ntohl(opo->buffer_id) == UINT32_MAX) { - ds_put_cstr(string, " out_port="); - ofp_print_port_name(string, ntohs(opo->out_port)); + int data_len = len - sizeof *opo - act_len; + ds_put_format(string, " data_len=%d", data_len); if (verbosity > 0 && len > sizeof *opo) { - char *packet = ofp_packet_to_string(opo->u.data, len - sizeof *opo, - len - sizeof *opo); + char *packet = ofp_packet_to_string(&opo->actions[n_actions], + data_len, data_len); ds_put_char(string, '\n'); ds_put_cstr(string, packet); free(packet); } } else { ds_put_format(string, " buffer=%08"PRIx32, ntohl(opo->buffer_id)); - ofp_print_actions(string, opo->u.actions, len - sizeof *opo); } ds_put_char(string, '\n'); } diff --git a/lib/vconn.c b/lib/vconn.c index cdaa933ce..6dfba22c1 100644 --- a/lib/vconn.c +++ b/lib/vconn.c @@ -536,17 +536,20 @@ make_unbuffered_packet_out(const struct buffer *packet, uint16_t in_port, uint16_t out_port) { struct ofp_packet_out *opo; - size_t size = sizeof *opo + packet->size; - struct buffer *out = buffer_new(size); + size_t size = sizeof *opo + sizeof opo->actions[0]; + struct buffer *out = buffer_new(size + packet->size); opo = buffer_put_uninit(out, size); memset(opo, 0, sizeof *opo); opo->header.version = OFP_VERSION; opo->header.type = OFPT_PACKET_OUT; - opo->header.length = htons(size); opo->buffer_id = htonl(UINT32_MAX); opo->in_port = htons(in_port); - opo->out_port = htons(out_port); - memcpy(opo->u.data, packet->data, packet->size); + opo->n_actions = htons(1); + opo->actions[0].type = htons(OFPAT_OUTPUT); + opo->actions[0].arg.output.max_len = htons(0); + opo->actions[0].arg.output.port = htons(out_port); + buffer_put(out, packet->data, packet->size); + update_openflow_length(out); return out; } @@ -555,7 +558,7 @@ make_buffered_packet_out(uint32_t buffer_id, uint16_t in_port, uint16_t out_port) { struct ofp_packet_out *opo; - size_t size = sizeof *opo + sizeof opo->u.actions[0]; + size_t size = sizeof *opo + sizeof opo->actions[0]; struct buffer *out = buffer_new(size); opo = buffer_put_uninit(out, size); memset(opo, 0, size); @@ -564,10 +567,10 @@ make_buffered_packet_out(uint32_t buffer_id, opo->header.length = htons(size); opo->buffer_id = htonl(buffer_id); opo->in_port = htons(in_port); - opo->out_port = htons(out_port); - opo->u.actions[0].type = htons(OFPAT_OUTPUT); - opo->u.actions[0].arg.output.max_len = htons(0); - opo->u.actions[0].arg.output.port = htons(out_port); + opo->n_actions = htons(1); + opo->actions[0].type = htons(OFPAT_OUTPUT); + opo->actions[0].arg.output.max_len = htons(0); + opo->actions[0].arg.output.port = htons(out_port); return out; } diff --git a/switch/datapath.c b/switch/datapath.c index 1d10ed421..e98106395 100644 --- a/switch/datapath.c +++ b/switch/datapath.c @@ -552,11 +552,18 @@ dp_output_port(struct datapath *dp, struct buffer *buffer, output_all(dp, buffer, in_port, 0); } else if (out_port == OFPP_CONTROLLER) { dp_output_control(dp, buffer, in_port, 0, OFPR_ACTION); + } else if (out_port == OFPP_IN_PORT) { + output_packet(dp, buffer, in_port); } else if (out_port == OFPP_TABLE) { if (run_flow_through_tables(dp, buffer, in_port)) { buffer_delete(buffer); } } else { + if (in_port == out_port) { + /* FIXME: ratelimit */ + VLOG_DBG("can't directly forward to input port"); + return; + } output_packet(dp, buffer, out_port); } } @@ -1011,31 +1018,33 @@ recv_packet_out(struct datapath *dp, const struct sender *sender UNUSED, const void *msg) { const struct ofp_packet_out *opo = msg; + struct sw_flow_key key; + struct buffer *buffer; + int n_actions = ntohs(opo->n_actions); + int act_len = n_actions * sizeof opo->actions[0]; + + if (act_len > (ntohs(opo->header.length) - sizeof *opo)) { + VLOG_DBG("message too short for number of actions"); + return -EINVAL; + } if (ntohl(opo->buffer_id) == (uint32_t) -1) { /* FIXME: can we avoid copying data here? */ - int data_len = ntohs(opo->header.length) - sizeof *opo; - struct buffer *buffer = buffer_new(data_len); - buffer_put(buffer, opo->u.data, data_len); - dp_output_port(dp, buffer, - ntohs(opo->in_port), ntohs(opo->out_port)); + int data_len = ntohs(opo->header.length) - sizeof *opo - act_len; + buffer = buffer_new(data_len); + buffer_put(buffer, &opo->actions[n_actions], data_len); } else { - struct sw_flow_key key; - struct buffer *buffer; - int n_acts; - buffer = retrieve_buffer(ntohl(opo->buffer_id)); if (!buffer) { return -ESRCH; } - - n_acts = (ntohs(opo->header.length) - sizeof *opo) - / sizeof *opo->u.actions; - flow_extract(buffer, ntohs(opo->in_port), &key.flow); - execute_actions(dp, buffer, ntohs(opo->in_port), - &key, opo->u.actions, n_acts); } - return 0; + + flow_extract(buffer, ntohs(opo->in_port), &key.flow); + execute_actions(dp, buffer, ntohs(opo->in_port), + &key, opo->actions, n_actions); + + return 0; } static int @@ -1053,7 +1062,7 @@ static int add_flow(struct datapath *dp, const struct ofp_flow_mod *ofm) { int error = -ENOMEM; - int n_acts; + int n_actions; int i; struct sw_flow *flow; @@ -1061,20 +1070,22 @@ add_flow(struct datapath *dp, const struct ofp_flow_mod *ofm) /* To prevent loops, make sure there's no action to send to the * OFP_TABLE virtual port. */ - n_acts = (ntohs(ofm->header.length) - sizeof *ofm) / sizeof *ofm->actions; - for (i=0; iheader.length) - sizeof *ofm) + / sizeof *ofm->actions; + for (i=0; iactions[i]; if (a->type == htons(OFPAT_OUTPUT) && (a->arg.output.port == htons(OFPP_TABLE) - || a->arg.output.port == htons(OFPP_NONE))) { + || a->arg.output.port == htons(OFPP_NONE) + || a->arg.output.port == ofm->match.in_port)) { /* xxx Send fancy new error message? */ goto error; } } /* Allocate memory. */ - flow = flow_alloc(n_acts); + flow = flow_alloc(n_actions); if (flow == NULL) goto error; @@ -1084,10 +1095,10 @@ add_flow(struct datapath *dp, const struct ofp_flow_mod *ofm) flow->idle_timeout = ntohs(ofm->idle_timeout); flow->hard_timeout = ntohs(ofm->hard_timeout); flow->used = flow->created = time_now(); - flow->n_actions = n_acts; + flow->n_actions = n_actions; flow->byte_count = 0; flow->packet_count = 0; - memcpy(flow->actions, ofm->actions, n_acts * sizeof *flow->actions); + memcpy(flow->actions, ofm->actions, n_actions * sizeof *flow->actions); /* Act. */ error = chain_insert(dp->chain, flow); @@ -1102,7 +1113,7 @@ add_flow(struct datapath *dp, const struct ofp_flow_mod *ofm) uint16_t in_port = ntohs(ofm->match.in_port); flow_used(flow, buffer); flow_extract(buffer, in_port, &key.flow); - execute_actions(dp, buffer, in_port, &key, ofm->actions, n_acts); + execute_actions(dp, buffer, in_port, &key, ofm->actions, n_actions); } else { error = -ESRCH; }