Skip to content

Commit

Permalink
netdev-offload-tc: Check geneve metadata length.
Browse files Browse the repository at this point in the history
Currently ovs-vswitchd crashes, with hw offloading enabled, if a geneve
packet with corrupted metadata is received, because the metadata header
is not verified correctly.

This commit adds a check for geneve metadata length and, if the header
is wrong, the packet is not sent to flower.

It also includes a system-traffic test for geneve packets with corrupted
metadata.

Fixes: a468645 ("lib/tc: add geneve with option match offload")
Reported-by: Haresh Khandelwal <hakhande@redhat.com>
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
  • Loading branch information
drizzt authored and igsilya committed Feb 8, 2024
1 parent a6c0a3d commit 935cd1d
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 5 deletions.
25 changes: 20 additions & 5 deletions lib/netdev-offload-tc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1715,12 +1715,12 @@ test_key_and_mask(struct match *match)
return 0;
}

static void
static int
flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl,
struct flow_tnl *tnl_mask)
{
struct geneve_opt *opt, *opt_mask;
int len, cnt = 0;
int tot_opt_len, len, cnt = 0;

/* 'flower' always has an exact match on tunnel metadata length, so having
* it in a wrong format is not acceptable unless it is empty. */
Expand All @@ -1736,7 +1736,7 @@ flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl,
memset(&tnl_mask->metadata.present.map, 0,
sizeof tnl_mask->metadata.present.map);
}
return;
return 0;
}

tnl_mask->flags &= ~FLOW_TNL_F_UDPIF;
Expand All @@ -1750,7 +1750,7 @@ flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl,
sizeof tnl_mask->metadata.present.len);

if (!tnl->metadata.present.len) {
return;
return 0;
}

memcpy(flower->key.tunnel.metadata.opts.gnv, tnl->metadata.opts.gnv,
Expand All @@ -1764,14 +1764,25 @@ flower_match_to_tun_opt(struct tc_flower *flower, const struct flow_tnl *tnl,
* also not masks, but actual lengths in the 'flower' structure. */
len = flower->key.tunnel.metadata.present.len;
while (len) {
if (len < sizeof *opt) {
return EOPNOTSUPP;
}

opt = &flower->key.tunnel.metadata.opts.gnv[cnt];
tot_opt_len = sizeof *opt + opt->length * 4;
if (len < tot_opt_len) {
return EOPNOTSUPP;
}

opt_mask = &flower->mask.tunnel.metadata.opts.gnv[cnt];

opt_mask->length = opt->length;

cnt += sizeof(struct geneve_opt) / 4 + opt->length;
len -= sizeof(struct geneve_opt) + opt->length * 4;
}

return 0;
}

static void
Expand Down Expand Up @@ -2209,7 +2220,11 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
tnl_mask->flags &= ~(FLOW_TNL_F_DONT_FRAGMENT | FLOW_TNL_F_CSUM);

if (!strcmp(netdev_get_type(netdev), "geneve")) {
flower_match_to_tun_opt(&flower, tnl, tnl_mask);
err = flower_match_to_tun_opt(&flower, tnl, tnl_mask);
if (err) {
VLOG_WARN_RL(&warn_rl, "Unable to parse geneve options");
return err;
}
}
flower.tunnel = true;
} else {
Expand Down
33 changes: 33 additions & 0 deletions tests/system-offloads-traffic.at
Original file line number Diff line number Diff line change
Expand Up @@ -855,3 +855,36 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network device ovs-p0/d
/failed to offload flow/d
"])
AT_CLEANUP

AT_SETUP([offloads - handling of geneve corrupted metadata - offloads enabled])
OVS_CHECK_GENEVE()

OVS_TRAFFIC_VSWITCHD_START(
[_ADD_BR([br-underlay]) -- \
set bridge br0 other-config:hwaddr=f2:ff:00:00:00:01 -- \
set bridge br-underlay other-config:hwaddr=f2:ff:00:00:00:02],
[], [-- set Open_vSwitch . other_config:hw-offload=true])

AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])

ADD_NAMESPACES(at_ns0)

dnl Set up underlay link from host into the namespace using veth pair.
ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
AT_CHECK([ip link set dev br-underlay up])

dnl Set up tunnel endpoints on OVS outside the namespace and with a native
dnl linux device inside the namespace.
ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.31.1.1], [10.1.1.100/24])
ADD_NATIVE_TUNNEL([geneve], [ns_gnv0], [at_ns0], [172.31.1.100], [10.1.1.1/24],
[vni 0], [address f2:ff:00:00:00:04])

NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f2 ff 00 00 00 02 f2 ff 00 00 00 03 08 00 45 00 00 52 00 01 00 00 40 11 1f f7 ac 1f 01 01 ac 1f 01 64 de c1 17 c1 00 3e 59 e9 01 00 65 58 00 00 00 00 00 03 00 02 f2 ff 00 00 00 01 f2 ff 00 00 00 04 08 00 45 00 00 1c 00 01 00 00 40 01 64 7a 0a 01 01 01 0a 01 01 64 08 00 f7 ff 00 00 00 00 > /dev/null])

OVS_WAIT_UNTIL([grep -q 'Invalid Geneve tunnel metadata' ovs-vswitchd.log])

OVS_TRAFFIC_VSWITCHD_STOP(["/Invalid Geneve tunnel metadata on bridge br0 while processing icmp,in_port=1,vlan_tci=0x0000,dl_src=f2:ff:00:00:00:04,dl_dst=f2:ff:00:00:00:01,nw_src=10.1.1.1,nw_dst=10.1.1.100,nw_tos=0,nw_ecn=0,nw_ttl=64,nw_frag=no,icmp_type=8,icmp_code=0/d
/Unable to parse geneve options/d"])
AT_CLEANUP

0 comments on commit 935cd1d

Please sign in to comment.