Eelco Chaudron <[email protected]> writes:

> On 16 May 2024, at 14:46, Ilya Maximets wrote:
>
>> On 5/14/24 15:15, Eelco Chaudron wrote:
>>> The flow_reval_monitor.py script incorrectly reported the reasons for
>>> FDR_PURGE and FDR_TOO_EXPENSIVE, as their descriptions were swapped.
>>> This patch rectifies the order using a dictionary to avoid similar
>>> problems in the future.
>>>
>>> In addition this patch also syncs the delete reason output of the
>>> script, with the comments in the code.
>>>
>>> Fixes: 86b9e653ef22 ("revalidator: Add a USDT probe during flow
>>> deletion with purge reason.")
>>> Signed-off-by: Eelco Chaudron <[email protected]>
>>>
>>> ---
>>> v3: - Renamed ofproto dpif to bridge in delete reasons.
>>>     - Added comment pointing back to delete reasons in .c.
>>> v2: - Converted the list of strings to dictionary.
>>>     - Added comment to code to keep code and script in sync.
>>>     - Unified flow_delete reason comments and script output.
>>> ---
>>
>> I didn't test, but the code looks fine to me.  Thanks!
>>
>> Acked-by: Ilya Maximets <[email protected]>
>
> Thanks Aaron and Ilya, applied upstream.
>
>
>> BTW, is there a reason why we can't just report a static string
>> from the USDT probe instead of an integer?  If we did that we
>> would not need to have this mapping in the script at all.
>
> Did not want to copy them into the ring buffer to save space and
> CPU. I guess we could copy in the address, but not sure how complex
> the Python code would become.

Let's not try to ship "ephemeral" (since ASLR will move things)
addresses around.  That feels like a recipe for disaster.

I guess one thing we could do is instead publish the strings and
attributes in an appctl command and then the python program could read
that to populate the fields.

  $ ovs-appctl usdt-details/flow-del-reasons

That might be a way to shift the burden from maintaining this details
twice, to just in a single place.

> //Eelco

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to