mbox series

[0/1,SRU,H/OEM-5.13/OEM-5.14/U] UBUNTU: SAUCE: Fix invalid MAC address after hotplug tbt dock

Message ID 20210908084952.41486-1-aaron.ma@canonical.com
Headers show
Series UBUNTU: SAUCE: Fix invalid MAC address after hotplug tbt dock | expand

Message

Aaron Ma Sept. 8, 2021, 8:49 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1942999

SRU justification:

[Impact]
igc driver can not connect to network after re-plugin thunderbolt dock
when
MAC passthrough enabled in BIOS.

[Fix]
Wait for the MAC copy of BIOS when enabled MAC passthrough.
Intel engineer wants a different solution and promise to discuss with
firmware engineer.
Due to the schedule, made this as a short term solution to fix the
issue, and wait for the other
fix from Intel.

[Test]
Verified on hardware, after hotplug the thunderbolt cable,
Ethernet works fine.

[Where problems could occur]
It may break the igc driver.

Aaron Ma (1):
  UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough

 drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Tim Gardner Sept. 8, 2021, 12:06 p.m. UTC | #1
Ick. It seems like there ought to be a more deterministic way to note 
when the MAC address has been copied. Why not a loop that does an 
msleep(1) until the MAC is address is updated or 600 msec elapses ?

On 9/8/21 2:49 AM, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1942999
> 
> SRU justification:
> 
> [Impact]
> igc driver can not connect to network after re-plugin thunderbolt dock
> when
> MAC passthrough enabled in BIOS.
> 
> [Fix]
> Wait for the MAC copy of BIOS when enabled MAC passthrough.
> Intel engineer wants a different solution and promise to discuss with
> firmware engineer.
> Due to the schedule, made this as a short term solution to fix the
> issue, and wait for the other
> fix from Intel.
> 
> [Test]
> Verified on hardware, after hotplug the thunderbolt cable,
> Ethernet works fine.
> 
> [Where problems could occur]
> It may break the igc driver.
> 
> Aaron Ma (1):
>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
> 
>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>   1 file changed, 3 insertions(+)
>
Aaron Ma Sept. 8, 2021, 12:39 p.m. UTC | #2
On 9/8/21 8:06 PM, Tim Gardner wrote:
> 
> Ick. It seems like there ought to be a more deterministic way to note when the MAC address has been copied. Why not a loop that does an msleep(1) until the MAC is address is updated or 600 msec elapses ?

The MAC address of dock will read out at first, after 600 msec the MAC in BIOS will be updated.
Driver doesn't know which one is the right MAC address. This allows enough time for hardware to update the

MAC address.

Aaron

> 
> On 9/8/21 2:49 AM, Aaron Ma wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1942999
>>
>> SRU justification:
>>
>> [Impact]
>> igc driver can not connect to network after re-plugin thunderbolt dock
>> when
>> MAC passthrough enabled in BIOS.
>>
>> [Fix]
>> Wait for the MAC copy of BIOS when enabled MAC passthrough.
>> Intel engineer wants a different solution and promise to discuss with
>> firmware engineer.
>> Due to the schedule, made this as a short term solution to fix the
>> issue, and wait for the other
>> fix from Intel.
>>
>> [Test]
>> Verified on hardware, after hotplug the thunderbolt cable,
>> Ethernet works fine.
>>
>> [Where problems could occur]
>> It may break the igc driver.
>>
>> Aaron Ma (1):
>>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
>>
>>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>
Kai-Heng Feng Sept. 10, 2021, 5:12 a.m. UTC | #3
On Wed, Sep 8, 2021 at 8:40 PM Aaron Ma <aaron.ma@canonical.com> wrote:
>
>
> On 9/8/21 8:06 PM, Tim Gardner wrote:
> >
> > Ick. It seems like there ought to be a more deterministic way to note when the MAC address has been copied. Why not a loop that does an msleep(1) until the MAC is address is updated or 600 msec elapses ?
>
> The MAC address of dock will read out at first, after 600 msec the MAC in BIOS will be updated.
> Driver doesn't know which one is the right MAC address. This allows enough time for hardware to update the

According to [1], there _can_ be IRQ raised when MAC update is
completed. So it's a good idea to check it.
If there's no IRQ from FW at current stage, then this is an unpleasant
but necessary workaround.

[1] https://lore.kernel.org/netdev/20daa122-aaec-0c6b-23f5-d2be2fcab1e9@intel.com/

Kai-Heng

