From patchwork Mon May 14 18:46:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: William Tu X-Patchwork-Id: 913189 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="bVxySeRs"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 40l8nn46Vpz9s08 for ; Tue, 15 May 2018 04:47:25 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id EF7862FC8; Mon, 14 May 2018 18:47:22 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 783752FC0 for ; Mon, 14 May 2018 18:47:21 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f195.google.com (mail-pf0-f195.google.com [209.85.192.195]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 13A37196 for ; Mon, 14 May 2018 18:47:19 +0000 (UTC) Received: by mail-pf0-f195.google.com with SMTP id c10-v6so6434867pfi.12 for ; Mon, 14 May 2018 11:47:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=vmzG33yotS/w0mVSH6IXkKctcz48oQ4WXKo5oMPNHIM=; b=bVxySeRsirACTpy0sqetZZ071BfgGWuWbxRw7UyCgDJJb823kPiOLBszJOnh7JneOw b7Ccgc584MbULckpwDoAmFfTAPfpYzIvyD9vb7fB0zjcpW28I41lIwDVTTce5KQ85kVF HHbzsaDsTlYycycpU/nThdtkqWir2nn2v1rOBzh2m9t01y6sxROhuyN2hJ7kdMkxwCIf N+uuJAch0hw9lZsOUJm5j/+2MBjfEah/jYG1TPjSR0L6gj49+Sw8n0sYAFFO9iM7CbX7 7yPavssAwYZoYoRcU6evO1+tDq6rI0u/S+d/8frv6YA99IsYwLVlyVFwsIYwinL+HZNg BXUw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=vmzG33yotS/w0mVSH6IXkKctcz48oQ4WXKo5oMPNHIM=; b=YRtMXl98HhXj//RErF+EB/5ggx1gRNrUO4/OIgXBxk3q1NIiZvaOEedy9j40IXnQ9/ p+N8U5OcHw9yjTv7vWn5EM89AwJA57HYGQmkS1boJQxTGqTl2oEIaLSnUz26eet47t3t iCSMZppd+1Y3U3d1qablp2LAMz4c4ET4zc7rysXzAymmL7xqH8SuITuVvGaSGEbyZepk U/Kvhu+1o6zyCsqFDHoSQbtb9T5pBgC8AWPF8NRCdre62Vfj0CKNgVD3xi01fnn39O6b 1AKCNhSyfCGGFfltDPN2Uxw3txjfqfEMKVMVHArg8n6AdxXVoWex0KaWqctNMCoc4iyn 5MnQ== X-Gm-Message-State: ALKqPwfz6nCWMj0mokTpgYI5ikLgEDhegDlJrPb6bDQrvv07E0dY5J7Q FzM+z15icMb/NvaR0/mLViyV02C/ X-Google-Smtp-Source: AB8JxZowz8Os5BPi6Jn6ipiEUFDI+o/7PW5rozhdY0HP0VmiK2KwQBgSJqvcaLi9GWP2l60EWNCHrA== X-Received: by 2002:a63:755e:: with SMTP id f30-v6mr9663581pgn.149.1526323638233; Mon, 14 May 2018 11:47:18 -0700 (PDT) Received: from sc9-mailhost3.vmware.com ([66.170.99.2]) by smtp.gmail.com with ESMTPSA id l63-v6sm20177452pfi.6.2018.05.14.11.47.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 14 May 2018 11:47:17 -0700 (PDT) From: William Tu To: dev@openvswitch.org Date: Mon, 14 May 2018 11:46:47 -0700 Message-Id: <1526323607-124200-1-git-send-email-u9012063@gmail.com> X-Mailer: git-send-email 2.7.4 X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCHv2] tunnel: make tun_key_to_attr aware of tunnel type. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org When there is a flow rule which forwards a packet from geneve port to another tunnel port, ex: gre, the tun_metadata carried from the geneve port might affect the outgoing port. For example, the datapath action from geneve port output to gre port (1) shows: set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64, geneve({class=0xffff,type=0,len=4,0x123}),flags(df|key))),1 Where the geneve(...) should not exist. When using kernel's tunnel port, this triggers an error saying: "Multiple metadata blocks provided", when there is a rule forwarding the geneve packet to vxlan/erspan tunnel port. A userspace test case using geneve and gre also demonstrates the issue. The patch makes the tun_key_to_attr aware of the tunnel type. So only the relevant output tunnel's options are set. Reported-by: Xiaoyan Jin Signed-off-by: William Tu Cc: Greg Rose --- v1->v2: - Remove unconditionally clean up the tun_metdata --- lib/dpif.c | 2 +- lib/odp-util.c | 28 +++++++++++++++++++--------- lib/odp-util.h | 6 ++++-- manpages.mk | 1 + ofproto/ofproto-dpif-xlate.c | 10 ++++++++-- ofproto/tunnel.c | 10 ++++++++++ ofproto/tunnel.h | 2 ++ tests/tunnel.at | 18 ++++++++++++++++++ 8 files changed, 63 insertions(+), 14 deletions(-) diff --git a/lib/dpif.c b/lib/dpif.c index a1be4fdca944..b098a4c9e064 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -1216,7 +1216,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_, * that we supply as metadata. We have to use a "set" action to * supply it. */ if (md->tunnel.ip_dst) { - odp_put_tunnel_action(&md->tunnel, &execute_actions); + odp_put_tunnel_action(&md->tunnel, &execute_actions, NULL); } ofpbuf_put(&execute_actions, action, NLA_ALIGN(action->nla_len)); diff --git a/lib/odp-util.c b/lib/odp-util.c index 95c584be3458..70188b6c4abd 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -2726,7 +2726,7 @@ odp_tun_key_from_attr(const struct nlattr *attr, struct flow_tnl *tun) static void tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl *tun_key, const struct flow_tnl *tun_flow_key, - const struct ofpbuf *key_buf) + const struct ofpbuf *key_buf, const char *tnl_type) { size_t tun_key_ofs; @@ -2767,7 +2767,14 @@ tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl *tun_key, if (tun_key->flags & FLOW_TNL_F_OAM) { nl_msg_put_flag(a, OVS_TUNNEL_KEY_ATTR_OAM); } - if (tun_key->gbp_flags || tun_key->gbp_id) { + + /* If tnl_type is set to a particular type of output tunnel, + * only put its relevant tunnel metadata to the nlattr. + * If tnl_type is NULL, put tunnel metadata according to the + * 'tun_key'. + */ + if ((!tnl_type || !strcmp(tnl_type, "vxlan")) && + (tun_key->gbp_flags || tun_key->gbp_id)) { size_t vxlan_opts_ofs; vxlan_opts_ofs = nl_msg_start_nested(a, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS); @@ -2775,7 +2782,10 @@ tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl *tun_key, (tun_key->gbp_flags << 16) | ntohs(tun_key->gbp_id)); nl_msg_end_nested(a, vxlan_opts_ofs); } - tun_metadata_to_geneve_nlattr(tun_key, tun_flow_key, key_buf, a); + + if (!tnl_type || !strcmp(tnl_type, "geneve")) { + tun_metadata_to_geneve_nlattr(tun_key, tun_flow_key, key_buf, a); + } nl_msg_end_nested(a, tun_key_ofs); } @@ -5409,7 +5419,7 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms *parms, if (flow_tnl_dst_is_set(&flow->tunnel) || export_mask) { tun_key_to_attr(buf, &data->tunnel, &parms->flow->tunnel, - parms->key_buf); + parms->key_buf, NULL); } nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, data->pkt_mark); @@ -5661,7 +5671,7 @@ odp_key_from_dp_packet(struct ofpbuf *buf, const struct dp_packet *packet) nl_msg_put_u32(buf, OVS_KEY_ATTR_PRIORITY, md->skb_priority); if (flow_tnl_dst_is_set(&md->tunnel)) { - tun_key_to_attr(buf, &md->tunnel, &md->tunnel, NULL); + tun_key_to_attr(buf, &md->tunnel, &md->tunnel, NULL, NULL); } nl_msg_put_u32(buf, OVS_KEY_ATTR_SKB_MARK, md->pkt_mark); @@ -6697,10 +6707,10 @@ odp_put_push_eth_action(struct ofpbuf *odp_actions, void odp_put_tunnel_action(const struct flow_tnl *tunnel, - struct ofpbuf *odp_actions) + struct ofpbuf *odp_actions, const char *tnl_type) { size_t offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET); - tun_key_to_attr(odp_actions, tunnel, tunnel, NULL); + tun_key_to_attr(odp_actions, tunnel, tunnel, NULL, tnl_type); nl_msg_end_nested(odp_actions, offset); } @@ -6755,7 +6765,7 @@ commit_masked_set_action(struct ofpbuf *odp_actions, * only on tunneling information. */ void commit_odp_tunnel_action(const struct flow *flow, struct flow *base, - struct ofpbuf *odp_actions) + struct ofpbuf *odp_actions, const char *tnl_type) { /* A valid IPV4_TUNNEL must have non-zero ip_dst; a valid IPv6 tunnel * must have non-zero ipv6_dst. */ @@ -6764,7 +6774,7 @@ commit_odp_tunnel_action(const struct flow *flow, struct flow *base, return; } memcpy(&base->tunnel, &flow->tunnel, sizeof base->tunnel); - odp_put_tunnel_action(&base->tunnel, odp_actions); + odp_put_tunnel_action(&base->tunnel, odp_actions, tnl_type); } } diff --git a/lib/odp-util.h b/lib/odp-util.h index 6fcd1bb0c773..49467c9333ab 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -273,7 +273,8 @@ int parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len, const char *odp_key_fitness_to_string(enum odp_key_fitness); void commit_odp_tunnel_action(const struct flow *, struct flow *base, - struct ofpbuf *odp_actions); + struct ofpbuf *odp_actions, + const char *tnl_type); void commit_masked_set_action(struct ofpbuf *odp_actions, enum ovs_key_attr key_type, const void *key, const void *mask, size_t key_size); @@ -356,7 +357,8 @@ size_t odp_put_userspace_action(uint32_t pid, bool include_actions, struct ofpbuf *odp_actions); void odp_put_tunnel_action(const struct flow_tnl *tunnel, - struct ofpbuf *odp_actions); + struct ofpbuf *odp_actions, + const char *tnl_type); void odp_put_tnl_push_action(struct ofpbuf *odp_actions, struct ovs_action_push_tnl *data); diff --git a/manpages.mk b/manpages.mk index 29fdaa0a1eb5..6ebcf6b704da 100644 --- a/manpages.mk +++ b/manpages.mk @@ -275,6 +275,7 @@ lib/memory-unixctl.man: lib/netdev-dpdk-unixctl.man: lib/service.man: lib/ssl-bootstrap.man: +lib/ssl-peer-ca-cert.man: lib/ssl.man: lib/unixctl.man: lib/vlog-unixctl.man: diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 441fa75d78d1..3cb7b5e35277 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3951,8 +3951,12 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, xlate_report(ctx, OFT_DETAIL, "output to native tunnel"); is_native_tunnel = true; } else { + const char *tnl_type; + xlate_report(ctx, OFT_DETAIL, "output to kernel tunnel"); - commit_odp_tunnel_action(flow, &ctx->base_flow, ctx->odp_actions); + tnl_type = tnl_port_get_type(xport->ofport); + commit_odp_tunnel_action(flow, &ctx->base_flow, + ctx->odp_actions, tnl_type); flow->tunnel = flow_tnl; /* Restore tunnel metadata */ } } else { @@ -5325,9 +5329,11 @@ xlate_sample_action(struct xlate_ctx *ctx, tnl_port_send(xport->ofport, flow, ctx->wc); if (!ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) { struct flow_tnl flow_tnl = flow->tunnel; + const char *tnl_type; + tnl_type = tnl_port_get_type(xport->ofport); commit_odp_tunnel_action(flow, &ctx->base_flow, - ctx->odp_actions); + ctx->odp_actions, tnl_type); flow->tunnel = flow_tnl; } } else { diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c index e0214ced69e2..a040b547e91f 100644 --- a/ofproto/tunnel.c +++ b/ofproto/tunnel.c @@ -712,6 +712,16 @@ tnl_port_get_name(const struct tnl_port *tnl_port) OVS_REQ_RDLOCK(rwlock) return netdev_get_name(tnl_port->netdev); } +const char * +tnl_port_get_type(const struct ofport_dpif *ofport) OVS_REQ_RDLOCK(rwlock) +{ + struct tnl_port *tnl_port; + + tnl_port = tnl_find_ofport(ofport); + return !tnl_port ? NULL : + netdev_get_type(tnl_port->netdev); +} + int tnl_port_build_header(const struct ofport_dpif *ofport, struct ovs_action_push_tnl *data, diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h index 47d3dd56df36..26644f2b429a 100644 --- a/ofproto/tunnel.h +++ b/ofproto/tunnel.h @@ -44,6 +44,8 @@ void tnl_wc_init(struct flow *, struct flow_wildcards *); bool tnl_process_ecn(struct flow *); odp_port_t tnl_port_send(const struct ofport_dpif *, struct flow *, struct flow_wildcards *wc); +const char * +tnl_port_get_type(const struct ofport_dpif *tnl_port); /* Returns true if 'flow' should be submitted to tnl_port_receive(). */ static inline bool diff --git a/tests/tunnel.at b/tests/tunnel.at index 3c217b344f9b..eeadb2f93060 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -708,5 +708,23 @@ AT_CHECK([tail -2 stdout], [0], Datapath actions: set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,flags(df|key))),1 ]) +AT_CHECK([ovs-ofctl add-tlv-map br0 "{class=0xffff,type=0,len=4}->tun_metadata0"]) +AT_CHECK([ovs-ofctl del-flows br0]) + +AT_DATA([flows2.txt], [dnl +priority=1,in_port=1,tun_metadata0=0x123, actions=3 +]) +AT_CHECK([ovs-ofctl add-flows br0 flows2.txt]) + +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=0,len=4,0x123}),flags(csum|key)),in_port(6081),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout]) +AT_CHECK([tail -2 stdout], [0], + [Megaflow: recirc_id=0,eth,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=-df+csum+key,tun_metadata0=0x123,in_port=1,nw_ecn=0,nw_frag=no +Datapath actions: set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,flags(df|key))),1 +]) + +dnl without the fix, the actions have geneve options: +dnl set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,geneve({class=0xffff,type=0,len=4,0x123}),flags(df|key))),1 +dnl which is not correct + OVS_VSWITCHD_STOP AT_CLEANUP