diff for duplicates of <1573544002.10368.34.camel@harmonicinc.com> diff --git a/a/1.txt b/N1/1.txt index c0cc45c..a3818ce 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -7,9 +7,9 @@ On Fri, 2019-11-08 at 16:43 +0000, Creeley, Brett wrote: > > -----Original Message----- > > From: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com> > > Sent: Thursday, November 7, 2019 11:39 AM -> > To: Arkady Gilinsky <arkady.gilinsky@harmonicinc.com>; Creeley, Brett <brett.creeley@intel.com>; intel-wired-lan at lis +> > To: Arkady Gilinsky <arkady.gilinsky@harmonicinc.com>; Creeley, Brett <brett.creeley@intel.com>; intel-wired-lan@lis > > ts.osuosl.org; -> > netdev at vger.kernel.org +> > netdev@vger.kernel.org > > Cc: Arkady Gilinsky <arcadyg@gmail.com> > > Subject: Re: [EXTERNAL] RE: [PATCH net] i40e/iavf: Fix msg interface between VF and PF > > @@ -19,7 +19,7 @@ On Fri, 2019-11-08 at 16:43 +0000, Creeley, Brett wrote: > > > > > From: Arkady Gilinsky <arkady.gilinsky@harmonicinc.com> > > > > > Sent: Monday, November 4, 2019 9:24 PM > > > > > To: Creeley, Brett <brett.creeley@intel.com>; -> > > > > intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; Kirsher, +> > > > > intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Kirsher, > > > > > Jeffrey T > > > > > <jeffrey.t.kirsher@intel.com> > > > > > Cc: Arkady Gilinsky <arcadyg@gmail.com> @@ -28,11 +28,11 @@ On Fri, 2019-11-08 at 16:43 +0000, Creeley, Brett wrote: > > > > > > static bool i40e_vc_verify_vqs_bitmaps(struct virtchnl_queue_select > > > > > > *vqs) > > > > > > { -> > > > > > ???/* this will catch any changes made to the virtchnl_queue_select +> > > > > > /* this will catch any changes made to the virtchnl_queue_select > > > > > > bitmap */ -> > > > > > ???if (sizeof(vqs->rx_queues) != sizeof(u32) || -> > > > > > ????????sizeof(vqs->tx_queues) != sizeof(u32)) -> > > > > > ???????????return false; +> > > > > > if (sizeof(vqs->rx_queues) != sizeof(u32) || +> > > > > > sizeof(vqs->tx_queues) != sizeof(u32)) +> > > > > > return false; > > > > > > > > > > If so, then is it better to check the type of the fields in compile- > > > > > time rather than in runtime ? @@ -47,19 +47,19 @@ On Fri, 2019-11-08 at 16:43 +0000, Creeley, Brett wrote: > > > Agree. > > > If other code in driver not need to be adjusted/verified, then this check > > > is not needed. -> > > > > > ???if ((vqs->rx_queues == 0 && vqs->tx_queues == 0) || -> > > > > > ?????????hweight32(vqs->rx_queues) > I40E_MAX_VF_QUEUES || -> > > > > > ?????????hweight32(vqs->tx_queues) > I40E_MAX_VF_QUEUES) -> > > > > > ???????????return false; +> > > > > > if ((vqs->rx_queues == 0 && vqs->tx_queues == 0) || +> > > > > > hweight32(vqs->rx_queues) > I40E_MAX_VF_QUEUES || +> > > > > > hweight32(vqs->tx_queues) > I40E_MAX_VF_QUEUES) +> > > > > > return false; > > > > > > > > > > Again, from optimization POV it is better to have constant changed > > > > > than variable, > > > > > since it is compile time and not run time action: -> > > > > ?????if ((vqs->rx_queues == 0 && vqs->tx_queues == 0) || -> > > > > ???????????vqs->rx_queues >= (BIT(I40E_MAX_VF_QUEUES)) || +> > > > > if ((vqs->rx_queues == 0 && vqs->tx_queues == 0) || +> > > > > vqs->rx_queues >= (BIT(I40E_MAX_VF_QUEUES)) || > > > > > -> > > > > ??????vqs->tx_queues >= (BIT(I40E_MAX_VF_QUEUES))) -> > > > > ?????????????return false; +> > > > > vqs->tx_queues >= (BIT(I40E_MAX_VF_QUEUES))) +> > > > > return false; > > > > > > > > This seems much better than my solution. It fixes the original issue, > > > > handles if the @@ -78,7 +78,7 @@ On Fri, 2019-11-08 at 16:43 +0000, Creeley, Brett wrote: > > > > As Brett pointed out, there are issues with this patch. Please make the > > suggested changes and re-submit the patch to -> > intel-wired-lan at lists.osuosl.org +> > intel-wired-lan@lists.osuosl.org > > Jeff/Arkady: I have already submitted patches for this internally for > official Intel drivers. Apologies for the delayed response. diff --git a/a/content_digest b/N1/content_digest index 8a87d73..88cfe2b 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -6,9 +6,15 @@ "ref\0d078d3efc784805a67ba1a1c6e94fb4ec1c0aec6.camel@intel.com\0" "ref\03508A0C5D531054DBDD98909F6FA64FA11B3EB75@ORSMSX113.amr.corp.intel.com\0" "From\0Arkady Gilinsky <arkady.gilinsky@harmonicinc.com>\0" - "Subject\0[Intel-wired-lan] [EXTERNAL] RE: [PATCH net] i40e/iavf: Fix msg interface between VF and PF\0" + "Subject\0Re: [EXTERNAL] RE: [PATCH net] i40e/iavf: Fix msg interface between VF and PF\0" "Date\0Tue, 12 Nov 2019 07:33:30 +0000\0" - "To\0intel-wired-lan@osuosl.org\0" + "To\0Creeley" + Brett <brett.creeley@intel.com> + Kirsher + Jeffrey T <jeffrey.t.kirsher@intel.com> + intel-wired-lan@lists.osuosl.org <intel-wired-lan@lists.osuosl.org> + " netdev@vger.kernel.org <netdev@vger.kernel.org>\0" + "Cc\0Arkady Gilinsky <arcadyg@gmail.com>\0" "\00:1\0" "b\0" "Hi All,\n" @@ -20,9 +26,9 @@ "> > -----Original Message-----\n" "> > From: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>\n" "> > Sent: Thursday, November 7, 2019 11:39 AM\n" - "> > To: Arkady Gilinsky <arkady.gilinsky@harmonicinc.com>; Creeley, Brett <brett.creeley@intel.com>; intel-wired-lan at lis\n" + "> > To: Arkady Gilinsky <arkady.gilinsky@harmonicinc.com>; Creeley, Brett <brett.creeley@intel.com>; intel-wired-lan@lis\n" "> > ts.osuosl.org;\n" - "> > netdev at vger.kernel.org\n" + "> > netdev@vger.kernel.org\n" "> > Cc: Arkady Gilinsky <arcadyg@gmail.com>\n" "> > Subject: Re: [EXTERNAL] RE: [PATCH net] i40e/iavf: Fix msg interface between VF and PF\n" "> > \n" @@ -32,7 +38,7 @@ "> > > > > From: Arkady Gilinsky <arkady.gilinsky@harmonicinc.com>\n" "> > > > > Sent: Monday, November 4, 2019 9:24 PM\n" "> > > > > To: Creeley, Brett <brett.creeley@intel.com>;\n" - "> > > > > intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; Kirsher,\n" + "> > > > > intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Kirsher,\n" "> > > > > Jeffrey T\n" "> > > > > <jeffrey.t.kirsher@intel.com>\n" "> > > > > Cc: Arkady Gilinsky <arcadyg@gmail.com>\n" @@ -41,11 +47,11 @@ "> > > > > > static bool i40e_vc_verify_vqs_bitmaps(struct virtchnl_queue_select\n" "> > > > > > *vqs)\n" "> > > > > > {\n" - "> > > > > > ???/* this will catch any changes made to the virtchnl_queue_select\n" + "> > > > > > \302\240\302\240\302\240/* this will catch any changes made to the virtchnl_queue_select\n" "> > > > > > bitmap */\n" - "> > > > > > ???if (sizeof(vqs->rx_queues) != sizeof(u32) ||\n" - "> > > > > > ????????sizeof(vqs->tx_queues) != sizeof(u32))\n" - "> > > > > > ???????????return false;\n" + "> > > > > > \302\240\302\240\302\240if (sizeof(vqs->rx_queues) != sizeof(u32) ||\n" + "> > > > > > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240sizeof(vqs->tx_queues) != sizeof(u32))\n" + "> > > > > > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240return false;\n" "> > > > > \n" "> > > > > If so, then is it better to check the type of the fields in compile-\n" "> > > > > time rather than in runtime ?\n" @@ -60,19 +66,19 @@ "> > > Agree.\n" "> > > If other code in driver not need to be adjusted/verified, then this check\n" "> > > is not needed.\n" - "> > > > > > ???if ((vqs->rx_queues == 0 && vqs->tx_queues == 0) ||\n" - "> > > > > > ?????????hweight32(vqs->rx_queues) > I40E_MAX_VF_QUEUES ||\n" - "> > > > > > ?????????hweight32(vqs->tx_queues) > I40E_MAX_VF_QUEUES)\n" - "> > > > > > ???????????return false;\n" + "> > > > > > \302\240\302\240\302\240if ((vqs->rx_queues == 0 && vqs->tx_queues == 0) ||\n" + "> > > > > > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240hweight32(vqs->rx_queues) > I40E_MAX_VF_QUEUES ||\n" + "> > > > > > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240hweight32(vqs->tx_queues) > I40E_MAX_VF_QUEUES)\n" + "> > > > > > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240return false;\n" "> > > > > \n" "> > > > > Again, from optimization POV it is better to have constant changed\n" "> > > > > than variable,\n" "> > > > > since it is compile time and not run time action:\n" - "> > > > > ?????if ((vqs->rx_queues == 0 && vqs->tx_queues == 0) ||\n" - "> > > > > ???????????vqs->rx_queues >= (BIT(I40E_MAX_VF_QUEUES)) ||\n" + "> > > > > \302\240\302\240\302\240\302\240\302\240if ((vqs->rx_queues == 0 && vqs->tx_queues == 0) ||\n" + "> > > > > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240vqs->rx_queues >= (BIT(I40E_MAX_VF_QUEUES)) ||\n" "> > > > > \n" - "> > > > > ??????vqs->tx_queues >= (BIT(I40E_MAX_VF_QUEUES)))\n" - "> > > > > ?????????????return false;\n" + "> > > > > \302\240\302\240\302\240\302\240\302\240\302\240vqs->tx_queues >= (BIT(I40E_MAX_VF_QUEUES)))\n" + "> > > > > \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240return false;\n" "> > > > \n" "> > > > This seems much better than my solution. It fixes the original issue,\n" "> > > > handles if the\n" @@ -91,7 +97,7 @@ "> > \n" "> > As Brett pointed out, there are issues with this patch. Please make the\n" "> > suggested changes and re-submit the patch to\n" - "> > intel-wired-lan at lists.osuosl.org\n" + "> > intel-wired-lan@lists.osuosl.org\n" "> \n" "> Jeff/Arkady: I have already submitted patches for this internally for\n" "> official Intel drivers. Apologies for the delayed response.\n" @@ -101,4 +107,4 @@ "\n" ------------------------------------------ -997e98dee40c374d8845311aed29cbc133773854071e2cc09decb3dcefecdfac +488f0a00f45c7817d29b6990efb855fe3f8017d99ae6605601dee21f84b17cb6
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.