On Sun, 2021-04-11 at 22:27 +0200, Gaëtan Rivet wrote:
> On Fri, Apr 9, 2021, at 20:27, Gregory Rose wrote:
> > 
> > 
> > On 4/9/2021 3:09 AM, Balazs Nemeth wrote:
> > > The call to recirc_depth_get involves accessing a TLS value which
> > > is
> > > slower than accessing the stack.
> > > 
> > > Signed-off-by: Balazs Nemeth <[email protected]>
> > > ---
> > >   lib/dpif-netdev.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > > index 251788b04..41ac500a1 100644
> > > --- a/lib/dpif-netdev.c
> > > +++ b/lib/dpif-netdev.c
> > > @@ -7095,6 +7095,7 @@ 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 depth = *recirc_depth_get();
> > >       int i;
> > >       uint16_t tcp_flags;
> > >       bool smc_enable_db;
> > > @@ -7127,7 +7128,7 @@ dfc_processing(struct dp_netdev_pmd_thread
> > > *pmd,
> > >               pkt_metadata_init(&packet->md, port_no);
> > >           }
> > >   
> > > -        if ((*recirc_depth_get() == 0) &&
> > > +        if (depth == 0 &&
> > >               dp_packet_has_flow_mark(packet, &mark)) {
> > >               flow = mark_to_flow_find(pmd, mark);
> > >               if (OVS_LIKELY(flow)) {
> > > 
> > 
> > Makes sense.
> > 
> > Acked-by: Greg Rose <[email protected]>
> 
> Hi,
> 
> How much of a difference does it make? Is it measurable?


Hi,

It's definitely measurable. On my setup (similar to pvp with one vm
running dpdk and 2 flows in ovs to forward packets, all running on an
arm server), this patch shows an improvement of ~1.6% in throughput.


> 
> Otherwise, I know 'depth' has been used in other places for a pointer
> to this TLS value,
> but 'recirc_depth' might be clearer to a reader?
> 


By reading the TLS variable and storing it in a const variable on the
stack explicitly communicates to the reader that the value doesn't
change while processing the current batch of packets. I feel that, in
this case, it actually makes the code a bit easier to understand.

Let me know what you think,
Balazs


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

Reply via email to