Re: [ovs-dev] [PATCH v9 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.
On 4 Mar 2024, at 16:46, Aaron Conole wrote: > Eelco Chaudron writes: > >> On 20 Feb 2024, at 22:47, Aaron Conole wrote: >> >>> From: Kevin Sprague >>> >>> During normal operations, it is useful to understand when a particular flow >>> gets removed from the system. This can be useful when debugging performance >>> issues tied to ofproto flow changes, trying to determine deployed traffic >>> patterns, or while debugging dynamic systems where ports come and go. >>> >>> Prior to this change, there was a lack of visibility around flow expiration. >>> The existing debugging infrastructure could tell us when a flow was added to >>> the datapath, but not when it was removed or why. >>> >>> This change introduces a USDT probe at the point where the revalidator >>> determines that the flow should be removed. Additionally, we track the >>> reason for the flow eviction and provide that information as well. With >>> this change, we can track the complete flow lifecycle for the netlink >>> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and >>> the revalidator USDT, letting us watch as flows are added and removed from >>> the kernel datapath. >>> >>> This change only enables this information via USDT probe, so it won't be >>> possible to access this information any other way (see: >>> Documentation/topics/usdt-probes.rst). >>> >>> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py) >>> which serves as a demonstration of how the new USDT probe might be used >>> going forward. >>> >>> Acked-by: Han Zhou >>> Signed-off-by: Kevin Sprague >>> Co-authored-by: Aaron Conole >>> Signed-off-by: Aaron Conole >> >> Thanks for doing the v9, some small comments remain below. >> >> Cheers, >> >> Eelco >> >>> --- >>> v8 -> v9: Reorganized the flow delete reasons enum >>> Updated flow_reval_monitor to use pahole to extract fields >>> Added the purge reason with a proper USDT point >>> Updated documentation >>> Dropped all the outstanding ACKs >>> >>> Documentation/topics/usdt-probes.rst | 43 + >>> ofproto/ofproto-dpif-upcall.c| 48 +- >>> utilities/automake.mk| 3 + >>> utilities/usdt-scripts/flow_reval_monitor.py | 997 +++ >>> 4 files changed, 1085 insertions(+), 6 deletions(-) >>> create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py >>> >>> diff --git a/Documentation/topics/usdt-probes.rst >>> b/Documentation/topics/usdt-probes.rst >>> index e527f43bab..015614a6b8 100644 >>> --- a/Documentation/topics/usdt-probes.rst >>> +++ b/Documentation/topics/usdt-probes.rst >>> @@ -214,8 +214,10 @@ Available probes in ``ovs_vswitchd``: >>> - dpif_recv:recv_upcall >>> - main:poll_block >>> - main:run_start >>> +- revalidate:flow_result >>> - revalidate_ukey\_\_:entry >>> - revalidate_ukey\_\_:exit >>> +- revalidator_sweep\_\_:flow_result >>> - udpif_revalidator:start_dump >>> - udpif_revalidator:sweep_done >>> >>> @@ -443,6 +445,47 @@ sweep phase was completed. >>> - ``utilities/usdt-scripts/reval_monitor.py`` >>> >>> >>> +probe revalidate:flow_result >>> + >>> + >>> +**Description**: >>> +This probe is triggered when the revalidator has executed on a particular >>> +flow key to make a determination whether to evict a flow, and the cause >>> +for eviction. The revalidator runs periodically, and this probe will only >>> +be triggered when a flow is flagged for revalidation. >>> + >>> +**Arguments**: >>> + >>> +- *arg0*: ``(enum reval_result) result`` >>> +- *arg1*: ``(enum flow_del_reason) reason`` >> >> nit: variable name changed, so should be del_reason. > > Good catch, I'll update. > >>> +- *arg2*: ``(struct udpif *) udpif`` >>> +- *arg3*: ``(struct udpif_key *) ukey`` >>> + >> >> I think you missed my previous comment on re-ordering the arguments to >> be more inline with existing probes, i.e.: >> >> +OVS_USDT_PROBE(revalidator_sweep__, flow_result, udpif, >> ukey, >> + result, del_reason); > > Guess so. I'll fix it. > >>> +**Script references**: >>> + >>> +- ``utilities/usdt-scripts/flow_reval_monitor.py`` >>> + >>> + >>> +probe revalidator_sweep\_\_:flow_result >>> +~~~ >>> + >>> +**Description**: >>> +This probe is placed in the path of the revalidator sweep, and is executed >>> +under the condition that a flow entry is in an unexpected state, or the >>> +flows were asked to be purged due to a user action. >>> + >>> +**Arguments**: >>> + >>> +- *arg0*: ``(enum reval_result) result`` >>> +- *arg1*: ``(enum flow_del_reason) reason`` >> >> nit: variable name changed, so should be del_reason. > > Okay. > >>> +- *arg2*: ``(struct udpif *) udpif`` >>> +- *arg3*: ``(struct udpif_key *) ukey`` >> >> See comments above on argument ordering. >> >>> + >>> +**Script references**: >>> + >>> +- ``utilities/usdt-scripts/flow_reval_monitor.py`` >>> + >>> +
Re: [ovs-dev] [PATCH v9 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.
Eelco Chaudron writes: > On 20 Feb 2024, at 22:47, Aaron Conole wrote: > >> From: Kevin Sprague >> >> During normal operations, it is useful to understand when a particular flow >> gets removed from the system. This can be useful when debugging performance >> issues tied to ofproto flow changes, trying to determine deployed traffic >> patterns, or while debugging dynamic systems where ports come and go. >> >> Prior to this change, there was a lack of visibility around flow expiration. >> The existing debugging infrastructure could tell us when a flow was added to >> the datapath, but not when it was removed or why. >> >> This change introduces a USDT probe at the point where the revalidator >> determines that the flow should be removed. Additionally, we track the >> reason for the flow eviction and provide that information as well. With >> this change, we can track the complete flow lifecycle for the netlink >> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and >> the revalidator USDT, letting us watch as flows are added and removed from >> the kernel datapath. >> >> This change only enables this information via USDT probe, so it won't be >> possible to access this information any other way (see: >> Documentation/topics/usdt-probes.rst). >> >> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py) >> which serves as a demonstration of how the new USDT probe might be used >> going forward. >> >> Acked-by: Han Zhou >> Signed-off-by: Kevin Sprague >> Co-authored-by: Aaron Conole >> Signed-off-by: Aaron Conole > > Thanks for doing the v9, some small comments remain below. > > Cheers, > > Eelco > >> --- >> v8 -> v9: Reorganized the flow delete reasons enum >> Updated flow_reval_monitor to use pahole to extract fields >> Added the purge reason with a proper USDT point >> Updated documentation >> Dropped all the outstanding ACKs >> >> Documentation/topics/usdt-probes.rst | 43 + >> ofproto/ofproto-dpif-upcall.c| 48 +- >> utilities/automake.mk| 3 + >> utilities/usdt-scripts/flow_reval_monitor.py | 997 +++ >> 4 files changed, 1085 insertions(+), 6 deletions(-) >> create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py >> >> diff --git a/Documentation/topics/usdt-probes.rst >> b/Documentation/topics/usdt-probes.rst >> index e527f43bab..015614a6b8 100644 >> --- a/Documentation/topics/usdt-probes.rst >> +++ b/Documentation/topics/usdt-probes.rst >> @@ -214,8 +214,10 @@ Available probes in ``ovs_vswitchd``: >> - dpif_recv:recv_upcall >> - main:poll_block >> - main:run_start >> +- revalidate:flow_result >> - revalidate_ukey\_\_:entry >> - revalidate_ukey\_\_:exit >> +- revalidator_sweep\_\_:flow_result >> - udpif_revalidator:start_dump >> - udpif_revalidator:sweep_done >> >> @@ -443,6 +445,47 @@ sweep phase was completed. >> - ``utilities/usdt-scripts/reval_monitor.py`` >> >> >> +probe revalidate:flow_result >> + >> + >> +**Description**: >> +This probe is triggered when the revalidator has executed on a particular >> +flow key to make a determination whether to evict a flow, and the cause >> +for eviction. The revalidator runs periodically, and this probe will only >> +be triggered when a flow is flagged for revalidation. >> + >> +**Arguments**: >> + >> +- *arg0*: ``(enum reval_result) result`` >> +- *arg1*: ``(enum flow_del_reason) reason`` > > nit: variable name changed, so should be del_reason. Good catch, I'll update. >> +- *arg2*: ``(struct udpif *) udpif`` >> +- *arg3*: ``(struct udpif_key *) ukey`` >> + > > I think you missed my previous comment on re-ordering the arguments to > be more inline with existing probes, i.e.: > > +OVS_USDT_PROBE(revalidator_sweep__, flow_result, udpif, ukey, > + result, del_reason); Guess so. I'll fix it. >> +**Script references**: >> + >> +- ``utilities/usdt-scripts/flow_reval_monitor.py`` >> + >> + >> +probe revalidator_sweep\_\_:flow_result >> +~~~ >> + >> +**Description**: >> +This probe is placed in the path of the revalidator sweep, and is executed >> +under the condition that a flow entry is in an unexpected state, or the >> +flows were asked to be purged due to a user action. >> + >> +**Arguments**: >> + >> +- *arg0*: ``(enum reval_result) result`` >> +- *arg1*: ``(enum flow_del_reason) reason`` > > nit: variable name changed, so should be del_reason. Okay. >> +- *arg2*: ``(struct udpif *) udpif`` >> +- *arg3*: ``(struct udpif_key *) ukey`` > > See comments above on argument ordering. > >> + >> +**Script references**: >> + >> +- ``utilities/usdt-scripts/flow_reval_monitor.py`` >> + >> + >> Adding your own probes >> -- >> >> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c >> index b5cbeed878..fbc7858690 100644 >> ---
Re: [ovs-dev] [PATCH v9 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.
On 20 Feb 2024, at 22:47, Aaron Conole wrote:
> From: Kevin Sprague
>
> During normal operations, it is useful to understand when a particular flow
> gets removed from the system. This can be useful when debugging performance
> issues tied to ofproto flow changes, trying to determine deployed traffic
> patterns, or while debugging dynamic systems where ports come and go.
>
> Prior to this change, there was a lack of visibility around flow expiration.
> The existing debugging infrastructure could tell us when a flow was added to
> the datapath, but not when it was removed or why.
>
> This change introduces a USDT probe at the point where the revalidator
> determines that the flow should be removed. Additionally, we track the
> reason for the flow eviction and provide that information as well. With
> this change, we can track the complete flow lifecycle for the netlink
> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and
> the revalidator USDT, letting us watch as flows are added and removed from
> the kernel datapath.
>
> This change only enables this information via USDT probe, so it won't be
> possible to access this information any other way (see:
> Documentation/topics/usdt-probes.rst).
>
> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
> which serves as a demonstration of how the new USDT probe might be used
> going forward.
>
> Acked-by: Han Zhou
> Signed-off-by: Kevin Sprague
> Co-authored-by: Aaron Conole
> Signed-off-by: Aaron Conole
Thanks for doing the v9, some small comments remain below.
Cheers,
Eelco
> ---
> v8 -> v9: Reorganized the flow delete reasons enum
> Updated flow_reval_monitor to use pahole to extract fields
> Added the purge reason with a proper USDT point
> Updated documentation
> Dropped all the outstanding ACKs
>
> Documentation/topics/usdt-probes.rst | 43 +
> ofproto/ofproto-dpif-upcall.c| 48 +-
> utilities/automake.mk| 3 +
> utilities/usdt-scripts/flow_reval_monitor.py | 997 +++
> 4 files changed, 1085 insertions(+), 6 deletions(-)
> create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>
> diff --git a/Documentation/topics/usdt-probes.rst
> b/Documentation/topics/usdt-probes.rst
> index e527f43bab..015614a6b8 100644
> --- a/Documentation/topics/usdt-probes.rst
> +++ b/Documentation/topics/usdt-probes.rst
> @@ -214,8 +214,10 @@ Available probes in ``ovs_vswitchd``:
> - dpif_recv:recv_upcall
> - main:poll_block
> - main:run_start
> +- revalidate:flow_result
> - revalidate_ukey\_\_:entry
> - revalidate_ukey\_\_:exit
> +- revalidator_sweep\_\_:flow_result
> - udpif_revalidator:start_dump
> - udpif_revalidator:sweep_done
>
> @@ -443,6 +445,47 @@ sweep phase was completed.
> - ``utilities/usdt-scripts/reval_monitor.py``
>
>
> +probe revalidate:flow_result
> +
> +
> +**Description**:
> +This probe is triggered when the revalidator has executed on a particular
> +flow key to make a determination whether to evict a flow, and the cause
> +for eviction. The revalidator runs periodically, and this probe will only
> +be triggered when a flow is flagged for revalidation.
> +
> +**Arguments**:
> +
> +- *arg0*: ``(enum reval_result) result``
> +- *arg1*: ``(enum flow_del_reason) reason``
nit: variable name changed, so should be del_reason.
> +- *arg2*: ``(struct udpif *) udpif``
> +- *arg3*: ``(struct udpif_key *) ukey``
> +
I think you missed my previous comment on re-ordering the arguments to be more
inline with existing probes, i.e.:
+OVS_USDT_PROBE(revalidator_sweep__, flow_result, udpif, ukey,
+ result, del_reason);
> +**Script references**:
> +
> +- ``utilities/usdt-scripts/flow_reval_monitor.py``
> +
> +
> +probe revalidator_sweep\_\_:flow_result
> +~~~
> +
> +**Description**:
> +This probe is placed in the path of the revalidator sweep, and is executed
> +under the condition that a flow entry is in an unexpected state, or the
> +flows were asked to be purged due to a user action.
> +
> +**Arguments**:
> +
> +- *arg0*: ``(enum reval_result) result``
> +- *arg1*: ``(enum flow_del_reason) reason``
nit: variable name changed, so should be del_reason.
> +- *arg2*: ``(struct udpif *) udpif``
> +- *arg3*: ``(struct udpif_key *) ukey``
See comments above on argument ordering.
> +
> +**Script references**:
> +
> +- ``utilities/usdt-scripts/flow_reval_monitor.py``
> +
> +
> Adding your own probes
> --
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index b5cbeed878..fbc7858690 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -269,6 +269,20 @@ enum ukey_state {
> };
> #define N_UKEY_STATES (UKEY_DELETED + 1)
>
> +enum flow_del_reason {
> +FDR_NONE = 0, /* No deletion reason
[ovs-dev] [PATCH v9 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.
From: Kevin Sprague
During normal operations, it is useful to understand when a particular flow
gets removed from the system. This can be useful when debugging performance
issues tied to ofproto flow changes, trying to determine deployed traffic
patterns, or while debugging dynamic systems where ports come and go.
Prior to this change, there was a lack of visibility around flow expiration.
The existing debugging infrastructure could tell us when a flow was added to
the datapath, but not when it was removed or why.
This change introduces a USDT probe at the point where the revalidator
determines that the flow should be removed. Additionally, we track the
reason for the flow eviction and provide that information as well. With
this change, we can track the complete flow lifecycle for the netlink
datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and
the revalidator USDT, letting us watch as flows are added and removed from
the kernel datapath.
This change only enables this information via USDT probe, so it won't be
possible to access this information any other way (see:
Documentation/topics/usdt-probes.rst).
Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
which serves as a demonstration of how the new USDT probe might be used
going forward.
Acked-by: Han Zhou
Signed-off-by: Kevin Sprague
Co-authored-by: Aaron Conole
Signed-off-by: Aaron Conole
---
v8 -> v9: Reorganized the flow delete reasons enum
Updated flow_reval_monitor to use pahole to extract fields
Added the purge reason with a proper USDT point
Updated documentation
Dropped all the outstanding ACKs
Documentation/topics/usdt-probes.rst | 43 +
ofproto/ofproto-dpif-upcall.c| 48 +-
utilities/automake.mk| 3 +
utilities/usdt-scripts/flow_reval_monitor.py | 997 +++
4 files changed, 1085 insertions(+), 6 deletions(-)
create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
diff --git a/Documentation/topics/usdt-probes.rst
b/Documentation/topics/usdt-probes.rst
index e527f43bab..015614a6b8 100644
--- a/Documentation/topics/usdt-probes.rst
+++ b/Documentation/topics/usdt-probes.rst
@@ -214,8 +214,10 @@ Available probes in ``ovs_vswitchd``:
- dpif_recv:recv_upcall
- main:poll_block
- main:run_start
+- revalidate:flow_result
- revalidate_ukey\_\_:entry
- revalidate_ukey\_\_:exit
+- revalidator_sweep\_\_:flow_result
- udpif_revalidator:start_dump
- udpif_revalidator:sweep_done
@@ -443,6 +445,47 @@ sweep phase was completed.
- ``utilities/usdt-scripts/reval_monitor.py``
+probe revalidate:flow_result
+
+
+**Description**:
+This probe is triggered when the revalidator has executed on a particular
+flow key to make a determination whether to evict a flow, and the cause
+for eviction. The revalidator runs periodically, and this probe will only
+be triggered when a flow is flagged for revalidation.
+
+**Arguments**:
+
+- *arg0*: ``(enum reval_result) result``
+- *arg1*: ``(enum flow_del_reason) reason``
+- *arg2*: ``(struct udpif *) udpif``
+- *arg3*: ``(struct udpif_key *) ukey``
+
+**Script references**:
+
+- ``utilities/usdt-scripts/flow_reval_monitor.py``
+
+
+probe revalidator_sweep\_\_:flow_result
+~~~
+
+**Description**:
+This probe is placed in the path of the revalidator sweep, and is executed
+under the condition that a flow entry is in an unexpected state, or the
+flows were asked to be purged due to a user action.
+
+**Arguments**:
+
+- *arg0*: ``(enum reval_result) result``
+- *arg1*: ``(enum flow_del_reason) reason``
+- *arg2*: ``(struct udpif *) udpif``
+- *arg3*: ``(struct udpif_key *) ukey``
+
+**Script references**:
+
+- ``utilities/usdt-scripts/flow_reval_monitor.py``
+
+
Adding your own probes
--
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index b5cbeed878..fbc7858690 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -269,6 +269,20 @@ enum ukey_state {
};
#define N_UKEY_STATES (UKEY_DELETED + 1)
+enum flow_del_reason {
+FDR_NONE = 0, /* No deletion reason for the flow. */
+FDR_AVOID_CACHING, /* Flow deleted to avoid caching. */
+FDR_BAD_ODP_FIT,/* The flow had a bad ODP flow fit. */
+FDR_FLOW_IDLE, /* The flow went unused and was deleted. */
+FDR_FLOW_LIMIT, /* All flows being killed. */
+FDR_FLOW_WILDCARDED,/* The flow needed a narrower wildcard mask. */
+FDR_NO_OFPROTO, /* The flow didn't have an associated ofproto. */
+FDR_PURGE, /* User action caused flows to be killed. */
+FDR_TOO_EXPENSIVE, /* The flow was too expensive to revalidate. */
+FDR_UPDATE_FAIL,/* Flow state transition was unexpected. */
+FDR_XLATION_ERROR, /* There was an error translating the flow. */
+};
+
/*
