mirror of
https://github.com/openvswitch/ovs
synced 2025-08-31 14:25:26 +00:00
dpif-netdev: Fix dpif_netdev_flow_put.
OVS allows overlapping megaflows, as long as the actions of these
megaflows are equal. However, the current implementation of action
modification relies on flow_lookup instead of UFID, this could result
in looking up a wrong megaflow and make the ukeys and megaflows
inconsistent.
Just like the test case in the patch, at first we have a rule with the
prefix:
10.1.2.0/24
And we will get a megaflow with prefixes 10.1.2.2/24 when a packet with
IP 10.1.2.2 is received.
Then suppose we change the rule into 10.1.0.0/16. OVS prefers to keep
the 10.1.2.2/24 megaflow and just changes its action instead of
extending the prefix into 10.1.2.2/16.
Then suppose we have a 10.1.0.2 packet, since it misses the megaflow,
this time, we will have an overlapping megaflow with the right prefix:
10.1.0.2/16
Now we have two megaflows:
10.1.2.2/24
10.1.0.2/16
Last, suppose we have changed the ruleset again. The revalidator this
time still decides to change the actions of both megaflows instead of
deleting them.
The dpif_netdev_flow_put will search the megaflow to modify with
unmasked keys, however it might lookup the wrong megaflow as the key
10.1.2.2 matches both 10.1.2.2/24 and 10.1.0.2/16!
This patch changes the megaflow lookup code in modification path into
relying the UFID to find the correct megaflow instead of key lookup.
Falling back to a classifier lookup in case where UFID was not provided
in order to support cases where UFID was not generated from the flow
data during the flow addition.
Fixes: beb75a40fd
("userspace: Switching of L3 packets in L2 pipeline")
Signed-off-by: Peng He <hepeng.0320@bytedance.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
@@ -4206,7 +4206,7 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
|
||||
const struct dpif_flow_put *put,
|
||||
struct dpif_flow_stats *stats)
|
||||
{
|
||||
struct dp_netdev_flow *netdev_flow;
|
||||
struct dp_netdev_flow *netdev_flow = NULL;
|
||||
int error = 0;
|
||||
|
||||
if (stats) {
|
||||
@@ -4214,16 +4214,35 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
|
||||
}
|
||||
|
||||
ovs_mutex_lock(&pmd->flow_mutex);
|
||||
netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
|
||||
if (!netdev_flow) {
|
||||
if (put->flags & DPIF_FP_CREATE) {
|
||||
dp_netdev_flow_add(pmd, match, ufid, put->actions,
|
||||
put->actions_len, ODPP_NONE);
|
||||
} else {
|
||||
error = ENOENT;
|
||||
}
|
||||
if (put->ufid) {
|
||||
netdev_flow = dp_netdev_pmd_find_flow(pmd, put->ufid,
|
||||
put->key, put->key_len);
|
||||
} else {
|
||||
if (put->flags & DPIF_FP_MODIFY) {
|
||||
/* Use key instead of the locally generated ufid
|
||||
* to search netdev_flow. */
|
||||
netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
|
||||
}
|
||||
|
||||
if (put->flags & DPIF_FP_CREATE) {
|
||||
if (!netdev_flow) {
|
||||
dp_netdev_flow_add(pmd, match, ufid,
|
||||
put->actions, put->actions_len, ODPP_NONE);
|
||||
} else {
|
||||
error = EEXIST;
|
||||
}
|
||||
goto exit;
|
||||
}
|
||||
|
||||
if (put->flags & DPIF_FP_MODIFY) {
|
||||
if (!netdev_flow) {
|
||||
error = ENOENT;
|
||||
} else {
|
||||
if (!put->ufid && !flow_equal(&match->flow, &netdev_flow->flow)) {
|
||||
/* Overlapping flow. */
|
||||
error = EINVAL;
|
||||
goto exit;
|
||||
}
|
||||
|
||||
struct dp_netdev_actions *new_actions;
|
||||
struct dp_netdev_actions *old_actions;
|
||||
|
||||
@@ -4254,15 +4273,11 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
|
||||
* counter, and subtracting it before outputting the stats */
|
||||
error = EOPNOTSUPP;
|
||||
}
|
||||
|
||||
ovsrcu_postpone(dp_netdev_actions_free, old_actions);
|
||||
} else if (put->flags & DPIF_FP_CREATE) {
|
||||
error = EEXIST;
|
||||
} else {
|
||||
/* Overlapping flow. */
|
||||
error = EINVAL;
|
||||
}
|
||||
}
|
||||
|
||||
exit:
|
||||
ovs_mutex_unlock(&pmd->flow_mutex);
|
||||
return error;
|
||||
}
|
||||
|
Reference in New Issue
Block a user