netdev - Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c, take two

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]

Message-ID: <CAMuHMdWSzG4npwjpJ=cwNFKhyBZjvgPn1sp6nZ2re6QsRWPpXg@mail.gmail.com>
Date:	Thu, 5 Apr 2012 11:28:25 +0200
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Paul Gortmaker <paul.gortmaker@...driver.com>
Cc:	Michael Schmitz <schmitzmic@...glemail.com>,
	linux-m68k@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH] m68k/atari: EtherNEC - rewrite to use mainstream ne.c,
 take two

On Wed, Apr 4, 2012 at 22:46, Paul Gortmaker
<paul.gortmaker@...driver.com> wrote:
> On 12-04-01 04:49 AM, Michael Schmitz wrote:
>>> And on re-reading the comments in the other part of the patch, i.e.
>>> "...emulates the card interrupt via a timer"  --perhaps the driver
>>> should be just fixed to support generic netpoll, instead of adding
>>> an arch specific thing that amounts to netpoll.  Then anyone can
>>> attempt to limp along and use one of these ancient relics w/o IRQ.
>>>
>> Here's take two of my patch to convert the m68k Atari ROM port Ethernet
>> driver to use mainstream ne.c, with minimal changes to the core NE2000
>> code.
>>
>> In particular:
>>
>> Changes to core net code:
>> * add a platform specific IRQ flag, so ne.c can share a hardware or
>> timer interrupt with some other interrupt source.
>>
>> Changes to arch/m68k code:
>> * register the 8390 platform device on Atari only if the hardware is present
>> * retain the old driver (atari_ethernec.c in Geert's tree) under a
>> different config option, to be removed soon.
>>
>> Regarding your suggestion that netpoll be used instead of a dedicated
>> timer interrupt: no changes to ne.c or 8390p.c are required to use
>> netpoll, it all works out of the box. All that is needed to use the
>> driver with netpoll is setting the device interrupt to some source that
>> can be registered, and enabling CONFIG_NETPOLL. Interrupt rate and hence
>> throughput is lower with netpoll though, which is why I still prefer the
>> dedicated timer option.
>
> How much lower?  Enough to matter?  Implicit in that question is
> the assumption that this is largely a hobbyist platform and nobody
> is using it in a closet to route gigabytes of traffic.

One other thing we could do is increase CONFIG_HZ to 250.

> Also, the only advantage to modifying ne.c is to allow dumping
> the old driver.  What is the "remove soon" plan?  Any reason
> for it to not be synchronous?  That would eliminate the Kconfig
> churn and the introduction of the _OLD option.  Modifying ne.c
> and then deciding to keep the old driver because it is "faster"
> would make this change pointless.

>From my point of view, "remove soon" means it will never hit mainline.

>> diff --git a/drivers/net/ethernet/8390/8390.h
>> b/drivers/net/ethernet/8390/8390.h
>> index ef325ff..9416245 100644
>> --- a/drivers/net/ethernet/8390/8390.h
>> +++ b/drivers/net/ethernet/8390/8390.h
>> @@ -32,6 +32,14 @@ extern void ei_poll(struct net_device *dev);
>>  extern void eip_poll(struct net_device *dev);
>>  #endif
>>
>> +/* Some platforms may need special IRQ flags */
>> +#if IS_ENABLED(CONFIG_ATARI_ETHERNEC)
>> +#  define EI_IRQ_FLAGS    IRQF_SHARED
>> +#endif
>> +
>> +#ifndef EI_IRQ_FLAGS
>> +#  define EI_IRQ_FLAGS    0
>> +#endif
>
> This seems more klunky than it needs to be.  If we assume that anyone
> building ne.c on atari is hence trying to drive an ethernec device
> than it can just be
>
> #ifdef CONFIG_ATARI
> #define EI_IRQ_FLAGS    IRQF_SHARED
> #else
> #define EI_IRQ_FLAGS    0
> #endif

Indeed, with a small modification (keep multi-platform kernels in mind):

#ifdef CONFIG_ATARI
#define EI_IRQ_FLAGS    (MACH_IS_ATARI ? IRQF_SHARED : 0)
#else
#define EI_IRQ_FLAGS    0
#endif

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html