>
> MAC address.
>
> Aaron
>
> >
> > On 9/8/21 2:49 AM, Aaron Ma wrote:
> >> BugLink: https://bugs.launchpad.net/bugs/1942999
> >>
> >> SRU justification:
> >>
> >> [Impact]
> >> igc driver can not connect to network after re-plugin thunderbolt dock
> >> when
> >> MAC passthrough enabled in BIOS.
> >>
> >> [Fix]
> >> Wait for the MAC copy of BIOS when enabled MAC passthrough.
> >> Intel engineer wants a different solution and promise to discuss with
> >> firmware engineer.
> >> Due to the schedule, made this as a short term solution to fix the
> >> issue, and wait for the other
> >> fix from Intel.
> >>
> >> [Test]
> >> Verified on hardware, after hotplug the thunderbolt cable,
> >> Ethernet works fine.
> >>
> >> [Where problems could occur]
> >> It may break the igc driver.
> >>
> >> Aaron Ma (1):
> >>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
> >>
> >>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> >
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Aaron Ma Sept. 10, 2021, 5:59 a.m. UTC | #4
On 9/10/21 1:12 PM, Kai-Heng Feng wrote:
> On Wed, Sep 8, 2021 at 8:40 PM Aaron Ma <aaron.ma@canonical.com> wrote:
>>
>>
>> On 9/8/21 8:06 PM, Tim Gardner wrote:
>>>
>>> Ick. It seems like there ought to be a more deterministic way to note when the MAC address has been copied. Why not a loop that does an msleep(1) until the MAC is address is updated or 600 msec elapses ?
>>
>> The MAC address of dock will read out at first, after 600 msec the MAC in BIOS will be updated.
>> Driver doesn't know which one is the right MAC address. This allows enough time for hardware to update the
> 
> According to [1], there _can_ be IRQ raised when MAC update is
> completed. So it's a good idea to check it.
> If there's no IRQ from FW at current stage, then this is an unpleasant
> but necessary workaround.
> 
> [1] https://lore.kernel.org/netdev/20daa122-aaec-0c6b-23f5-d2be2fcab1e9@intel.com/
> 

In current fw, no such irq.

Aaron

> Kai-Heng
> 
>>
>> MAC address.
>>
>> Aaron
>>
>>>
>>> On 9/8/21 2:49 AM, Aaron Ma wrote:
>>>> BugLink: https://bugs.launchpad.net/bugs/1942999
>>>>
>>>> SRU justification:
>>>>
>>>> [Impact]
>>>> igc driver can not connect to network after re-plugin thunderbolt dock
>>>> when
>>>> MAC passthrough enabled in BIOS.
>>>>
>>>> [Fix]
>>>> Wait for the MAC copy of BIOS when enabled MAC passthrough.
>>>> Intel engineer wants a different solution and promise to discuss with
>>>> firmware engineer.
>>>> Due to the schedule, made this as a short term solution to fix the
>>>> issue, and wait for the other
>>>> fix from Intel.
>>>>
>>>> [Test]
>>>> Verified on hardware, after hotplug the thunderbolt cable,
>>>> Ethernet works fine.
>>>>
>>>> [Where problems could occur]
>>>> It may break the igc driver.
>>>>
>>>> Aaron Ma (1):
>>>>     UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
>>>>
>>>>    drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>
>>
>> --
>> kernel-team mailing list
>> kernel-team@lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Kai-Heng Feng Sept. 10, 2021, 6:03 a.m. UTC | #5
On Fri, Sep 10, 2021 at 1:59 PM Aaron Ma <aaron.ma@canonical.com> wrote:
>
>
> On 9/10/21 1:12 PM, Kai-Heng Feng wrote:
> > On Wed, Sep 8, 2021 at 8:40 PM Aaron Ma <aaron.ma@canonical.com> wrote:
> >>
> >>
> >> On 9/8/21 8:06 PM, Tim Gardner wrote:
> >>>
> >>> Ick. It seems like there ought to be a more deterministic way to note when the MAC address has been copied. Why not a loop that does an msleep(1) until the MAC is address is updated or 600 msec elapses ?
> >>
> >> The MAC address of dock will read out at first, after 600 msec the MAC in BIOS will be updated.
> >> Driver doesn't know which one is the right MAC address. This allows enough time for hardware to update the
> >
> > According to [1], there _can_ be IRQ raised when MAC update is
> > completed. So it's a good idea to check it.
> > If there's no IRQ from FW at current stage, then this is an unpleasant
> > but necessary workaround.
> >
> > [1] https://lore.kernel.org/netdev/20daa122-aaec-0c6b-23f5-d2be2fcab1e9@intel.com/
> >
>
> In current fw, no such irq.

