mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 06:15:47 +00:00
Code Issues Releases Wiki Activity

netdev-offload-tc: Fix the mask for tunnel metadata length.

Browse Source 
'wc.masks.tunnel.metadata.present.len' must be a mask for the same
field in the flow key, but in the tc_flower structure it's the real
length of metadata masks.

That is correctly handled for the individual opt->length, setting all
the masks to 0x1f like it's done in the tun_metadata_to_geneve_mask__(),
but not handled for the main 'len' field.

Fix that by setting the mask to 0xff like it's done during the flow
translation in xlate_actions() and during the flow dump in the
tun_metadata_from_geneve_nlattr().

Also, flower always has an exact match on the present.len field
regardless of its value and regardless of this field being masked
by OVS flow translation layer while installing the flow.  Hence,
all tunnel flows dumped from TC should have an exact match on
present.len and also UDPIF flag, because present.len doesn't make
sense without that flag.  Without the change, zero-length options
match is incorrectly reported as a wildcard match.  The side effect
though is that zero-length match on geneve options is reported even
for other tunnel types, e.g. vxlan.  But that should be fairly
harmless.  To avoid reporting a match on empty geneve options for
vxlan/etc. tunnels we'll need to check the tunnel port type, there
is no enough information in the TUNNEL attribute itself.

Extra checks and comments added around the code to better explain
what is going on.

Fixes: a468645c6d ("lib/tc: add geneve with option match offload")
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
Ilya Maximets
2022-08-14 16:45:59 +02:00
parent 8d35e5fafc
commit 7a6c8074c5
2 changed files with 33 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
lib/netdev-offload-tc.c
View File

@@ -585,30 +585,42 @@ flower_tun_opt_to_match(struct match *match, struct tc_flower *flower)
struct geneve_opt *opt, *opt_mask;
int len, cnt = 0;
/* Options are always in UDPIF format in the 'flower'. */
match->flow.tunnel.flags |= FLOW_TNL_F_UDPIF;
match->wc.masks.tunnel.flags |= FLOW_TNL_F_UDPIF;
match->flow.tunnel.metadata.present.len =
flower->key.tunnel.metadata.present.len;
/* In the 'flower' mask len is an actual length, not a mask. But in the
* 'match' it is an actual mask, so should be an exact match, because TC
* will always match on the exact value. */
match->wc.masks.tunnel.metadata.present.len = 0xff;
if (!flower->key.tunnel.metadata.present.len) {
/* No options present. */
return;
}
memcpy(match->flow.tunnel.metadata.opts.gnv,
flower->key.tunnel.metadata.opts.gnv,
flower->key.tunnel.metadata.present.len);
match->flow.tunnel.metadata.present.len =
flower->key.tunnel.metadata.present.len;
match->flow.tunnel.flags |= FLOW_TNL_F_UDPIF;
memcpy(match->wc.masks.tunnel.metadata.opts.gnv,
flower->mask.tunnel.metadata.opts.gnv,
flower->mask.tunnel.metadata.present.len);
/* Fixing up 'length' fields of particular options, since these are
* also not masks, but actual lengths in the 'flower' structure. */
len = flower->key.tunnel.metadata.present.len;
while (len) {
opt = &match->flow.tunnel.metadata.opts.gnv[cnt];
opt_mask = &match->wc.masks.tunnel.metadata.opts.gnv[cnt];
/* "Exact" match as set in tun_metadata_to_geneve_mask__(). */
opt_mask->length = 0x1f;
cnt += sizeof(struct geneve_opt) / 4 + opt->length;
len -= sizeof(struct geneve_opt) + opt->length * 4;
}
match->wc.masks.tunnel.metadata.present.len =
flower->mask.tunnel.metadata.present.len;
match->wc.masks.tunnel.flags |= FLOW_TNL_F_UDPIF;
}
static void
@@ -1116,9 +1128,8 @@ parse_tc_flower_to_match(struct tc_flower *flower,
if (flower->key.tunnel.tp_dst) {
match_set_tun_tp_dst(match, flower->key.tunnel.tp_dst);
}
if (flower->key.tunnel.metadata.present.len) {
flower_tun_opt_to_match(match, flower);
}
flower_tun_opt_to_match(match, flower);
}
act_off = nl_msg_start_nested(buf, OVS_FLOW_ATTR_ACTIONS);
@@ -1604,6 +1615,8 @@ flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl,
len -= sizeof(struct geneve_opt) + opt->length * 4;
}
/* Copying from the key and not from the mask, since in the 'flower'
* the length for a mask is not a mask, but the actual length. */
flower->mask.tunnel.metadata.present.len = tnl->metadata.present.len;
}

13
lib/tc.c
View File

@@ -721,15 +721,17 @@ flower_tun_geneve_opt_check_len(struct tun_metadata *key,
const struct geneve_opt *opt, *opt_mask;
int len, cnt = 0;
if (key->present.len != mask->present.len) {
goto bad_length;
}
len = key->present.len;
while (len) {
opt = &key->opts.gnv[cnt];
opt_mask = &mask->opts.gnv[cnt];
if (opt->length != opt_mask->length) {
VLOG_ERR_RL(&error_rl,
"failed to parse tun options; key/mask length differ");
return EINVAL;
goto bad_length;
}
cnt += sizeof(struct geneve_opt) / 4 + opt->length;
@@ -737,6 +739,11 @@ flower_tun_geneve_opt_check_len(struct tun_metadata *key,
}
return 0;
bad_length:
VLOG_ERR_RL(&error_rl,
"failed to parse tun options; key/mask length differ");
return EINVAL;
}
static int