[PATCH] mmc: atmel-mci: do not assume idle after atmci_request_end
On our AT91SAM9260 board we use the same sdio bus for wifi and for the sd card slot. This caused the atmel-mci to give the following splat on the serial console: [ cut here ] WARNING: CPU: 0 PID: 538 at drivers/mmc/host/atmel-mci.c:859 atmci_send_command+0x24/0x44 Modules linked in: CPU: 0 PID: 538 Comm: mmcqd/0 Not tainted 4.14.76 #14 Hardware name: Atmel AT91SAM9 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (__warn+0xd8/0xf4) [] (__warn) from [] (warn_slowpath_null+0x1c/0x24) [] (warn_slowpath_null) from [] (atmci_send_command+0x24/0x44) [] (atmci_send_command) from [] (atmci_start_request+0x1f4/0x2dc) [] (atmci_start_request) from [] (atmci_request+0xf0/0x164) [] (atmci_request) from [] (mmc_start_request+0x280/0x2d0) [] (mmc_start_request) from [] (mmc_start_areq+0x230/0x330) [] (mmc_start_areq) from [] (mmc_blk_issue_rw_rq+0xc4/0x310) [] (mmc_blk_issue_rw_rq) from [] (mmc_blk_issue_rq+0x118/0x5ac) [] (mmc_blk_issue_rq) from [] (mmc_queue_thread+0xc4/0x118) [] (mmc_queue_thread) from [] (kthread+0x100/0x118) [] (kthread) from [] (ret_from_fork+0x14/0x34) ---[ end trace 594371ddfa284bd6 ]--- This is: WARN_ON(host->cmd); This was fixed on our board by letting atmci_request_end determine what state we are in. Instead of unconditionally setting it to STATE_IDLE on STATE_END_REQUEST. Signed-off-by: Jonas Danielsson --- drivers/mmc/host/atmel-mci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c index 0a0ebf3a0..c8a591d8a 100644 --- a/drivers/mmc/host/atmel-mci.c +++ b/drivers/mmc/host/atmel-mci.c @@ -1954,13 +1954,14 @@ static void atmci_tasklet_func(unsigned long priv) } atmci_request_end(host, host->mrq); - state = STATE_IDLE; + goto unlock; /* atmci_request_end() sets host->state */ break; } } while (state != prev_state); host->state = state; +unlock: spin_unlock(>lock); } -- 2.17.2
[PATCH] mmc: atmel-mci: do not assume idle after atmci_request_end
On our AT91SAM9260 board we use the same sdio bus for wifi and for the sd card slot. This caused the atmel-mci to give the following splat on the serial console: [ cut here ] WARNING: CPU: 0 PID: 538 at drivers/mmc/host/atmel-mci.c:859 atmci_send_command+0x24/0x44 Modules linked in: CPU: 0 PID: 538 Comm: mmcqd/0 Not tainted 4.14.76 #14 Hardware name: Atmel AT91SAM9 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (__warn+0xd8/0xf4) [] (__warn) from [] (warn_slowpath_null+0x1c/0x24) [] (warn_slowpath_null) from [] (atmci_send_command+0x24/0x44) [] (atmci_send_command) from [] (atmci_start_request+0x1f4/0x2dc) [] (atmci_start_request) from [] (atmci_request+0xf0/0x164) [] (atmci_request) from [] (mmc_start_request+0x280/0x2d0) [] (mmc_start_request) from [] (mmc_start_areq+0x230/0x330) [] (mmc_start_areq) from [] (mmc_blk_issue_rw_rq+0xc4/0x310) [] (mmc_blk_issue_rw_rq) from [] (mmc_blk_issue_rq+0x118/0x5ac) [] (mmc_blk_issue_rq) from [] (mmc_queue_thread+0xc4/0x118) [] (mmc_queue_thread) from [] (kthread+0x100/0x118) [] (kthread) from [] (ret_from_fork+0x14/0x34) ---[ end trace 594371ddfa284bd6 ]--- This is: WARN_ON(host->cmd); This was fixed on our board by letting atmci_request_end determine what state we are in. Instead of unconditionally setting it to STATE_IDLE on STATE_END_REQUEST. Signed-off-by: Jonas Danielsson --- drivers/mmc/host/atmel-mci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c index 0a0ebf3a0..c8a591d8a 100644 --- a/drivers/mmc/host/atmel-mci.c +++ b/drivers/mmc/host/atmel-mci.c @@ -1954,13 +1954,14 @@ static void atmci_tasklet_func(unsigned long priv) } atmci_request_end(host, host->mrq); - state = STATE_IDLE; + goto unlock; /* atmci_request_end() sets host->state */ break; } } while (state != prev_state); host->state = state; +unlock: spin_unlock(>lock); } -- 2.17.2
Re: [PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
On Wed, Oct 17, 2018 at 3:10 PM wrote: > > > > > We take the normal path of sys_reboot => kernel_restart => machine_restart > > ... > > > > I added code to print the c1 register in different paths. And I-cache > > is enabled. > > So now I am really confused about why the patch worked. > > Just saying... maybe your instructions add some delay on the execution path > and this is why it helps... try to access cp15 co-processor for read and > write back the value you read without actually to modify it, to see if this > could be the reason: e.g.: > > mrc p15, 0, r0, c1, c0, 0 > orr r1, r1, #4096 // whatever is in r1, doesn't matter > mcr p15, 0, r0, c1, c0, 0 > Yes, this also seems to work. I have over 100 reboots completed with this code. So what could be the issue here? It seem related to the powering down of the sdram at least. This thread on the AT91SAM community deals with the same issue: http://www.at91.com/viewtopic.php?t=25830 There the solution people chose was removing the SDRAM powering down. But that leaves one open to the cause of the errata. Do you have any thought on how to approach this? > Thank you, > Claudiu Beznea > Regards Jonas > > > >> Best regards, > >> Alexander > > > > Jonas > > > >> > >> > >> > > > > -- JONAS DANIELSSON Software Developer +46 72 361 5022 Malmö - Sweden ORBITAL SYSTEMS orbital-systems.com The information contained in this message is intended for the personal and confidential use of the designated recipients named above and may contain confidential and/or privileged material. If the reader of this message is not the intended recipient or an agent responsible for delivering it to the intended recipient, you are hereby notified that you have received this document in error, and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender immediately and delete this e-mail from your system. Although this transmission and any attachments are believed to be free of any virus or other defect that might affect any computer system into which it is received and opened, it is the responsibility of the recipient to ensure that it is virus free and no responsibility is accepted by ORBITAL SYSTEMS AB, its subsidiaries and affiliates, as applicable, for any loss or damage arising in any way.
Re: [PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
On Wed, Oct 17, 2018 at 3:10 PM wrote: > > > > > We take the normal path of sys_reboot => kernel_restart => machine_restart > > ... > > > > I added code to print the c1 register in different paths. And I-cache > > is enabled. > > So now I am really confused about why the patch worked. > > Just saying... maybe your instructions add some delay on the execution path > and this is why it helps... try to access cp15 co-processor for read and > write back the value you read without actually to modify it, to see if this > could be the reason: e.g.: > > mrc p15, 0, r0, c1, c0, 0 > orr r1, r1, #4096 // whatever is in r1, doesn't matter > mcr p15, 0, r0, c1, c0, 0 > Yes, this also seems to work. I have over 100 reboots completed with this code. So what could be the issue here? It seem related to the powering down of the sdram at least. This thread on the AT91SAM community deals with the same issue: http://www.at91.com/viewtopic.php?t=25830 There the solution people chose was removing the SDRAM powering down. But that leaves one open to the cause of the errata. Do you have any thought on how to approach this? > Thank you, > Claudiu Beznea > Regards Jonas > > > >> Best regards, > >> Alexander > > > > Jonas > > > >> > >> > >> > > > > -- JONAS DANIELSSON Software Developer +46 72 361 5022 Malmö - Sweden ORBITAL SYSTEMS orbital-systems.com The information contained in this message is intended for the personal and confidential use of the designated recipients named above and may contain confidential and/or privileged material. If the reader of this message is not the intended recipient or an agent responsible for delivering it to the intended recipient, you are hereby notified that you have received this document in error, and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender immediately and delete this e-mail from your system. Although this transmission and any attachments are believed to be free of any virus or other defect that might affect any computer system into which it is received and opened, it is the responsibility of the recipient to ensure that it is virus free and no responsibility is accepted by ORBITAL SYSTEMS AB, its subsidiaries and affiliates, as applicable, for any loss or damage arising in any way.
Re: [PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
On Tue, Oct 16, 2018 at 4:52 PM Alexander Stein wrote: > > On Tuesday, October 16, 2018, 3:30:24 PM CEST claudiu.bez...@microchip.com > wrote: > > Hi Jonas, > > > > On 07.10.2018 15:57, Jonas Danielsson wrote: > > > From: Jonas Danielsson > > > > > > This fixes a bug where our embedded system (AT91SAM9260 based) would > > > hang at reboot. At the most we managed 16 boot loops without a hang. > > > > > > With this patch applied the problem has not been observed and the board > > > has managed above 250 boot loops. > > > > > > The AT91SAM9260 datasheet tells us that with the instruction cache > > > disabled all instructions are fetched from SDRAM. And we have an errata > > > telling us we must power down the SDRAM before issuing cpu reset. > > > > > > This means we need the instruction cache enabled in at91sam9260_reset() > > > At the moment it is being disabled in cpu_proc_fin() which is called from > > > arch/arm/kernel/reboot.c. > > > > Are you using kexec reboot or implemented hibernate mode on this machine? > > I'm seeing cpu_proc_fin() is called only in case of kexec reboot or > > switching to hibernate mode. > > > > In case of normal reboot (e.g. reboot command) machine_restart() from > > arch/arm/kernel/reboot.c is called. Please correct me if I'm wrong. > > Another location is cpu_reset() aka cpu_arm926_reset() in proc-arm926.S > which also disables I-cache. But I can't track down a callstack > ending there. > We take the normal path of sys_reboot => kernel_restart => machine_restart ... I added code to print the c1 register in different paths. And I-cache is enabled. So now I am really confused about why the patch worked. > Best regards, > Alexander Jonas > > > -- JONAS DANIELSSON Software Developer +46 72 361 5022 Malmö - Sweden ORBITAL SYSTEMS orbital-systems.com The information contained in this message is intended for the personal and confidential use of the designated recipients named above and may contain confidential and/or privileged material. If the reader of this message is not the intended recipient or an agent responsible for delivering it to the intended recipient, you are hereby notified that you have received this document in error, and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender immediately and delete this e-mail from your system. Although this transmission and any attachments are believed to be free of any virus or other defect that might affect any computer system into which it is received and opened, it is the responsibility of the recipient to ensure that it is virus free and no responsibility is accepted by ORBITAL SYSTEMS AB, its subsidiaries and affiliates, as applicable, for any loss or damage arising in any way.
Re: [PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
On Tue, Oct 16, 2018 at 4:52 PM Alexander Stein wrote: > > On Tuesday, October 16, 2018, 3:30:24 PM CEST claudiu.bez...@microchip.com > wrote: > > Hi Jonas, > > > > On 07.10.2018 15:57, Jonas Danielsson wrote: > > > From: Jonas Danielsson > > > > > > This fixes a bug where our embedded system (AT91SAM9260 based) would > > > hang at reboot. At the most we managed 16 boot loops without a hang. > > > > > > With this patch applied the problem has not been observed and the board > > > has managed above 250 boot loops. > > > > > > The AT91SAM9260 datasheet tells us that with the instruction cache > > > disabled all instructions are fetched from SDRAM. And we have an errata > > > telling us we must power down the SDRAM before issuing cpu reset. > > > > > > This means we need the instruction cache enabled in at91sam9260_reset() > > > At the moment it is being disabled in cpu_proc_fin() which is called from > > > arch/arm/kernel/reboot.c. > > > > Are you using kexec reboot or implemented hibernate mode on this machine? > > I'm seeing cpu_proc_fin() is called only in case of kexec reboot or > > switching to hibernate mode. > > > > In case of normal reboot (e.g. reboot command) machine_restart() from > > arch/arm/kernel/reboot.c is called. Please correct me if I'm wrong. > > Another location is cpu_reset() aka cpu_arm926_reset() in proc-arm926.S > which also disables I-cache. But I can't track down a callstack > ending there. > We take the normal path of sys_reboot => kernel_restart => machine_restart ... I added code to print the c1 register in different paths. And I-cache is enabled. So now I am really confused about why the patch worked. > Best regards, > Alexander Jonas > > > -- JONAS DANIELSSON Software Developer +46 72 361 5022 Malmö - Sweden ORBITAL SYSTEMS orbital-systems.com The information contained in this message is intended for the personal and confidential use of the designated recipients named above and may contain confidential and/or privileged material. If the reader of this message is not the intended recipient or an agent responsible for delivering it to the intended recipient, you are hereby notified that you have received this document in error, and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender immediately and delete this e-mail from your system. Although this transmission and any attachments are believed to be free of any virus or other defect that might affect any computer system into which it is received and opened, it is the responsibility of the recipient to ensure that it is virus free and no responsibility is accepted by ORBITAL SYSTEMS AB, its subsidiaries and affiliates, as applicable, for any loss or damage arising in any way.
Re: [PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
On Tue, Oct 16, 2018 at 4:03 PM Alexander Stein
wrote:
>
> Do you have CONFIG_CPU_ICACHE_DISABLE enabled?
> I wonder why I-cache is disabled. I know about this errata, AT91SAM9G20 is
> affected as well.
>
Hi Alexander!
I just checked, we do not have CONFIG_CPU_ICACHE_DISABLE enabled.
I wonder as well! If you have any idea I can try them out on my board tomorrow!
The effect of enabling them is real. I have managed over 1000 reboots
with this patch.
And at the most 20 without.
> Best regards,
> Alexander
Jonas
>
> On Sunday, October 7, 2018, 2:57:45 PM CEST Jonas Danielsson wrote:
> > From: Jonas Danielsson
> >
> > This fixes a bug where our embedded system (AT91SAM9260 based) would
> > hang at reboot. At the most we managed 16 boot loops without a hang.
> >
> > With this patch applied the problem has not been observed and the board
> > has managed above 250 boot loops.
> >
> > The AT91SAM9260 datasheet tells us that with the instruction cache
> > disabled all instructions are fetched from SDRAM. And we have an errata
> > telling us we must power down the SDRAM before issuing cpu reset.
> >
> > This means we need the instruction cache enabled in at91sam9260_reset()
> > At the moment it is being disabled in cpu_proc_fin() which is called from
> > arch/arm/kernel/reboot.c.
> >
> > Signed-off-by: Jonas Danielsson
> > ---
> > drivers/power/reset/at91-reset.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/power/reset/at91-reset.c
> > b/drivers/power/reset/at91-reset.c
> > index f44a9ffcc2ab..78972bba64df 100644
> > --- a/drivers/power/reset/at91-reset.c
> > +++ b/drivers/power/reset/at91-reset.c
> > @@ -50,14 +50,24 @@ static void __iomem *at91_ramc_base[2], *at91_rstc_base;
> > static struct clk *sclk;
> >
> > /*
> > -* unless the SDRAM is cleanly shutdown before we hit the
> > +* Errata 43.1.7.1 RSTC: Reset during SDRAM Accesses
> > +*
> > +* Unless the SDRAM is cleanly shutdown before we hit the
> > * reset register it can be left driving the data bus and
> > * killing the chance of a subsequent boot from NAND
> > +*
> > +* Since we are disabling SDRAM need to make sure that the
> > +* instruction cache is enabled.
> > */
> > static int at91sam9260_restart(struct notifier_block *this, unsigned long
> > mode,
> > void *cmd)
> > {
> > asm volatile(
> > + /* Enable I-cache */
> > + "mrcp15, 0, r0, c1, c0, 0\n\t"
> > + "orrr0, r0, #4096\n\t" /* CR_I (bit 12) */
> > + "mcrp15, 0, r0, c1, c0, 0\n\t"
> > +
> > /* Align to cache lines */
> > ".balign 32\n\t"
> >
> >
>
>
> --
> SYS TEC electronic GmbH
> Am Windrad 2
> 08468 Heinsdorfergrund
> Germany
> Telefon : +49 (0) 3765 38600-0
> Fax : +49 (0) 3765 38600-4100
> Email : alexander.st...@systec-electronic.com
> Website : http://www.systec-electronic.com
>
> Managing Director : Dipl.-Phys. Siegmar Schmidt
> Commercial registry : Amtsgericht Chemnitz, HRB 28082
> USt.-Id Nr. : DE150534010
>
> Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen.
> Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich
> erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie
> diese Mail.
> Das unerlaubte Kopieren sowie die unbefugte Weitergabe dieser Mail sind nicht
> gestattet.
>
> This e-mail may contain confidential and/or privileged information.
> If you are not the intended recipient (or have received this e-mail in error)
> please notify the sender immediately and destroy this e-mail.
> Any unauthorized copying, disclosure or distribution of the material in this
> e-mail is strictly forbidden.
>
>
--
JONAS DANIELSSON
Software Developer
+46 72 361 5022
Malmö - Sweden
ORBITAL SYSTEMS
orbital-systems.com
The information contained in this message is intended for the personal
and confidential use of the designated recipients named above and may
contain confidential and/or privileged material. If the reader of this
message is not the intended recipient or an agent responsible for
delivering it to the intended recipient, you are hereby notified that
you have received this document in error, and that any review,
dissemination, distribution, or copying of this message is strictly
prohibited. If you have received this communication in error, please
notify the sender immediately and delete this e-mail from your system.
Although this transmission and any attachments are believed to be free
of any virus or other defect that might affect any computer system
into which it is received and opened, it is the responsibility of the
recipient to ensure that it is virus free and no responsibility is
accepted by ORBITAL SYSTEMS AB, its subsidiaries and affiliates, as
applicable, for any loss or damage arising in any way.
Re: [PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
On Tue, Oct 16, 2018 at 4:03 PM Alexander Stein
wrote:
>
> Do you have CONFIG_CPU_ICACHE_DISABLE enabled?
> I wonder why I-cache is disabled. I know about this errata, AT91SAM9G20 is
> affected as well.
>
Hi Alexander!
I just checked, we do not have CONFIG_CPU_ICACHE_DISABLE enabled.
I wonder as well! If you have any idea I can try them out on my board tomorrow!
The effect of enabling them is real. I have managed over 1000 reboots
with this patch.
And at the most 20 without.
> Best regards,
> Alexander
Jonas
>
> On Sunday, October 7, 2018, 2:57:45 PM CEST Jonas Danielsson wrote:
> > From: Jonas Danielsson
> >
> > This fixes a bug where our embedded system (AT91SAM9260 based) would
> > hang at reboot. At the most we managed 16 boot loops without a hang.
> >
> > With this patch applied the problem has not been observed and the board
> > has managed above 250 boot loops.
> >
> > The AT91SAM9260 datasheet tells us that with the instruction cache
> > disabled all instructions are fetched from SDRAM. And we have an errata
> > telling us we must power down the SDRAM before issuing cpu reset.
> >
> > This means we need the instruction cache enabled in at91sam9260_reset()
> > At the moment it is being disabled in cpu_proc_fin() which is called from
> > arch/arm/kernel/reboot.c.
> >
> > Signed-off-by: Jonas Danielsson
> > ---
> > drivers/power/reset/at91-reset.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/power/reset/at91-reset.c
> > b/drivers/power/reset/at91-reset.c
> > index f44a9ffcc2ab..78972bba64df 100644
> > --- a/drivers/power/reset/at91-reset.c
> > +++ b/drivers/power/reset/at91-reset.c
> > @@ -50,14 +50,24 @@ static void __iomem *at91_ramc_base[2], *at91_rstc_base;
> > static struct clk *sclk;
> >
> > /*
> > -* unless the SDRAM is cleanly shutdown before we hit the
> > +* Errata 43.1.7.1 RSTC: Reset during SDRAM Accesses
> > +*
> > +* Unless the SDRAM is cleanly shutdown before we hit the
> > * reset register it can be left driving the data bus and
> > * killing the chance of a subsequent boot from NAND
> > +*
> > +* Since we are disabling SDRAM need to make sure that the
> > +* instruction cache is enabled.
> > */
> > static int at91sam9260_restart(struct notifier_block *this, unsigned long
> > mode,
> > void *cmd)
> > {
> > asm volatile(
> > + /* Enable I-cache */
> > + "mrcp15, 0, r0, c1, c0, 0\n\t"
> > + "orrr0, r0, #4096\n\t" /* CR_I (bit 12) */
> > + "mcrp15, 0, r0, c1, c0, 0\n\t"
> > +
> > /* Align to cache lines */
> > ".balign 32\n\t"
> >
> >
>
>
> --
> SYS TEC electronic GmbH
> Am Windrad 2
> 08468 Heinsdorfergrund
> Germany
> Telefon : +49 (0) 3765 38600-0
> Fax : +49 (0) 3765 38600-4100
> Email : alexander.st...@systec-electronic.com
> Website : http://www.systec-electronic.com
>
> Managing Director : Dipl.-Phys. Siegmar Schmidt
> Commercial registry : Amtsgericht Chemnitz, HRB 28082
> USt.-Id Nr. : DE150534010
>
> Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen.
> Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich
> erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie
> diese Mail.
> Das unerlaubte Kopieren sowie die unbefugte Weitergabe dieser Mail sind nicht
> gestattet.
>
> This e-mail may contain confidential and/or privileged information.
> If you are not the intended recipient (or have received this e-mail in error)
> please notify the sender immediately and destroy this e-mail.
> Any unauthorized copying, disclosure or distribution of the material in this
> e-mail is strictly forbidden.
>
>
--
JONAS DANIELSSON
Software Developer
+46 72 361 5022
Malmö - Sweden
ORBITAL SYSTEMS
orbital-systems.com
The information contained in this message is intended for the personal
and confidential use of the designated recipients named above and may
contain confidential and/or privileged material. If the reader of this
message is not the intended recipient or an agent responsible for
delivering it to the intended recipient, you are hereby notified that
you have received this document in error, and that any review,
dissemination, distribution, or copying of this message is strictly
prohibited. If you have received this communication in error, please
notify the sender immediately and delete this e-mail from your system.
Although this transmission and any attachments are believed to be free
of any virus or other defect that might affect any computer system
into which it is received and opened, it is the responsibility of the
recipient to ensure that it is virus free and no responsibility is
accepted by ORBITAL SYSTEMS AB, its subsidiaries and affiliates, as
applicable, for any loss or damage arising in any way.
Re: [PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
On Tue, Oct 16, 2018 at 3:30 PM wrote:
>
> Hi Jonas,
>
Hi,
> On 07.10.2018 15:57, Jonas Danielsson wrote:
> > From: Jonas Danielsson
> >
> > This fixes a bug where our embedded system (AT91SAM9260 based) would
> > hang at reboot. At the most we managed 16 boot loops without a hang.
> >
> > With this patch applied the problem has not been observed and the board
> > has managed above 250 boot loops.
> >
> > The AT91SAM9260 datasheet tells us that with the instruction cache
> > disabled all instructions are fetched from SDRAM. And we have an errata
> > telling us we must power down the SDRAM before issuing cpu reset.
> >
> > This means we need the instruction cache enabled in at91sam9260_reset()
> > At the moment it is being disabled in cpu_proc_fin() which is called from
> > arch/arm/kernel/reboot.c.
>
> Are you using kexec reboot or implemented hibernate mode on this machine?
> I'm seeing cpu_proc_fin() is called only in case of kexec reboot or
> switching to hibernate mode.
>
> In case of normal reboot (e.g. reboot command) machine_restart() from
> arch/arm/kernel/reboot.c is called. Please correct me if I'm wrong.
>
We are not, we do regular reboots. I have read the code paths wrong.
Then I wonder what disables icache.
> Thank you,
> Claudiu Beznea
Thank you!
>
> >
> > Signed-off-by: Jonas Danielsson
> > ---
> > drivers/power/reset/at91-reset.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/power/reset/at91-reset.c
> > b/drivers/power/reset/at91-reset.c
> > index f44a9ffcc2ab..78972bba64df 100644
> > --- a/drivers/power/reset/at91-reset.c
> > +++ b/drivers/power/reset/at91-reset.c
> > @@ -50,14 +50,24 @@ static void __iomem *at91_ramc_base[2], *at91_rstc_base;
> > static struct clk *sclk;
> >
> > /*
> > -* unless the SDRAM is cleanly shutdown before we hit the
> > +* Errata 43.1.7.1 RSTC: Reset during SDRAM Accesses
> > +*
> > +* Unless the SDRAM is cleanly shutdown before we hit the
> > * reset register it can be left driving the data bus and
> > * killing the chance of a subsequent boot from NAND
> > +*
> > +* Since we are disabling SDRAM need to make sure that the
> > +* instruction cache is enabled.
> > */
> > static int at91sam9260_restart(struct notifier_block *this, unsigned long
> > mode,
> > void *cmd)
> > {
> > asm volatile(
> > + /* Enable I-cache */
> > + "mrcp15, 0, r0, c1, c0, 0\n\t"
> > + "orrr0, r0, #4096\n\t" /* CR_I (bit 12) */
> > + "mcrp15, 0, r0, c1, c0, 0\n\t"
> > +
> > /* Align to cache lines */
> > ".balign 32\n\t"
> >
> >
--
JONAS DANIELSSON
Software Developer
+46 72 361 5022
Malmö - Sweden
ORBITAL SYSTEMS
orbital-systems.com
The information contained in this message is intended for the personal
and confidential use of the designated recipients named above and may
contain confidential and/or privileged material. If the reader of this
message is not the intended recipient or an agent responsible for
delivering it to the intended recipient, you are hereby notified that
you have received this document in error, and that any review,
dissemination, distribution, or copying of this message is strictly
prohibited. If you have received this communication in error, please
notify the sender immediately and delete this e-mail from your system.
Although this transmission and any attachments are believed to be free
of any virus or other defect that might affect any computer system
into which it is received and opened, it is the responsibility of the
recipient to ensure that it is virus free and no responsibility is
accepted by ORBITAL SYSTEMS AB, its subsidiaries and affiliates, as
applicable, for any loss or damage arising in any way.
Re: [PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
On Tue, Oct 16, 2018 at 3:30 PM wrote:
>
> Hi Jonas,
>
Hi,
> On 07.10.2018 15:57, Jonas Danielsson wrote:
> > From: Jonas Danielsson
> >
> > This fixes a bug where our embedded system (AT91SAM9260 based) would
> > hang at reboot. At the most we managed 16 boot loops without a hang.
> >
> > With this patch applied the problem has not been observed and the board
> > has managed above 250 boot loops.
> >
> > The AT91SAM9260 datasheet tells us that with the instruction cache
> > disabled all instructions are fetched from SDRAM. And we have an errata
> > telling us we must power down the SDRAM before issuing cpu reset.
> >
> > This means we need the instruction cache enabled in at91sam9260_reset()
> > At the moment it is being disabled in cpu_proc_fin() which is called from
> > arch/arm/kernel/reboot.c.
>
> Are you using kexec reboot or implemented hibernate mode on this machine?
> I'm seeing cpu_proc_fin() is called only in case of kexec reboot or
> switching to hibernate mode.
>
> In case of normal reboot (e.g. reboot command) machine_restart() from
> arch/arm/kernel/reboot.c is called. Please correct me if I'm wrong.
>
We are not, we do regular reboots. I have read the code paths wrong.
Then I wonder what disables icache.
> Thank you,
> Claudiu Beznea
Thank you!
>
> >
> > Signed-off-by: Jonas Danielsson
> > ---
> > drivers/power/reset/at91-reset.c | 12 +++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/power/reset/at91-reset.c
> > b/drivers/power/reset/at91-reset.c
> > index f44a9ffcc2ab..78972bba64df 100644
> > --- a/drivers/power/reset/at91-reset.c
> > +++ b/drivers/power/reset/at91-reset.c
> > @@ -50,14 +50,24 @@ static void __iomem *at91_ramc_base[2], *at91_rstc_base;
> > static struct clk *sclk;
> >
> > /*
> > -* unless the SDRAM is cleanly shutdown before we hit the
> > +* Errata 43.1.7.1 RSTC: Reset during SDRAM Accesses
> > +*
> > +* Unless the SDRAM is cleanly shutdown before we hit the
> > * reset register it can be left driving the data bus and
> > * killing the chance of a subsequent boot from NAND
> > +*
> > +* Since we are disabling SDRAM need to make sure that the
> > +* instruction cache is enabled.
> > */
> > static int at91sam9260_restart(struct notifier_block *this, unsigned long
> > mode,
> > void *cmd)
> > {
> > asm volatile(
> > + /* Enable I-cache */
> > + "mrcp15, 0, r0, c1, c0, 0\n\t"
> > + "orrr0, r0, #4096\n\t" /* CR_I (bit 12) */
> > + "mcrp15, 0, r0, c1, c0, 0\n\t"
> > +
> > /* Align to cache lines */
> > ".balign 32\n\t"
> >
> >
--
JONAS DANIELSSON
Software Developer
+46 72 361 5022
Malmö - Sweden
ORBITAL SYSTEMS
orbital-systems.com
The information contained in this message is intended for the personal
and confidential use of the designated recipients named above and may
contain confidential and/or privileged material. If the reader of this
message is not the intended recipient or an agent responsible for
delivering it to the intended recipient, you are hereby notified that
you have received this document in error, and that any review,
dissemination, distribution, or copying of this message is strictly
prohibited. If you have received this communication in error, please
notify the sender immediately and delete this e-mail from your system.
Although this transmission and any attachments are believed to be free
of any virus or other defect that might affect any computer system
into which it is received and opened, it is the responsibility of the
recipient to ensure that it is virus free and no responsibility is
accepted by ORBITAL SYSTEMS AB, its subsidiaries and affiliates, as
applicable, for any loss or damage arising in any way.
[PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
From: Jonas Danielsson
This fixes a bug where our embedded system (AT91SAM9260 based) would
hang at reboot. At the most we managed 16 boot loops without a hang.
With this patch applied the problem has not been observed and the board
has managed above 250 boot loops.
The AT91SAM9260 datasheet tells us that with the instruction cache
disabled all instructions are fetched from SDRAM. And we have an errata
telling us we must power down the SDRAM before issuing cpu reset.
This means we need the instruction cache enabled in at91sam9260_reset()
At the moment it is being disabled in cpu_proc_fin() which is called from
arch/arm/kernel/reboot.c.
Signed-off-by: Jonas Danielsson
---
drivers/power/reset/at91-reset.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index f44a9ffcc2ab..78972bba64df 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -50,14 +50,24 @@ static void __iomem *at91_ramc_base[2], *at91_rstc_base;
static struct clk *sclk;
/*
-* unless the SDRAM is cleanly shutdown before we hit the
+* Errata 43.1.7.1 RSTC: Reset during SDRAM Accesses
+*
+* Unless the SDRAM is cleanly shutdown before we hit the
* reset register it can be left driving the data bus and
* killing the chance of a subsequent boot from NAND
+*
+* Since we are disabling SDRAM need to make sure that the
+* instruction cache is enabled.
*/
static int at91sam9260_restart(struct notifier_block *this, unsigned long mode,
void *cmd)
{
asm volatile(
+ /* Enable I-cache */
+ "mrcp15, 0, r0, c1, c0, 0\n\t"
+ "orrr0, r0, #4096\n\t" /* CR_I (bit 12) */
+ "mcrp15, 0, r0, c1, c0, 0\n\t"
+
/* Align to cache lines */
".balign 32\n\t"
--
2.14.4
[PATCH] power: reset: at91-reset: enable I-cache for at91sam9260_reset
From: Jonas Danielsson
This fixes a bug where our embedded system (AT91SAM9260 based) would
hang at reboot. At the most we managed 16 boot loops without a hang.
With this patch applied the problem has not been observed and the board
has managed above 250 boot loops.
The AT91SAM9260 datasheet tells us that with the instruction cache
disabled all instructions are fetched from SDRAM. And we have an errata
telling us we must power down the SDRAM before issuing cpu reset.
This means we need the instruction cache enabled in at91sam9260_reset()
At the moment it is being disabled in cpu_proc_fin() which is called from
arch/arm/kernel/reboot.c.
Signed-off-by: Jonas Danielsson
---
drivers/power/reset/at91-reset.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c
index f44a9ffcc2ab..78972bba64df 100644
--- a/drivers/power/reset/at91-reset.c
+++ b/drivers/power/reset/at91-reset.c
@@ -50,14 +50,24 @@ static void __iomem *at91_ramc_base[2], *at91_rstc_base;
static struct clk *sclk;
/*
-* unless the SDRAM is cleanly shutdown before we hit the
+* Errata 43.1.7.1 RSTC: Reset during SDRAM Accesses
+*
+* Unless the SDRAM is cleanly shutdown before we hit the
* reset register it can be left driving the data bus and
* killing the chance of a subsequent boot from NAND
+*
+* Since we are disabling SDRAM need to make sure that the
+* instruction cache is enabled.
*/
static int at91sam9260_restart(struct notifier_block *this, unsigned long mode,
void *cmd)
{
asm volatile(
+ /* Enable I-cache */
+ "mrcp15, 0, r0, c1, c0, 0\n\t"
+ "orrr0, r0, #4096\n\t" /* CR_I (bit 12) */
+ "mcrp15, 0, r0, c1, c0, 0\n\t"
+
/* Align to cache lines */
".balign 32\n\t"
--
2.14.4
Regression introduced by commit dbe9a4173ea53
Hi,
I have seen a regression, that I think is caused by:
commit dbe9a4173ea53b72b2c35d19f676a85b69f1c9fe
Author: Eric W. Biederman
Date: Thu Sep 6 18:20:01 2012 +
scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie.
With this commit the value send as uid when credentials are missing
changes from -1 to overflowuid (default -2, the 'nobody' user).
I was using dbus-send to perform a method call on a gdbus server.
And sometimes I fall victim to the race condition caused by:
commit 16e5726 ("af_unix: dont send SCM_CREDENTIALS by default")
And then there will be no credentials on the socket. The glib library checks
the uid, and if it is -1, it will fall back to using SO_PEERCRED and things
will work for me. But since the commit in $subject changes the uid I get
failure with NoReply from dbus-send.
It seems that before the commit in $subject that the function from_kuid_munged
was only called if there were credentials present. Otherwise we would set uid
to -1. Now uid gets set to -1 if there is no credentials but from_kuid_munged
is always called. And from the documentation of from_kuid_munged it states
that it 'never fails and always returns a valid uid'. And in the case of
uid being -1, it returns overflowuid.
So this seems like it broke glib. Caused by 1) the introduced race condition
that makes it not totally safe to set SO_PEERCRED on the accepted socket and
2) that the value of uid when credentials are missing changed from -1 to
overflowuid.
Thanks for your time
Jonas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Regression introduced by commit dbe9a4173ea53
Hi, I have seen a regression, that I think is caused by: commit dbe9a4173ea53b72b2c35d19f676a85b69f1c9fe Author: Eric W. Biederman ebied...@xmission.com Date: Thu Sep 6 18:20:01 2012 + scm: Don't use struct ucred in NETLINK_CB and struct scm_cookie. With this commit the value send as uid when credentials are missing changes from -1 to overflowuid (default -2, the 'nobody' user). I was using dbus-send to perform a method call on a gdbus server. And sometimes I fall victim to the race condition caused by: commit 16e5726 (af_unix: dont send SCM_CREDENTIALS by default) And then there will be no credentials on the socket. The glib library checks the uid, and if it is -1, it will fall back to using SO_PEERCRED and things will work for me. But since the commit in $subject changes the uid I get failure with NoReply from dbus-send. It seems that before the commit in $subject that the function from_kuid_munged was only called if there were credentials present. Otherwise we would set uid to -1. Now uid gets set to -1 if there is no credentials but from_kuid_munged is always called. And from the documentation of from_kuid_munged it states that it 'never fails and always returns a valid uid'. And in the case of uid being -1, it returns overflowuid. So this seems like it broke glib. Caused by 1) the introduced race condition that makes it not totally safe to set SO_PEERCRED on the accepted socket and 2) that the value of uid when credentials are missing changed from -1 to overflowuid. Thanks for your time Jonas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH][v2] net/ipv4/arp.c: Fix arp reply when sender ip 0
Fix arp reply when received arp probe with sender ip 0. Send arp reply with target ip address 0.0.0.0 and target hardware address set to hardware address of requester. Previously sent reply with target ip address and target hardware address set to same as source fields. Signed-off-by: Jonas Danielsson <[EMAIL PROTECTED]> Acked-by: Alexey Kuznetov <[EMAIL PROTECTED]> --- arp.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -828,7 +828,8 @@ static int arp_process(struct sk_buff *skb) if (arp->ar_op == htons(ARPOP_REQUEST) && inet_addr_type(tip) == RTN_LOCAL && !arp_ignore(in_dev,dev,sip,tip)) - arp_send(ARPOP_REPLY,ETH_P_ARP,tip,dev,tip,sha,dev->dev_addr,dev->dev_addr); + arp_send(ARPOP_REPLY, ETH_P_ARP, sip, dev, tip, sha, +dev->dev_addr, sha); goto out; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH][v2] net/ipv4/arp.c: Fix arp reply when sender ip 0
Fix arp reply when received arp probe with sender ip 0. Send arp reply with target ip address 0.0.0.0 and target hardware address set to hardware address of requester. Previously sent reply with target ip address and target hardware address set to same as source fields. Signed-off-by: Jonas Danielsson [EMAIL PROTECTED] Acked-by: Alexey Kuznetov [EMAIL PROTECTED] --- arp.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -828,7 +828,8 @@ static int arp_process(struct sk_buff *skb) if (arp-ar_op == htons(ARPOP_REQUEST) inet_addr_type(tip) == RTN_LOCAL !arp_ignore(in_dev,dev,sip,tip)) - arp_send(ARPOP_REPLY,ETH_P_ARP,tip,dev,tip,sha,dev-dev_addr,dev-dev_addr); + arp_send(ARPOP_REPLY, ETH_P_ARP, sip, dev, tip, sha, +dev-dev_addr, sha); goto out; } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net/ipv4/arp.c: Fix arp reply when sender ip 0
2007/11/16, David Miller <[EMAIL PROTECTED]>: > From: "Jonas Danielsson" <[EMAIL PROTECTED]> > Date: Thu, 15 Nov 2007 22:40:13 +0100 > > > Is there a reason that the target hardware address isn't the target > > hardware address? > > Because of this, in cases where a choice can be made Linux will > advertise what is most likely to result in successful communication. > > This is likely why we are changing that target address to the one of > the interface actually sending back the reply rather than the zero > value you used. > > In fact I think this information can be useful to the sender of > the DAD request. > There seem to be some confusion about what my patch really does. It does not set the hardware address to a zero value. An example to illustrate: We have two computers. Computer A IP: 192.168.0.1 HW: 00:AA:00:AA:00:AA Computer B IP: None HW: 00:BB:00:BB:00:BB Computer B want to find out if IP 192.168.0.1 is free for use. It sends an arp-request. Request: Opcode: request (0x0001) Sender HW: 00:BB:00:BB:00:BB Sender IP:0.0.0.0 Target HW: 00:00:00:00:00:00 Target IP: 192.168.0.1 The reply from the Linux kernel in computer A, before the patch would look like: Reply: Opcode: reply (0x0002) Sender HW: 00:AA.00:AA:00:AA Sender IP: 192.168.0.1 Target HW: 00:AA:00:AA:00:AA Target IP:192.168.0.1 And the reply from the Linux kernel, in computer A, after the patch would look like: Reply: Opcode: reply (0x0002) Sender HW: 00:AA.00:AA:00:AA Sender IP: 192.168.0.1 Target HW: 00:BB:00:BB:00:BB Target IP:0.0.0.0 It is the fact that the Target HW address is set to computer A and not computer B that my patch wants to fix. And the Target IP: 0.0.0.0 is because OpenBSD and windows replied in that way. I wanted to change this because, among other things, dhcpcd-2.0.3 has the following code in its arpCheck-function: if ( memcmp(ArpMsgRecv.tHaddr,ClientHwAddr,ETH_ALEN) ) if ( DebugFlag ) syslog(LOG_DEBUG, "target hardware address mismatch:" [...] This check will always fail when replies come from Linux machines, since the target hardware address will not match the client mac address. -Jonas - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net/ipv4/arp.c: Fix arp reply when sender ip 0
2007/11/16, David Miller [EMAIL PROTECTED]: From: Jonas Danielsson [EMAIL PROTECTED] Date: Thu, 15 Nov 2007 22:40:13 +0100 Is there a reason that the target hardware address isn't the target hardware address? Because of this, in cases where a choice can be made Linux will advertise what is most likely to result in successful communication. This is likely why we are changing that target address to the one of the interface actually sending back the reply rather than the zero value you used. In fact I think this information can be useful to the sender of the DAD request. There seem to be some confusion about what my patch really does. It does not set the hardware address to a zero value. An example to illustrate: We have two computers. Computer A IP: 192.168.0.1 HW: 00:AA:00:AA:00:AA Computer B IP: None HW: 00:BB:00:BB:00:BB Computer B want to find out if IP 192.168.0.1 is free for use. It sends an arp-request. Request: Opcode: request (0x0001) Sender HW: 00:BB:00:BB:00:BB Sender IP:0.0.0.0 Target HW: 00:00:00:00:00:00 Target IP: 192.168.0.1 The reply from the Linux kernel in computer A, before the patch would look like: Reply: Opcode: reply (0x0002) Sender HW: 00:AA.00:AA:00:AA Sender IP: 192.168.0.1 Target HW: 00:AA:00:AA:00:AA Target IP:192.168.0.1 And the reply from the Linux kernel, in computer A, after the patch would look like: Reply: Opcode: reply (0x0002) Sender HW: 00:AA.00:AA:00:AA Sender IP: 192.168.0.1 Target HW: 00:BB:00:BB:00:BB Target IP:0.0.0.0 It is the fact that the Target HW address is set to computer A and not computer B that my patch wants to fix. And the Target IP: 0.0.0.0 is because OpenBSD and windows replied in that way. I wanted to change this because, among other things, dhcpcd-2.0.3 has the following code in its arpCheck-function: if ( memcmp(ArpMsgRecv.tHaddr,ClientHwAddr,ETH_ALEN) ) if ( DebugFlag ) syslog(LOG_DEBUG, target hardware address mismatch: [...] This check will always fail when replies come from Linux machines, since the target hardware address will not match the client mac address. -Jonas - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net/ipv4/arp.c: Fix arp reply when sender ip 0 (was: Strange behavior in arp probe reply, bug or feature?)
Hi, I started to look at this code when I was working on a project of rewriting a dhcp-client. I wanted to make the client use arp to determine if the offered address was free or in use. Thats when I noticed that linux machines responded in this, for me, odd way. The problem is not really the target ip address in the reply, it is the fact that the target hardware address is set to the hardware address of the machines that is sending the reply. The target hardware address should be the same as the destination address in the ethernet frame. The dhcp clients I examined, and the implementation of the arpcheck that I use will compare the target hardware field of the arp-reply and match it against its own mac, to verify the reply. And this fails with the current implementation in the kernel. As for the the target ip set to 0, that is the behavior I saw in Windows and OpenBSD machines and figured it was a valid approach. The main thing is however that the target machine address in the arp reply in this case will confuse dhcp-clients trying to verify the reply. And even if your arping implementation will work with any variant, other implementation of this approach of duplicate ip detection expects a differeant behavior. Is there a reason that the target hardware address isn't the target hardware address? -Jonas 2007/11/15, Alexey Kuznetsov <[EMAIL PROTECTED]>: > Hello! > > > Send a correct arp reply instead of one with sender ip and sender > > hardware adress in target fields. > > I do not see anything more legal in setting target address to 0. > > > Actually, semantics of target address in ARP reply is ambiguous. > If it is a reply to some real request, it is set to address of requestor > and protocol requires recipient of this arp reply to test that the address > matches its own address before creating new entry triggered by unsolicited > arp reply. That's all. > > In the case of duplicate address detection, requestor does not have > any address, so that it is absolutely not essential what we use as target > address. The only place, which could depend on this is the tool, which > tests for duplicate address. At least, arping written by me, should > work with any variant. > > So, please, could you explain what did force you to think that use of 0 > is better? > > Alexey > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] net/ipv4/arp.c: Fix arp reply when sender ip 0 (was: Strange behavior in arp probe reply, bug or feature?)
Fix arp reply when received arp probe with sender ip 0. Can't find any ground in RFC2131 to send a non-valid arp-reply in the special case of sender ip being set to 0. - Bug fix for arp handling when sender ip is set to 0. Send a correct arp reply instead of one with sender ip and sender hardware adress in target fields. Now sends target ip and target hw as received in arp probe. Signed-off-by: Jonas Danielsson <[EMAIL PROTECTED]> --- arp.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: arp.c === RCS file: /usr/local/cvs/linux/os/linux-2.6/net/ipv4/arp.c,v retrieving revision 1.22 diff -u -w -r1.22 arp.c --- arp.c 13 Oct 2006 12:45:47 - 1.22 +++ arp.c 15 Nov 2007 10:34:44 - @@ -827,7 +827,8 @@ if (arp->ar_op == htons(ARPOP_REQUEST) && inet_addr_type(tip) == RTN_LOCAL && !arp_ignore(in_dev,dev,sip,tip)) - arp_send(ARPOP_REPLY,ETH_P_ARP,tip,dev,tip,sha,dev->dev_addr,dev->dev_addr); + arp_send(ARPOP_REPLY, ETH_P_ARP, sip, dev, tip, sha, +dev->dev_addr, sha); goto out; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] net/ipv4/arp.c: Fix arp reply when sender ip 0 (was: Strange behavior in arp probe reply, bug or feature?)
Fix arp reply when received arp probe with sender ip 0. Can't find any ground in RFC2131 to send a non-valid arp-reply in the special case of sender ip being set to 0. - Bug fix for arp handling when sender ip is set to 0. Send a correct arp reply instead of one with sender ip and sender hardware adress in target fields. Now sends target ip and target hw as received in arp probe. Signed-off-by: Jonas Danielsson [EMAIL PROTECTED] --- arp.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: arp.c === RCS file: /usr/local/cvs/linux/os/linux-2.6/net/ipv4/arp.c,v retrieving revision 1.22 diff -u -w -r1.22 arp.c --- arp.c 13 Oct 2006 12:45:47 - 1.22 +++ arp.c 15 Nov 2007 10:34:44 - @@ -827,7 +827,8 @@ if (arp-ar_op == htons(ARPOP_REQUEST) inet_addr_type(tip) == RTN_LOCAL !arp_ignore(in_dev,dev,sip,tip)) - arp_send(ARPOP_REPLY,ETH_P_ARP,tip,dev,tip,sha,dev-dev_addr,dev-dev_addr); + arp_send(ARPOP_REPLY, ETH_P_ARP, sip, dev, tip, sha, +dev-dev_addr, sha); goto out; } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net/ipv4/arp.c: Fix arp reply when sender ip 0 (was: Strange behavior in arp probe reply, bug or feature?)
Hi, I started to look at this code when I was working on a project of rewriting a dhcp-client. I wanted to make the client use arp to determine if the offered address was free or in use. Thats when I noticed that linux machines responded in this, for me, odd way. The problem is not really the target ip address in the reply, it is the fact that the target hardware address is set to the hardware address of the machines that is sending the reply. The target hardware address should be the same as the destination address in the ethernet frame. The dhcp clients I examined, and the implementation of the arpcheck that I use will compare the target hardware field of the arp-reply and match it against its own mac, to verify the reply. And this fails with the current implementation in the kernel. As for the the target ip set to 0, that is the behavior I saw in Windows and OpenBSD machines and figured it was a valid approach. The main thing is however that the target machine address in the arp reply in this case will confuse dhcp-clients trying to verify the reply. And even if your arping implementation will work with any variant, other implementation of this approach of duplicate ip detection expects a differeant behavior. Is there a reason that the target hardware address isn't the target hardware address? -Jonas 2007/11/15, Alexey Kuznetsov [EMAIL PROTECTED]: Hello! Send a correct arp reply instead of one with sender ip and sender hardware adress in target fields. I do not see anything more legal in setting target address to 0. Actually, semantics of target address in ARP reply is ambiguous. If it is a reply to some real request, it is set to address of requestor and protocol requires recipient of this arp reply to test that the address matches its own address before creating new entry triggered by unsolicited arp reply. That's all. In the case of duplicate address detection, requestor does not have any address, so that it is absolutely not essential what we use as target address. The only place, which could depend on this is the tool, which tests for duplicate address. At least, arping written by me, should work with any variant. So, please, could you explain what did force you to think that use of 0 is better? Alexey - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Strange behavior in arp probe reply, bug or feature?
Hi,
I was working on a project that involved having our dhcp-client
performing a check on the offered IP-address to make sure it wasn't in
use.
The standard way of doing this is sending out an arp request with the
sender ip set to 0. (RFC2131).
When I was testing out my implemented solution I noticed that my linux
machines responded a bit odd to this arp-request.
The arp-reply had the sender-ip field set to the same as the target-ip
field and the target-hwaddr field the same as the sender-hwaddr field.
All the values where the addresses of the machine replying to the request.
This is not a standard arp way of replying.
I checked the code in net/ipv4/arp.c and found this:
/* Special case: IPv4 duplicate address detection packet (RFC2131) */
if (sip == 0) {
if (arp->ar_op == __constant_htons(ARPOP_REQUEST) &&
inet_addr_type(tip) == RTN_LOCAL)
arp_send(ARPOP_REPLY,ETH_P_ARP
,tip,dev,tip,sha,dev->dev_addr,dev->dev_addr);
goto out;
}
This code will implement the strange behavior mentioned above.
I checked the arp-request against a box running Windows XP and one
running OpenBSD, none of them had this strange beahvior, but rather
replied in a way that
I expected to see, with the requesting machines mac in the target
hw-addr field and 0.0.0.0 in the target ip field.
The linux boxes I tested against ran the 2.16.18 and the 2.16.22
kernel and the code above is still used in the 2.16.23-1 kernel.
I changed the code above on one of them to have the arp_send called in
a "normal" way:
arp_send(ARPOP_REPLY,ETH_P_ARP,sip,dev,tip,sha,dev->dev_addr,sha);
After the change the replies looked like the ones I was receiving from
the OpenBSD and Windows-box.
The comment above the code mention the RFC2131, but there is no
mention, that I can see to this construction of the reply. I can only
see the specification of setting the sender ip to 0.
I can't understand the need for this strange way of constructing an
arp-reply packet. Is this a feature or a bug?
Regards
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Strange behavior in arp probe reply, bug or feature?
Hi,
I was working on a project that involved having our dhcp-client
performing a check on the offered IP-address to make sure it wasn't in
use.
The standard way of doing this is sending out an arp request with the
sender ip set to 0. (RFC2131).
When I was testing out my implemented solution I noticed that my linux
machines responded a bit odd to this arp-request.
The arp-reply had the sender-ip field set to the same as the target-ip
field and the target-hwaddr field the same as the sender-hwaddr field.
All the values where the addresses of the machine replying to the request.
This is not a standard arp way of replying.
I checked the code in net/ipv4/arp.c and found this:
/* Special case: IPv4 duplicate address detection packet (RFC2131) */
if (sip == 0) {
if (arp-ar_op == __constant_htons(ARPOP_REQUEST)
inet_addr_type(tip) == RTN_LOCAL)
arp_send(ARPOP_REPLY,ETH_P_ARP
,tip,dev,tip,sha,dev-dev_addr,dev-dev_addr);
goto out;
}
This code will implement the strange behavior mentioned above.
I checked the arp-request against a box running Windows XP and one
running OpenBSD, none of them had this strange beahvior, but rather
replied in a way that
I expected to see, with the requesting machines mac in the target
hw-addr field and 0.0.0.0 in the target ip field.
The linux boxes I tested against ran the 2.16.18 and the 2.16.22
kernel and the code above is still used in the 2.16.23-1 kernel.
I changed the code above on one of them to have the arp_send called in
a normal way:
arp_send(ARPOP_REPLY,ETH_P_ARP,sip,dev,tip,sha,dev-dev_addr,sha);
After the change the replies looked like the ones I was receiving from
the OpenBSD and Windows-box.
The comment above the code mention the RFC2131, but there is no
mention, that I can see to this construction of the reply. I can only
see the specification of setting the sender ip to 0.
I can't understand the need for this strange way of constructing an
arp-reply packet. Is this a feature or a bug?
Regards
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