Since it's confirmed there's no IRQ raised on MAC update completion,
it's plausible to use this workaround.

Acked-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

>
> Aaron
>
> > Kai-Heng
> >
> >>
> >> MAC address.
> >>
> >> Aaron
> >>
> >>>
> >>> On 9/8/21 2:49 AM, Aaron Ma wrote:
> >>>> BugLink: https://bugs.launchpad.net/bugs/1942999
> >>>>
> >>>> SRU justification:
> >>>>
> >>>> [Impact]
> >>>> igc driver can not connect to network after re-plugin thunderbolt dock
> >>>> when
> >>>> MAC passthrough enabled in BIOS.
> >>>>
> >>>> [Fix]
> >>>> Wait for the MAC copy of BIOS when enabled MAC passthrough.
> >>>> Intel engineer wants a different solution and promise to discuss with
> >>>> firmware engineer.
> >>>> Due to the schedule, made this as a short term solution to fix the
> >>>> issue, and wait for the other
> >>>> fix from Intel.
> >>>>
> >>>> [Test]
> >>>> Verified on hardware, after hotplug the thunderbolt cable,
> >>>> Ethernet works fine.
> >>>>
> >>>> [Where problems could occur]
> >>>> It may break the igc driver.
> >>>>
> >>>> Aaron Ma (1):
> >>>>     UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
> >>>>
> >>>>    drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
> >>>>    1 file changed, 3 insertions(+)
> >>>>
> >>>
> >>
> >> --
> >> kernel-team mailing list
> >> kernel-team@lists.ubuntu.com
> >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Timo Aaltonen Sept. 12, 2021, 2:56 p.m. UTC | #6
On 8.9.2021 11.49, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1942999
> 
> SRU justification:
> 
> [Impact]
> igc driver can not connect to network after re-plugin thunderbolt dock
> when
> MAC passthrough enabled in BIOS.
> 
> [Fix]
> Wait for the MAC copy of BIOS when enabled MAC passthrough.
> Intel engineer wants a different solution and promise to discuss with
> firmware engineer.
> Due to the schedule, made this as a short term solution to fix the
> issue, and wait for the other
> fix from Intel.
> 
> [Test]
> Verified on hardware, after hotplug the thunderbolt cable,
> Ethernet works fine.
> 
> [Where problems could occur]
> It may break the igc driver.
> 
> Aaron Ma (1):
>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
> 
>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>   1 file changed, 3 insertions(+)
> 

Should this be for impish (5.13) too?
Aaron Ma Sept. 13, 2021, 2:16 a.m. UTC | #7
On 9/12/21 22:56, Timo Aaltonen wrote:
> On 8.9.2021 11.49, Aaron Ma wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1942999
>>
>> SRU justification:
>>
>> [Impact]
>> igc driver can not connect to network after re-plugin thunderbolt dock
>> when
>> MAC passthrough enabled in BIOS.
>>
>> [Fix]
>> Wait for the MAC copy of BIOS when enabled MAC passthrough.
>> Intel engineer wants a different solution and promise to discuss with
>> firmware engineer.
>> Due to the schedule, made this as a short term solution to fix the
>> issue, and wait for the other
>> fix from Intel.
>>
>> [Test]
>> Verified on hardware, after hotplug the thunderbolt cable,
>> Ethernet works fine.
>>
>> [Where problems could occur]
>> It may break the igc driver.
>>
>> Aaron Ma (1):
>>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
>>
>>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
> 
> Should this be for impish (5.13) too?
> 

Yes.

Aaron
Aaron Ma Sept. 13, 2021, 5:52 a.m. UTC | #8
Sorry for missing Impish tag.
Please apply for Impish too.
LP bug tag updated with subject.

Please let me know if you like a v2.

Thanks,
Aaron

