mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-22 01:51:26 +00:00
Code Issues Releases Wiki Activity

tunnel: make tun_key_to_attr aware of tunnel type.

Browse Source 
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 <xiaoyanj@vmware.com>
Signed-off-by: William Tu <u9012063@gmail.com>
Cc: Greg Rose <gvrose8192@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
This commit is contained in:
William Tu 2018-05-14 11:46:47 -07:00 committed by Ben Pfaff
parent 07aec2ac19
commit c6d8720137
8 changed files with 62 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/dpif.c
View File

@ -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));

28
lib/odp-util.c
View File

@ -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);
}
}

6
lib/odp-util.h
View File

@ -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);

1
manpages.mk
View File

@ -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:

10
ofproto/ofproto-dpif-xlate.c
View File

@ -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 {

10
ofproto/tunnel.c
View File

@ -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,

1
ofproto/tunnel.h
View File

@ -44,6 +44,7 @@ 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

18
tests/tunnel.at
View File

@ -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