| Message ID | 1536319175-3660-3-git-send-email-dongsheng.wang@hxt-semitech.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: ethernet: i40e/i40evf fix build error | expand |
On 9/7/2018 11:33 PM, Sergei Shtylyov wrote: > On 09/07/2018 02:19 PM, Wang Dongsheng wrote: > >> Can't have non-inline function in a header file. >> There is a risk of "Multiple definition" from cross-including. >> >> Tested on: x86_64, make ARCH=i386 >> >> Modules section .text: >> i40e: 00019380 <__i40e_add_stat_strings>: >> i40evf: 00006b00 <__i40e_add_stat_strings>: >> >> Buildin section .text: >> i40e: c351ca60 <__i40e_add_stat_strings>: >> i40evf: c354f2c0 <__i40e_add_stat_strings>: >> >> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com> >> --- >> V3: add static >> --- >> .../intel/i40evf/i40e_ethtool_stats.h | 23 +----------------- >> .../ethernet/intel/i40evf/i40evf_ethtool.c | 24 +++++++++++++++++++ >> 2 files changed, 25 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h >> index 60b595dd8c39..62ab67a77753 100644 >> --- a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h >> +++ b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h >> @@ -181,29 +181,8 @@ i40evf_add_queue_stats(u64 **data, struct i40e_ring *ring) >> *data += size; >> } >> >> -/** >> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer >> - * @p: ethtool supplied buffer >> - * @stats: stat definitions array >> - * @size: size of the stats array >> - * >> - * Format and copy the strings described by stats into the buffer pointed at >> - * by p. >> - **/ >> static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[], > There's no point to keeping *static* function in the header file (unless it's > also *inline*). Yes， we need it for now. Because there is a copy file at i40e dir, and a "Multiple definition" will show up when we buildin i40e&i40evf and remove this *static* . Cause the header file is only used in ethtool.c so we can keep this static, and another option is not touch this header. As I replied to Jacob's email earlier, we can do without touch i40evf at all. Because this header is only for one and not included in another. Cheers, Dongsheng >> - const unsigned int size, ...) >> -{ >> - unsigned int i; >> - >> - for (i = 0; i < size; i++) { >> - va_list args; >> - >> - va_start(args, size); >> - vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args); >> - *p += ETH_GSTRING_LEN; >> - va_end(args); >> - } >> -} >> + const unsigned int size, ...); >> >> /** >> * 40e_add_stat_strings - copy stat strings into ethtool buffer > [...] > > MBR, Sergei >
On 9/7/2018 11:27 PM, Wyborny, Carolyn wrote: >> -----Original Message----- >> From: Wang, Dongsheng [mailto:dongsheng.wang@hxt-semitech.com] >> Sent: Friday, September 07, 2018 5:34 AM >> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; >> sergei.shtylyov@cogentembedded.com >> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; davem@davemloft.net; >> intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux- >> kernel@vger.kernel.org; Wyborny, Carolyn <carolyn.wyborny@intel.com> >> Subject: Re: [PATCH v3 2/2] net: ethernet: i40evf: fix underlying build error >> >> Hello Jacob, >> >> Since Carolyn' team is working this, I think we don't need this patch >> anymore because this header file is only for ethtool.c. >> [..] > Hello Dongsheng, > > The commonization effort we're working on is prioritizing our newest drivers. The i40e work is still being scoped, so we should fix this problem as needed now and not wait. After Sergei agree. ;) Cheers, Dongsheng > I apologize for any miscommunication. Was trying to let people know that we aware of the issue and are trying to make progress in that direction. > > Thanks, > > Carolyn >
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h index 60b595dd8c39..62ab67a77753 100644 --- a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h +++ b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h @@ -181,29 +181,8 @@ i40evf_add_queue_stats(u64 **data, struct i40e_ring *ring) *data += size; } -/** - * __i40e_add_stat_strings - copy stat strings into ethtool buffer - * @p: ethtool supplied buffer - * @stats: stat definitions array - * @size: size of the stats array - * - * Format and copy the strings described by stats into the buffer pointed at - * by p. - **/ static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[], - const unsigned int size, ...) -{ - unsigned int i; - - for (i = 0; i < size; i++) { - va_list args; - - va_start(args, size); - vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args); - *p += ETH_GSTRING_LEN; - va_end(args); - } -} + const unsigned int size, ...); /** * 40e_add_stat_strings - copy stat strings into ethtool buffer diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c index 9319971c5c92..eb2e910bc3a1 100644 --- a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c +++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c @@ -171,6 +171,30 @@ static void i40evf_get_priv_flag_strings(struct net_device *netdev, u8 *data) } } +/** + * __i40e_add_stat_strings - copy stat strings into ethtool buffer + * @p: ethtool supplied buffer + * @stats: stat definitions array + * @size: size of the stats array + * + * Format and copy the strings described by stats into the buffer pointed at + * by p. + **/ +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[], + const unsigned int size, ...) +{ + unsigned int i; + + for (i = 0; i < size; i++) { + va_list args; + + va_start(args, size); + vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args); + *p += ETH_GSTRING_LEN; + va_end(args); + } +} + /** * i40evf_get_stat_strings - Get stat strings * @netdev: network interface device structure
Can't have non-inline function in a header file. There is a risk of "Multiple definition" from cross-including. Tested on: x86_64, make ARCH=i386 Modules section .text: i40e: 00019380 <__i40e_add_stat_strings>: i40evf: 00006b00 <__i40e_add_stat_strings>: Buildin section .text: i40e: c351ca60 <__i40e_add_stat_strings>: i40evf: c354f2c0 <__i40e_add_stat_strings>: Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com> --- V3: add static --- .../intel/i40evf/i40e_ethtool_stats.h | 23 +----------------- .../ethernet/intel/i40evf/i40evf_ethtool.c | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 22 deletions(-)