On 9/13/21 10:16, Aaron Ma wrote:
> 
> 
> On 9/12/21 22:56, Timo Aaltonen wrote:
>> On 8.9.2021 11.49, Aaron Ma wrote:
>>> BugLink: https://bugs.launchpad.net/bugs/1942999
>>>
>>> SRU justification:
>>>
>>> [Impact]
>>> igc driver can not connect to network after re-plugin thunderbolt dock
>>> when
>>> MAC passthrough enabled in BIOS.
>>>
>>> [Fix]
>>> Wait for the MAC copy of BIOS when enabled MAC passthrough.
>>> Intel engineer wants a different solution and promise to discuss with
>>> firmware engineer.
>>> Due to the schedule, made this as a short term solution to fix the
>>> issue, and wait for the other
>>> fix from Intel.
>>>
>>> [Test]
>>> Verified on hardware, after hotplug the thunderbolt cable,
>>> Ethernet works fine.
>>>
>>> [Where problems could occur]
>>> It may break the igc driver.
>>>
>>> Aaron Ma (1):
>>>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
>>>
>>>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>
>> Should this be for impish (5.13) too?
>>
> 
> Yes.
> 
> Aaron
>
Stefan Bader Sept. 22, 2021, 1:22 p.m. UTC | #9
On 08.09.21 10:49, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1942999
> 
> SRU justification:
> 
> [Impact]
> igc driver can not connect to network after re-plugin thunderbolt dock
> when
> MAC passthrough enabled in BIOS.
> 
> [Fix]
> Wait for the MAC copy of BIOS when enabled MAC passthrough.
> Intel engineer wants a different solution and promise to discuss with
> firmware engineer.
> Due to the schedule, made this as a short term solution to fix the
> issue, and wait for the other
> fix from Intel.

What is the status for the proper fix?

-Stefan

> 
> [Test]
> Verified on hardware, after hotplug the thunderbolt cable,
> Ethernet works fine.
> 
> [Where problems could occur]
> It may break the igc driver.
> 
> Aaron Ma (1):
>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
> 
>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>   1 file changed, 3 insertions(+)
>
Aaron Ma Sept. 22, 2021, 2:10 p.m. UTC | #10
On 9/22/21 21:22, Stefan Bader wrote:
> On 08.09.21 10:49, Aaron Ma wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1942999
>>
>> SRU justification:
>>
>> [Impact]
>> igc driver can not connect to network after re-plugin thunderbolt dock
>> when
>> MAC passthrough enabled in BIOS.
>>
>> [Fix]
>> Wait for the MAC copy of BIOS when enabled MAC passthrough.
>> Intel engineer wants a different solution and promise to discuss with
>> firmware engineer.
>> Due to the schedule, made this as a short term solution to fix the
>> issue, and wait for the other
>> fix from Intel.
> 
> What is the status for the proper fix?
> 

No updates from Intel.

Aaron

> -Stefan
> 
>>
>> [Test]
>> Verified on hardware, after hotplug the thunderbolt cable,
>> Ethernet works fine.
>>
>> [Where problems could occur]
>> It may break the igc driver.
>>
>> Aaron Ma (1):
>>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
>>
>>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
> 
>
Andrea Righi Sept. 23, 2021, 8:19 a.m. UTC | #11
On Wed, Sep 08, 2021 at 04:49:51PM +0800, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1942999
> 
> SRU justification:
> 
> [Impact]
> igc driver can not connect to network after re-plugin thunderbolt dock
> when
> MAC passthrough enabled in BIOS.
> 
> [Fix]
> Wait for the MAC copy of BIOS when enabled MAC passthrough.
> Intel engineer wants a different solution and promise to discuss with
> firmware engineer.
> Due to the schedule, made this as a short term solution to fix the
> issue, and wait for the other
> fix from Intel.
> 
> [Test]
> Verified on hardware, after hotplug the thunderbolt cable,
> Ethernet works fine.
> 
> [Where problems could occur]
> It may break the igc driver.

Applied to impish/linux 5.13.

Thanks,
-Andrea
Timo Aaltonen Sept. 27, 2021, 1:40 p.m. UTC | #12
On 8.9.2021 11.49, Aaron Ma wrote:
> BugLink: https://bugs.launchpad.net/bugs/1942999
> 
> SRU justification:
> 
> [Impact]
> igc driver can not connect to network after re-plugin thunderbolt dock
> when
> MAC passthrough enabled in BIOS.
> 
> [Fix]
> Wait for the MAC copy of BIOS when enabled MAC passthrough.
> Intel engineer wants a different solution and promise to discuss with
> firmware engineer.
> Due to the schedule, made this as a short term solution to fix the
> issue, and wait for the other
> fix from Intel.
> 
> [Test]
> Verified on hardware, after hotplug the thunderbolt cable,
> Ethernet works fine.
> 
> [Where problems could occur]
> It may break the igc driver.
> 
> Aaron Ma (1):
>    UBUNTU: SAUCE: igc: wait for the MAC copy when enabled MAC passthrough
> 
>   drivers/net/ethernet/intel/igc/igc_main.c | 3 +++
>   1 file changed, 3 insertions(+)
> 

applied to oem-5.13 via rebase

and to oem-5.14 earlier