From: Ben Pfaff Date: Mon, 30 Dec 2013 22:49:25 +0000 (-0800) Subject: ofproto-dpif: Enable NXAST_SAMPLE only if the datapath supports it. X-Git-Tag: sliver-openvswitch-2.1.90-1~10^2~64 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=4b97b70d;p=sliver-openvswitch.git ofproto-dpif: Enable NXAST_SAMPLE only if the datapath supports it. This prevents using an older datapath from breaking forwarding. CC: Romain Lenglet Signed-off-by: Ben Pfaff --- diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 848c7780b..558598c51 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -87,6 +87,11 @@ struct xbridge { enum ofp_config_flags frag; /* Fragmentation handling. */ bool has_in_band; /* Bridge has in band control? */ bool forward_bpdu; /* Bridge forwards STP BPDUs? */ + + /* True if the datapath supports variable-length + * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions. + * False if the datapath supports only 8-byte (or shorter) userdata. */ + bool variable_length_userdata; }; struct xbundle { @@ -249,7 +254,8 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name, const struct dpif_sflow *sflow, const struct dpif_ipfix *ipfix, const struct netflow *netflow, enum ofp_config_flags frag, - bool forward_bpdu, bool has_in_band) + bool forward_bpdu, bool has_in_band, + bool variable_length_userdata) { struct xbridge *xbridge = xbridge_lookup(ofproto); @@ -301,6 +307,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, const char *name, xbridge->frag = frag; xbridge->miss_rule = miss_rule; xbridge->no_packet_in_rule = no_packet_in_rule; + xbridge->variable_length_userdata = variable_length_userdata; } void @@ -2521,6 +2528,15 @@ xlate_sample_action(struct xlate_ctx *ctx, * the same percentage. */ uint32_t probability = (os->probability << 16) | os->probability; + if (!ctx->xbridge->variable_length_userdata) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + + VLOG_ERR_RL(&rl, "ignoring NXAST_SAMPLE action because datapath " + "lacks support (needs Linux 3.10+ or kernel module from " + "OVS 1.11+)"); + return; + } + ctx->xout->slow |= commit_odp_actions(&ctx->xin->flow, &ctx->base_flow, &ctx->xout->odp_actions, &ctx->xout->wc, diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h index 68076ca5f..982f1a468 100644 --- a/ofproto/ofproto-dpif-xlate.h +++ b/ofproto/ofproto-dpif-xlate.h @@ -129,7 +129,7 @@ void xlate_ofproto_set(struct ofproto_dpif *, const char *name, const struct mbridge *, const struct dpif_sflow *, const struct dpif_ipfix *, const struct netflow *, enum ofp_config_flags, bool forward_bpdu, - bool has_in_band) + bool has_in_band, bool variable_length_userdata) OVS_REQ_WRLOCK(xlate_rwlock); void xlate_remove_ofproto(struct ofproto_dpif *) OVS_REQ_WRLOCK(xlate_rwlock); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 52759b563..e48cc00b3 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -42,6 +42,7 @@ #include "netdev-vport.h" #include "netdev.h" #include "netlink.h" +#include "netlink-socket.h" #include "nx-match.h" #include "odp-util.h" #include "odp-execute.h" @@ -251,6 +252,11 @@ struct dpif_backer { enum revalidate_reason need_revalidate; /* Revalidate all flows. */ bool recv_set_enable; /* Enables or disables receiving packets. */ + + /* True if the datapath supports variable-length + * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions. + * False if the datapath supports only 8-byte (or shorter) userdata. */ + bool variable_length_userdata; }; /* All existing ofproto_backer instances, indexed by ofproto->up.type. */ @@ -552,7 +558,8 @@ type_run(const char *type) ofproto->sflow, ofproto->ipfix, ofproto->netflow, ofproto->up.frag_handling, ofproto->up.forward_bpdu, - connmgr_has_in_band(ofproto->up.connmgr)); + connmgr_has_in_band(ofproto->up.connmgr), + ofproto->backer->variable_length_userdata); HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) { xlate_bundle_set(ofproto, bundle, bundle->name, @@ -774,6 +781,8 @@ struct odp_garbage { odp_port_t odp_port; }; +static bool check_variable_length_userdata(struct dpif_backer *backer); + static int open_dpif_backer(const char *type, struct dpif_backer **backerp) { @@ -872,6 +881,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) close_dpif_backer(backer); return error; } + backer->variable_length_userdata = check_variable_length_userdata(backer); if (backer->recv_set_enable) { udpif_set_threads(backer->udpif, n_handlers, n_revalidators); @@ -880,6 +890,86 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) return error; } +/* Tests whether 'backer''s datapath supports variable-length + * OVS_USERSPACE_ATTR_USERDATA in OVS_ACTION_ATTR_USERSPACE actions. We need + * to disable some features on older datapaths that don't support this + * feature. + * + * Returns false if 'backer' definitely does not support variable-length + * userdata, true if it seems to support them or if at least the error we get + * is ambiguous. */ +static bool +check_variable_length_userdata(struct dpif_backer *backer) +{ + struct eth_header *eth; + struct ofpbuf actions; + struct ofpbuf key; + struct ofpbuf packet; + size_t start; + int error; + + /* Compose a userspace action that will cause an ERANGE error on older + * datapaths that don't support variable-length userdata. + * + * We really test for using userdata longer than 8 bytes, but older + * datapaths accepted these, silently truncating the userdata to 8 bytes. + * The same older datapaths rejected userdata shorter than 8 bytes, so we + * test for that instead as a proxy for longer userdata support. */ + ofpbuf_init(&actions, 64); + start = nl_msg_start_nested(&actions, OVS_ACTION_ATTR_USERSPACE); + nl_msg_put_u32(&actions, OVS_USERSPACE_ATTR_PID, + dpif_port_get_pid(backer->dpif, ODPP_NONE)); + nl_msg_put_unspec_zero(&actions, OVS_USERSPACE_ATTR_USERDATA, 4); + nl_msg_end_nested(&actions, start); + + /* Compose an ODP flow key. The key is arbitrary but it must match the + * packet that we compose later. */ + ofpbuf_init(&key, 64); + nl_msg_put_u32(&key, OVS_KEY_ATTR_IN_PORT, 0); + nl_msg_put_unspec_zero(&key, OVS_KEY_ATTR_ETHERNET, + sizeof(struct ovs_key_ethernet)); + nl_msg_put_be16(&key, OVS_KEY_ATTR_ETHERTYPE, htons(0x1234)); + + /* Compose a packet that matches the key. */ + ofpbuf_init(&packet, ETH_HEADER_LEN); + eth = ofpbuf_put_zeros(&packet, ETH_HEADER_LEN); + eth->eth_type = htons(0x1234); + + /* Execute the actions. On older datapaths this fails with -ERANGE, on + * newer datapaths it succeeds. */ + error = dpif_execute(backer->dpif, key.data, key.size, + actions.data, actions.size, &packet, false); + + ofpbuf_uninit(&packet); + ofpbuf_uninit(&key); + ofpbuf_uninit(&actions); + + switch (error) { + case 0: + /* Variable-length userdata is supported. + * + * Purge received packets to avoid processing the nonsense packet we + * sent to userspace, then report success. */ + dpif_recv_purge(backer->dpif); + return true; + + case ERANGE: + /* Variable-length userdata is not supported. */ + VLOG_WARN("%s: datapath does not support variable-length userdata " + "feature (needs Linux 3.10+ or kernel module from OVS " + "1..11+). The NXAST_SAMPLE action will be ignored.", + dpif_name(backer->dpif)); + return false; + + default: + /* Something odd happened. We're not sure whether variable-length + * userdata is supported. Default to "yes". */ + VLOG_WARN("%s: variable-length userdata feature probe failed (%s)", + dpif_name(backer->dpif), ovs_strerror(error)); + return true; + } +} + static int construct(struct ofproto *ofproto_) {