Re: [ovs-dev] [PATCH V2 1/3] dpif-netdev: Do not flush PMD offloads on reload
On 7/25/21 8:40 AM, Eli Britstein wrote:
>
> On 7/23/2021 9:00 PM, Ilya Maximets wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 7/12/21 5:07 PM, Eli Britstein wrote:
>>> Before flushing offloads of a removed port was supported by [1], it was
>>> necessary to flush the 'marks'. In doing so, all offloads of the PMD are
>>> removed, include the ones that are not related to the removed port and
>>> that are not modified following this removal. As a result such flows are
>>> evicted from being offloaded, and won't resume offloading.
>>>
>>> As PMD offload flush is not necessary, avoid it.
>>>
>>> [1] 62d1c28e9ce0 ("dpif-netdev: Flush offload rules upon port deletion.")
>>>
>>> Signed-off-by: Eli Britstein
>>> Reviewed-by: Gaetan Rivet
>>> ---
>> Is my understanding here correct:
>> On a port deletion netdev_fow_flush() will remove flows from HW and
>> offloading layer. Later, ofproto will request to remove flows from
>> the datapath. flow marks will be freed, but actual netdev_flow_del()
>> will fail, because netdev-offload already removed these flows. But
>> we do not really care about this failure. Right?
>
> That is correct.
>
> It was also the same before above [1], but then in the race condition, the
> offload memory was leaked, and the offloads themselves were either not
> destroyed (e.g. leaked) or removed by the PMD, which was PMD dependent.
>
> [1] resolved the leaking, but didn't change the mentioned failure behavior.
>
> This commit doesn't prevents this failure either, but removes the code that
> destroys offloads of flows that should not be destroyed.
Make sense. Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/3] dpif-netdev: Do not flush PMD offloads on reload
On 7/23/2021 9:00 PM, Ilya Maximets wrote:
External email: Use caution opening links or attachments
On 7/12/21 5:07 PM, Eli Britstein wrote:
Before flushing offloads of a removed port was supported by [1], it was
necessary to flush the 'marks'. In doing so, all offloads of the PMD are
removed, include the ones that are not related to the removed port and
that are not modified following this removal. As a result such flows are
evicted from being offloaded, and won't resume offloading.
As PMD offload flush is not necessary, avoid it.
[1] 62d1c28e9ce0 ("dpif-netdev: Flush offload rules upon port deletion.")
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
Is my understanding here correct:
On a port deletion netdev_fow_flush() will remove flows from HW and
offloading layer. Later, ofproto will request to remove flows from
the datapath. flow marks will be freed, but actual netdev_flow_del()
will fail, because netdev-offload already removed these flows. But
we do not really care about this failure. Right?
That is correct.
It was also the same before above [1], but then in the race condition,
the offload memory was leaked, and the offloads themselves were either
not destroyed (e.g. leaked) or removed by the PMD, which was PMD dependent.
[1] resolved the leaking, but didn't change the mentioned failure behavior.
This commit doesn't prevents this failure either, but removes the code
that destroys offloads of flows that should not be destroyed.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/3] dpif-netdev: Do not flush PMD offloads on reload
On 7/12/21 5:07 PM, Eli Britstein wrote:
> Before flushing offloads of a removed port was supported by [1], it was
> necessary to flush the 'marks'. In doing so, all offloads of the PMD are
> removed, include the ones that are not related to the removed port and
> that are not modified following this removal. As a result such flows are
> evicted from being offloaded, and won't resume offloading.
>
> As PMD offload flush is not necessary, avoid it.
>
> [1] 62d1c28e9ce0 ("dpif-netdev: Flush offload rules upon port deletion.")
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
> ---
Is my understanding here correct:
On a port deletion netdev_fow_flush() will remove flows from HW and
offloading layer. Later, ofproto will request to remove flows from
the datapath. flow marks will be freed, but actual netdev_flow_del()
will fail, because netdev-offload already removed these flows. But
we do not really care about this failure. Right?
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/3] dpif-netdev: Do not flush PMD offloads on reload
On Mon, Jul 12, 2021 at 5:07 PM Eli Britstein wrote:
>
> Before flushing offloads of a removed port was supported by [1], it was
> necessary to flush the 'marks'. In doing so, all offloads of the PMD are
> removed, include the ones that are not related to the removed port and
> that are not modified following this removal. As a result such flows are
> evicted from being offloaded, and won't resume offloading.
>
> As PMD offload flush is not necessary, avoid it.
>
> [1] 62d1c28e9ce0 ("dpif-netdev: Flush offload rules upon port deletion.")
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
Reviewed-by: David Marchand
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V2 1/3] dpif-netdev: Do not flush PMD offloads on reload
Before flushing offloads of a removed port was supported by [1], it was
necessary to flush the 'marks'. In doing so, all offloads of the PMD are
removed, include the ones that are not related to the removed port and
that are not modified following this removal. As a result such flows are
evicted from being offloaded, and won't resume offloading.
As PMD offload flush is not necessary, avoid it.
[1] 62d1c28e9ce0 ("dpif-netdev: Flush offload rules upon port deletion.")
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
lib/dpif-netdev.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 610949f36..21b0e025d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2232,18 +2232,6 @@ mark_to_flow_disassociate(struct dp_netdev_pmd_thread
*pmd,
return ret;
}
-static void
-flow_mark_flush(struct dp_netdev_pmd_thread *pmd)
-{
-struct dp_netdev_flow *flow;
-
-CMAP_FOR_EACH (flow, mark_node, &flow_mark.mark_to_flow) {
-if (flow->pmd_id == pmd->core_id) {
-queue_netdev_flow_del(pmd, flow);
-}
-}
-}
-
static struct dp_netdev_flow *
mark_to_flow_find(const struct dp_netdev_pmd_thread *pmd,
const uint32_t mark)
@@ -4811,7 +4799,6 @@ reload_affected_pmds(struct dp_netdev *dp)
CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
if (pmd->need_reload) {
-flow_mark_flush(pmd);
dp_netdev_reload_pmd__(pmd);
}
}
--
2.28.0.2311.g225365fb51
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
