diff mbox series

[ovs-dev] dpif-netdev: Only hash port number when necessary.

Message ID 20220429154433.2750870-1-cian.ferriter@intel.com
State Accepted
Commit 5ec5473304fe469546ac5c5dc35d2600f1a092ff
Headers show
Series [ovs-dev] dpif-netdev: Only hash port number when necessary. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Ferriter, Cian April 29, 2022, 3:44 p.m. UTC
The hash of the port number is only needed when a DPCLS needs to be
created. Move the hash calculation inside the if to accomplish this.

Signed-off-by: Cian Ferriter <cian.ferriter@intel.com>
---
 lib/dpif-netdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Mike Pattrick May 10, 2022, 3:03 p.m. UTC | #1
On Fri, Apr 29, 2022 at 11:44 AM Cian Ferriter <cian.ferriter@intel.com> wrote:
>
> The hash of the port number is only needed when a DPCLS needs to be
> created. Move the hash calculation inside the if to accomplish this.
>
> Signed-off-by: Cian Ferriter <cian.ferriter@intel.com>
> ---

Good catch!

Acked-by: Mike Pattrick <mkp@redhat.com>
Ilya Maximets May 18, 2022, 10:35 a.m. UTC | #2
On 5/10/22 17:03, Mike Pattrick wrote:
> On Fri, Apr 29, 2022 at 11:44 AM Cian Ferriter <cian.ferriter@intel.com> wrote:
>>
>> The hash of the port number is only needed when a DPCLS needs to be
>> created. Move the hash calculation inside the if to accomplish this.
>>
>> Signed-off-by: Cian Ferriter <cian.ferriter@intel.com>
>> ---
> 
> Good catch!
> 
> Acked-by: Mike Pattrick <mkp@redhat.com>

I'd guess that compiler will produce identical code in most cases,
but the change makes the code a bit cleaner.

Applied.  Thanks!

Best regards, Ilya Maximets.
Ferriter, Cian May 18, 2022, 1:16 p.m. UTC | #3
> -----Original Message-----
> From: Ilya Maximets <i.maximets@ovn.org>
> Sent: Wednesday 18 May 2022 11:35
> To: Mike Pattrick <mkp@redhat.com>; Ferriter, Cian <cian.ferriter@intel.com>
> Cc: ovs-dev@openvswitch.org; i.maximets@ovn.org
> Subject: Re: [ovs-dev] [PATCH] dpif-netdev: Only hash port number when necessary.
> 
> On 5/10/22 17:03, Mike Pattrick wrote:
> > On Fri, Apr 29, 2022 at 11:44 AM Cian Ferriter <cian.ferriter@intel.com> wrote:
> >>
> >> The hash of the port number is only needed when a DPCLS needs to be
> >> created. Move the hash calculation inside the if to accomplish this.
> >>
> >> Signed-off-by: Cian Ferriter <cian.ferriter@intel.com>
> >> ---
> >
> > Good catch!
> >
> > Acked-by: Mike Pattrick <mkp@redhat.com>
> 
> I'd guess that compiler will produce identical code in most cases,
> but the change makes the code a bit cleaner.
> 
> Applied.  Thanks!
> 
> Best regards, Ilya Maximets.

Thanks for the review Mike and for applying Ilya.

Agreed, the compiler should clean this up for us. I was thinking of the change as a code readability improvement rather than an optimization.
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 676434308..6a607b96a 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2412,9 +2412,10 @@  dp_netdev_pmd_find_dpcls(struct dp_netdev_pmd_thread *pmd,
     OVS_REQUIRES(pmd->flow_mutex)
 {
     struct dpcls *cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port);
-    uint32_t hash = hash_port_no(in_port);
 
     if (!cls) {
+        uint32_t hash = hash_port_no(in_port);
+
         /* Create new classifier for in_port */
         cls = xmalloc(sizeof(*cls));
         dpcls_init(cls);