From: Ben Pfaff Date: Tue, 26 May 2009 17:44:43 +0000 (-0700) Subject: secchan: Disallow port numbers not supported by datapath. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2bbd77aaf5f3c031bc6d80a30e65676000bb7878;p=openvswitch secchan: Disallow port numbers not supported by datapath. The OpenFlow protocol supports port numbers from 0 to 0xff00. The datapath protocol supports port numbers from 0 to 0x100. The datapath will reject port numbers in the range 0x100 to 0xff00, but the secchan was not screening these out. Thus, attempting to add a flow or send a packet to one of these ports would result in a kernel EINVAL error, instead of a more sensible OpenFlow error. This commit remedies the situation. Thanks to Justin for pointing out the issue. --- diff --git a/lib/vconn.c b/lib/vconn.c index 6af44214..e9afac27 100644 --- a/lib/vconn.c +++ b/lib/vconn.c @@ -1089,7 +1089,7 @@ check_ofp_message_array(const struct ofp_header *msg, uint8_t type, int check_ofp_packet_out(const struct ofp_header *oh, struct ofpbuf *data, - int *n_actionsp) + int *n_actionsp, int max_ports) { const struct ofp_packet_out *opo; unsigned int actions_len, n_actions; @@ -1120,7 +1120,7 @@ check_ofp_packet_out(const struct ofp_header *oh, struct ofpbuf *data, n_actions = actions_len / sizeof(union ofp_action); error = validate_actions((const union ofp_action *) opo->actions, - n_actions); + n_actions, max_ports); if (error) { return error; } @@ -1193,7 +1193,7 @@ check_action_exact_len(const union ofp_action *a, unsigned int len, } static int -check_action_port(int port) +check_action_port(int port, int max_ports) { switch (port) { case OFPP_IN_PORT: @@ -1206,7 +1206,7 @@ check_action_port(int port) return 0; default: - if (port >= 0 && port < OFPP_MAX) { + if (port >= 0 && port < max_ports) { return 0; } VLOG_WARN_RL(&bad_ofmsg_rl, "unknown output port %x", port); @@ -1235,13 +1235,13 @@ check_nicira_action(const union ofp_action *a, unsigned int len) } static int -check_action(const union ofp_action *a, unsigned int len) +check_action(const union ofp_action *a, unsigned int len, int max_ports) { int error; switch (a->type) { case OFPAT_OUTPUT: - error = check_action_port(ntohs(a->output.port)); + error = check_action_port(ntohs(a->output.port), max_ports); if (error) { return error; } @@ -1286,7 +1286,8 @@ check_action(const union ofp_action *a, unsigned int len) } int -validate_actions(const union ofp_action *actions, size_t n_actions) +validate_actions(const union ofp_action *actions, size_t n_actions, + int max_ports) { const union ofp_action *a; @@ -1302,7 +1303,7 @@ validate_actions(const union ofp_action *actions, size_t n_actions) n_slots, slots_left); return ofp_mkerr(OFPET_BAD_ACTION, OFPBAC_BAD_LEN); } - error = check_action(a, len); + error = check_action(a, len, max_ports); if (error) { return error; } diff --git a/lib/vconn.h b/lib/vconn.h index 54893e06..44736d61 100644 --- a/lib/vconn.h +++ b/lib/vconn.h @@ -114,7 +114,7 @@ int check_ofp_message_array(const struct ofp_header *, uint8_t type, size_t size, size_t array_elt_size, size_t *n_array_elts); int check_ofp_packet_out(const struct ofp_header *, struct ofpbuf *data, - int *n_actions); + int *n_actions, int max_ports); struct flow_stats_iterator { const uint8_t *pos, *end; @@ -130,7 +130,8 @@ const union ofp_action *actions_first(struct actions_iterator *, const union ofp_action *, size_t n_actions); const union ofp_action *actions_next(struct actions_iterator *); -int validate_actions(const union ofp_action *, size_t n_actions); +int validate_actions(const union ofp_action *, size_t n_actions, + int max_ports); void normalize_match(struct ofp_match *); diff --git a/secchan/ofproto.c b/secchan/ofproto.c index 534d001f..9a70f3e1 100644 --- a/secchan/ofproto.c +++ b/secchan/ofproto.c @@ -213,6 +213,7 @@ struct ofproto { struct port_array ports; /* Index is ODP port nr; ofport->opp.port_no is * OFP port nr. */ struct shash port_by_name; + uint32_t max_ports; /* Configuration. */ struct switch_status *switch_status; @@ -275,6 +276,7 @@ ofproto_create(const char *datapath, const struct ofhooks *ofhooks, void *aux, struct ofproto **ofprotop) { struct dpifmon *dpifmon; + struct odp_stats stats; struct ofproto *p; struct dpif dpif; int error; @@ -287,6 +289,13 @@ ofproto_create(const char *datapath, const struct ofhooks *ofhooks, void *aux, VLOG_ERR("failed to open datapath %s: %s", datapath, strerror(error)); return error; } + error = dpif_get_dp_stats(&dpif, &stats); + if (error) { + VLOG_ERR("failed to obtain stats for datapath %s: %s", + datapath, strerror(error)); + dpif_close(&dpif); + return error; + } error = dpif_set_listen_mask(&dpif, ODPL_MISS | ODPL_ACTION); if (error) { VLOG_ERR("failed to listen on datapath %s: %s", @@ -321,6 +330,7 @@ ofproto_create(const char *datapath, const struct ofhooks *ofhooks, void *aux, p->dpifmon = dpifmon; port_array_init(&p->ports); shash_init(&p->port_by_name); + p->max_ports = stats.max_ports; /* Initialize submodules. */ p->switch_status = switch_status_create(p); @@ -2099,7 +2109,7 @@ handle_packet_out(struct ofproto *p, struct ofconn *ofconn, flow_t flow; int error; - error = check_ofp_packet_out(oh, &payload, &n_actions); + error = check_ofp_packet_out(oh, &payload, &n_actions, p->max_ports); if (error) { return error; } @@ -2721,7 +2731,7 @@ handle_flow_mod(struct ofproto *p, struct ofconn *ofconn, } error = validate_actions((const union ofp_action *) ofm->actions, - n_actions); + n_actions, p->max_ports); if (error) { return error; }