[ovs-dev] [PATCH v3] flow: Read recirc depth and flow api enabled once per batch in miniflow_extract

[Ilya Maximets]

On 6/29/21 7:35 AM, Eli Britstein wrote:
> 
> On 6/28/2021 6:19 PM, Balazs Nemeth wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> The call to recirc_depth_get involves accessing a TLS value. So read
>> that once, and store it on the stack for re-use while processing the
>> batch. The same goes for reading netdev_is_flow_api_enabled(), a
>> non-inlined function.
> 
> A small further suggestion:
> 
> The config other_config:hw-offload is read only once upon init, so for netdev_is_flow_api_enabled(), we can have a global static variable to be set only at init (dpif_netdev_set_config) by the non-inline function.
> 
> Then, we can replace all other calls with this variable.

While it's required by documentation to restart OVS after
changing the 'hw-offload' knob, technically, this could
be done in runtime.  So, I'm not sure about this solution.

> 
> Other than that, LGTM.
> 
>>
>> Signed-off-by: Balazs Nemeth <bnemeth at redhat.com>
>> Acked-by: Gaetan Rivet <grive at u256.net>
>> Acked-by: Paolo Valerio <pvalerio at redhat.com>
>> ---
>>   lib/dpif-netdev.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index c5ab35d2a..bf2112ead 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -7165,6 +7165,8 @@ dfc_processing(struct dp_netdev_pmd_thread *pmd,
>>       struct dp_packet *packet;
>>       const size_t cnt = dp_packet_batch_size(packets_);
>>       uint32_t cur_min = pmd->ctx.emc_insert_min;
>> +    const uint32_t recirc_depth = *recirc_depth_get();
>> +    const bool netdev_flow_api = netdev_is_flow_api_enabled();
>>       int i;
>>       uint16_t tcp_flags;
>>       bool smc_enable_db;
>> @@ -7196,7 +7198,7 @@ dfc_processing(struct dp_netdev_pmd_thread *pmd,
>>               pkt_metadata_init(&packet->md, port_no);
>>           }
>>
>> -        if (netdev_is_flow_api_enabled() && *recirc_depth_get() == 0) {
>> +        if (netdev_flow_api && recirc_depth == 0) {
>>               if (OVS_UNLIKELY(dp_netdev_hw_flow(pmd, port_no, packet, &flow))) {
>>                   /* Packet restoration failed and it was dropped, do not
>>                    * continue processing.
>> -- 
>> 2.31.1
>>