Skip to content

Commit

Permalink
netdev-offload-tc: Fix ignore of dl type mask when installing flows.
Browse files Browse the repository at this point in the history
When TC parses the flow to install, it assumes that the datalink type
mask is set. However, this may not always be the case, for example,
when multiple VLANs exist but only one is enabled (vlan-limit).

This patch will only process the dl_type if the mask is set. It also
includes a unit test to verify that the TC rules are offloaded in this
case.

Fixes: 1be33d5 ("netdev-tc-offloads: Don't offload header modification on ip fragments.")
Reported-at: https://issues.redhat.com/browse/FDP-1114
Acked-by: Aaron Conole <aconole@redhat.com>
Acked-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
  • Loading branch information
chaudron committed Jan 29, 2025
1 parent 2b7de35 commit acc1e93
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
11 changes: 7 additions & 4 deletions lib/netdev-offload-tc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2253,6 +2253,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
const struct flow_tnl *tnl = &match->flow.tunnel;
struct flow_tnl *tnl_mask = &mask->tunnel;
struct dpif_flow_stats adjust_stats;
bool exact_match_on_dl_type;
bool recirc_act = false;
uint32_t block_id = 0;
struct tcf_id id;
Expand All @@ -2270,6 +2271,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,

memset(&flower, 0, sizeof flower);

exact_match_on_dl_type = mask->dl_type == htons(0xffff);
chain = key->recirc_id;
mask->recirc_id = 0;

Expand Down Expand Up @@ -2434,7 +2436,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
mask->dl_type = 0;
mask->in_port.odp_port = 0;

if (key->dl_type == htons(ETH_P_ARP)) {
if (exact_match_on_dl_type && key->dl_type == htons(ETH_P_ARP)) {
flower.key.arp.spa = key->nw_src;
flower.key.arp.tpa = key->nw_dst;
flower.key.arp.sha = key->arp_sha;
Expand All @@ -2453,7 +2455,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
memset(&mask->arp_tha, 0, sizeof mask->arp_tha);
}

if (is_ip_any(key) && !is_ipv6_fragment_and_masked(key, mask)) {
if (exact_match_on_dl_type && is_ip_any(key)
&& !is_ipv6_fragment_and_masked(key, mask)) {
flower.key.ip_proto = key->nw_proto;
flower.mask.ip_proto = mask->nw_proto;
mask->nw_proto = 0;
Expand Down Expand Up @@ -2483,9 +2486,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
} else {
/* This scenario should not occur. Currently, all installed IP DP
* flows perform a fully masked match on the fragmentation bits.
* However, since TC depends on this behavior, we return ENOTSUPP
* However, since TC depends on this behavior, we return EOPNOTSUPP
* for now in case this behavior changes in the future. */
return EOPNOTSUPP;
return EOPNOTSUPP;
}

if (key->nw_proto == IPPROTO_TCP) {
Expand Down
30 changes: 30 additions & 0 deletions tests/system-offloads-traffic.at
Original file line number Diff line number Diff line change
Expand Up @@ -958,3 +958,33 @@ OVS_WAIT_UNTIL([grep -q "Datapath does not support explicit drop action" ovs-vsw

OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP

AT_SETUP([offloads - 802.1ad should be offloaded])
OVS_TRAFFIC_VSWITCHD_START(
[], [], [-- set Open_vSwitch . other_config:hw-offload=true])
OVS_CHECK_8021AD()

ADD_NAMESPACES(at_ns0, at_ns1)

ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")

ADD_SVLAN(p0, at_ns0, 4094, "10.255.2.1/24")
ADD_SVLAN(p1, at_ns1, 4094, "10.255.2.2/24")

ADD_CVLAN(p0.4094, at_ns0, 100, "10.2.2.1/24")
ADD_CVLAN(p1.4094, at_ns1, 100, "10.2.2.2/24")

AT_CHECK([ovs-ofctl add-flow br0 "priority=1 action=normal"])

OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.2.2.2])

AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | DUMP_CLEAN_SORTED], [0], [dnl
in_port(2),eth(macs),eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x0800)), packets:0, bytes:0, used:0.001s, actions:output
in_port(3),eth(macs),eth_type(0x88a8),vlan(vid=4094,pcp=0),encap(eth_type(0x0800)), packets:0, bytes:0, used:0.001s, actions:output
])

AT_CHECK([ovs-appctl dpctl/dump-flows type=ovs | grep "eth_type(0x0800)" | DUMP_CLEAN_SORTED], [0], [])

OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP

0 comments on commit acc1e93

Please sign in to comment.