[dpdk-dev,v3] net/tap: fix isolation mode toggling
Checks
Commit Message
Running testpmd command "flow isolae <port> 0" (i.e. disabling flow
isolation) followed by command "flow isolate <port> 1" (i.e. enabling
flow isolation) may result in a TAP error:
PMD: Kernel refused TC filter rule creation (17): File exists
Root cause analysis: when disabling flow isolation we keep the local
rule to redirect packets on TX (TAP_REMOTE_TX index) while we add it
again when enabling flow isolation. As a result this rule is added
two times in a row which results in "File exists" error.
The fix is to identify the "File exists" error and silently ignore it.
Another issue occurs when enabling isolation mode several times in a
row in which case the same tc rules are added consecutively and
rte_flow structs are added to a linked list before removing the
previous rte_flow structs.
The fix is to act upon isolation mode command only when there is a
change from "0" to "1" (or vice versa).
Fixes: f503d2694825 ("net/tap: support flow API isolated mode")
Cc: stable@dpdk.org
Reviewed-by: Raslan Darawsheh <rasland@mellanox.com>
Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
v1:
initial release
v2:
Please ignore (due to typo errors)
v3:
1. Updates based on Keith Wiles review
2. Do not empty list of implicit TC rules (role back to legacy implementation)
to ensure TC implicit rules cleanup during implicit rules flushing
drivers/net/tap/tap_flow.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
Comments
> On May 14, 2018, at 5:26 PM, Ophir Munk <ophirmu@mellanox.com> wrote:
>
> Running testpmd command "flow isolae <port> 0" (i.e. disabling flow
> isolation) followed by command "flow isolate <port> 1" (i.e. enabling
> flow isolation) may result in a TAP error:
> PMD: Kernel refused TC filter rule creation (17): File exists
>
> Root cause analysis: when disabling flow isolation we keep the local
> rule to redirect packets on TX (TAP_REMOTE_TX index) while we add it
> again when enabling flow isolation. As a result this rule is added
> two times in a row which results in "File exists" error.
> The fix is to identify the "File exists" error and silently ignore it.
>
> Another issue occurs when enabling isolation mode several times in a
> row in which case the same tc rules are added consecutively and
> rte_flow structs are added to a linked list before removing the
> previous rte_flow structs.
> The fix is to act upon isolation mode command only when there is a
> change from "0" to "1" (or vice versa).
>
> Fixes: f503d2694825 ("net/tap: support flow API isolated mode")
> Cc: stable@dpdk.org
>
> Reviewed-by: Raslan Darawsheh <rasland@mellanox.com>
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> —
Acked by: Keith Wiles<keith.wile@intel.com>
Regards,
Keith
On 5/14/2018 11:46 PM, Wiles, Keith wrote:
>
>
>> On May 14, 2018, at 5:26 PM, Ophir Munk <ophirmu@mellanox.com> wrote:
>>
>> Running testpmd command "flow isolae <port> 0" (i.e. disabling flow
>> isolation) followed by command "flow isolate <port> 1" (i.e. enabling
>> flow isolation) may result in a TAP error:
>> PMD: Kernel refused TC filter rule creation (17): File exists
>>
>> Root cause analysis: when disabling flow isolation we keep the local
>> rule to redirect packets on TX (TAP_REMOTE_TX index) while we add it
>> again when enabling flow isolation. As a result this rule is added
>> two times in a row which results in "File exists" error.
>> The fix is to identify the "File exists" error and silently ignore it.
>>
>> Another issue occurs when enabling isolation mode several times in a
>> row in which case the same tc rules are added consecutively and
>> rte_flow structs are added to a linked list before removing the
>> previous rte_flow structs.
>> The fix is to act upon isolation mode command only when there is a
>> change from "0" to "1" (or vice versa).
>>
>> Fixes: f503d2694825 ("net/tap: support flow API isolated mode")
>> Cc: stable@dpdk.org
>>
>> Reviewed-by: Raslan Darawsheh <rasland@mellanox.com>
>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
>> —
>
> Acked by: Keith Wiles<keith.wile@intel.com>
Applied to dpdk-next-net/master, thanks.
@@ -1568,10 +1568,14 @@ tap_flow_isolate(struct rte_eth_dev *dev,
{
struct pmd_internals *pmd = dev->data->dev_private;
+ /* normalize 'set' variable to contain 0 or 1 values */
if (set)
- pmd->flow_isolate = 1;
- else
- pmd->flow_isolate = 0;
+ set = 1;
+ /* if already in the right isolation mode - nothing to do */
+ if ((set ^ pmd->flow_isolate) == 0)
+ return 0;
+ /* mark the isolation mode for tap_flow_implicit_create() */
+ pmd->flow_isolate = set;
/*
* If netdevice is there, setup appropriate flow rules immediately.
* Otherwise it will be set when bringing up the netdevice (tun_alloc).
@@ -1579,20 +1583,20 @@ tap_flow_isolate(struct rte_eth_dev *dev,
if (!pmd->rxq[0].fd)
return 0;
if (set) {
- struct rte_flow *flow;
+ struct rte_flow *remote_flow;
while (1) {
- flow = LIST_FIRST(&pmd->implicit_flows);
- if (!flow)
+ remote_flow = LIST_FIRST(&pmd->implicit_flows);
+ if (!remote_flow)
break;
/*
* Remove all implicit rules on the remote.
* Keep the local rule to redirect packets on TX.
* Keep also the last implicit local rule: ISOLATE.
*/
- if (flow->msg.t.tcm_ifindex == pmd->if_index)
+ if (remote_flow->msg.t.tcm_ifindex == pmd->if_index)
break;
- if (tap_flow_destroy_pmd(pmd, flow, NULL) < 0)
+ if (tap_flow_destroy_pmd(pmd, remote_flow, NULL) < 0)
goto error;
}
/* Switch the TC rule according to pmd->flow_isolate */
@@ -1739,8 +1743,8 @@ int tap_flow_implicit_create(struct pmd_internals *pmd,
}
err = tap_nl_recv_ack(pmd->nlsk_fd);
if (err < 0) {
- /* Silently ignore re-entering remote promiscuous rule */
- if (errno == EEXIST && idx == TAP_REMOTE_PROMISC)
+ /* Silently ignore re-entering existing rule */
+ if (errno == EEXIST)
goto success;
TAP_LOG(ERR,
"Kernel refused TC filter rule creation (%d): %s",