netdev - Re: [PATCH v3 net] ionic: fix ptp support config breakage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 May 2021 18:16:49 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Shannon Nelson <snelson@...sando.io>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, drivers@...sando.io,
        Allen Hubbe <allenbh@...sando.io>
Subject: Re: [PATCH v3 net] ionic: fix ptp support config breakage

On 5/6/21 6:12 PM, Randy Dunlap wrote:
> On 5/6/21 5:48 PM, Shannon Nelson wrote:
>> On 5/6/21 5:21 PM, Randy Dunlap wrote:
>>> On 5/6/21 5:15 PM, Jakub Kicinski wrote:
>>>> On Wed,  5 May 2021 21:18:46 -0700 Shannon Nelson wrote:
>>>>> Driver link failed with undefined references in some
>>>>> kernel config variations.
>>>> This is really vague and the patch is not very obvious.
>>>>
>>>>>   ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
>>>>>          ionic_debugfs.o ionic_lif.o ionic_rx_filter.o ionic_ethtool.o \
>>>>> -       ionic_txrx.o ionic_stats.o ionic_fw.o
>>>>> -ionic-$(CONFIG_PTP_1588_CLOCK) += ionic_phc.o
>>>>> +       ionic_txrx.o ionic_stats.o ionic_fw.o ionic_phc.o
>>>> So we'd replace a build dependency..
>>>>
>>>>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>> index a87c87e86aef..30c78808c45a 100644
>>>>> --- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
>>>>> @@ -1,6 +1,8 @@
>>>>>   // SPDX-License-Identifier: GPL-2.0
>>>>>   /* Copyright(c) 2017 - 2021 Pensando Systems, Inc */
>>>>>   +#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>>>> +
>>>>>   #include <linux/netdevice.h>
>>>>>   #include <linux/etherdevice.h>
>>>>>   @@ -613,3 +615,4 @@ void ionic_lif_free_phc(struct ionic_lif *lif)
>>>>>       devm_kfree(lif->ionic->dev, lif->phc);
>>>>>       lif->phc = NULL;
>>>>>   }
>>>>> +#endif /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
>>>> .. with an ifdef around an entire file? Does not feel very clean.
>>>>
>>>> The construct of using:
>>>>
>>>>     drv-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>>>>
>>>> seems relatively common, why does it now work here?
>>>>
>>>> Maybe the config in question has PTP as a module and ionic built in?
>>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
>>>>
>>>> Maybe somehow the "ionic-y" confuses kbuild and it should be ionic-objs?
>>>>
>>>> At the very least we need a better explanation in the commit message.
>>>>
>>> I'll take a look if someone can point me to the .config file.
>>>
>>
>> These are the notes I got from kernel test robot:
>> https://lore.kernel.org/lkml/202105041020.efEaBOYC-lkp@intel.com/
>> https://lore.kernel.org/lkml/202105041154.GrLZmjGh-lkp@intel.com/
>> https://lore.kernel.org/lkml/202105041634.paURyDp0-lkp@intel.com/
>> https://lore.kernel.org/lkml/202105050636.UXXDl7m2-lkp@intel.com/
> 
> At first glance it looks like Jakub's suggestion of
>>>> Maybe the config in question has PTP as a module and ionic built in?
>>>> Then you should add depends on PTP_1588_CLOCK || !PTP_1588_CLOCK.
> 
> is what is needed, but I'm still doing build testing ATM.

Nope, eat my words. These build issues are not about PTP.
I'm still looking.

-- 
~Randy