Re: [ovs-dev] [PATCH v3] flow: Read recirc depth and flow api enabled once per batch in miniflow_extract
On 6/28/21 5:19 PM, Balazs Nemeth wrote: > 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. > > Signed-off-by: Balazs Nemeth > Acked-by: Gaetan Rivet > Acked-by: Paolo Valerio > --- Thanks! I changed the title of the patch a little bit and applied to master. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] flow: Read recirc depth and flow api enabled once per batch in miniflow_extract
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 >> Acked-by: Gaetan Rivet >> Acked-by: Paolo Valerio >> --- >> 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 >> ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] flow: Read recirc depth and flow api enabled once per batch in miniflow_extract
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. Other than that, LGTM. Signed-off-by: Balazs Nemeth Acked-by: Gaetan Rivet Acked-by: Paolo Valerio --- 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 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev