netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: "David S. Miller" <davem@redhat.com>
Cc: willy@debian.org, netdev@oss.sgi.com
Subject: Re: [PATCH] ethtool_ops rev 4
Date: Fri, 01 Aug 2003 19:01:21 -0400	[thread overview]
Message-ID: <3F2AF141.2010308@pobox.com> (raw)
In-Reply-To: <20030801153255.204baf66.davem@redhat.com>

David S. Miller wrote:
> On Fri, 01 Aug 2003 18:26:37 -0400
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
> 
>>Strangely enough, creating a SET_ETHTOOL_OPS() macro (or 
>>netif_ethtool_ops or pick your name) reduces ifdefs.
> 
> 
> And then we'll have all of these functions present in
> the driver, unused, and we'll get tons of warning from
> gcc.
> 
> The duplication of code is still there, and this is the
> main point.

Not correct:  there is nothing unused, there are no warnings, in either 
the in-kernel case or the older-kernel case.  Look at kcompat.  That is 
code that is working, and producing the 2.4/2.6-ready vendor drivers I 
spoke of.

I'm apparently not communicating the design that exists in kcompat, if 
you think this.  The design is:

	code for 2.6, and it magically works in 2.4

It's a back-compat system that is so good you don't even know it's 
there.  It's completely invisible to the mainline kernel -- as it should 
be -- presuming that one pays attention to subtle API change effects.

Do you see yet how there is no code duplication, no ifdefs, no warnings 
about unused functions?  That is the key point of the whole design, and 
key to the thread of discussion here.


> You can't get rid of the duplicated code without accepting that you
> will have seperate 2.6.x and 2.4.x strains of your driver.
> 
> If you aren't willing to accept seperate strains of your driver, you
> simply don't use netdev_ops.

Look at kcompat.  That is real, working code that demonstrates the approach.


>>the few things that is not easily work-around-able is new additions to 
>>existing structures (which wouldn't exist in older kernels).  That's 
>>what SET_ETHTOOL_OPS would wrap, while also providing a trigger for 
>>generic compat glue.
> 
> 
> What gets rid of the static functions that do the work when
> SET_ETHTOOL_OPS() is a nop?

SET_ETHTOOL_OPS is never a no-op.

The back-compat form of SET_ETHTOOL_OPS registers the ethtool_ops 
pointer in storage for later use.

A DO_ETHTOOL_OPS macro in the driver's ->do_ioctl -- intentionally not 
included in the kernel -- does the rest, calling kcompat's backported 
net/core/ethtool.c, which in turn calls the ethtool_ops hooks in the 
driver.  Making the kcompat'd net driver ready for 2.6 would then 
involve simply deleting one line.

That's why there is no code duplication or unused driver code.

	Jeff

  reply	other threads:[~2003-08-01 23:01 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-08-01 15:02 [PATCH] ethtool_ops rev 4 Matthew Wilcox
2003-08-01 15:40 ` Jeff Garzik
2003-08-01 15:46   ` Matthew Wilcox
2003-08-01 16:25     ` Jeff Garzik
2003-08-01 20:20       ` David S. Miller
2003-08-01 22:26         ` Jeff Garzik
2003-08-01 22:32           ` David S. Miller
2003-08-01 23:01             ` Jeff Garzik [this message]
2003-08-01 23:01               ` David S. Miller
2003-08-01 23:17                 ` Jeff Garzik
2003-08-01 23:19                   ` David S. Miller
2003-08-01 23:42                     ` Jeff Garzik
2003-08-01 23:43                       ` David S. Miller
2003-08-01 22:35           ` Jeff Garzik
2003-08-01 22:34             ` David S. Miller
2003-08-01 23:09               ` Jeff Garzik
2003-08-01 23:08                 ` David S. Miller
2003-08-01 23:35                   ` Jeff Garzik
2003-08-01 23:34                     ` David S. Miller
2003-08-02 22:21       ` Matthew Wilcox
2003-08-02 22:34         ` Jeff Garzik
2003-08-03  0:27           ` Matthew Wilcox
2003-08-03  3:14             ` Jeff Garzik
2003-08-03 14:56               ` Matthew Wilcox
2003-08-03 17:09                 ` Jeff Garzik
2003-08-05 14:32                   ` Matthew Wilcox
2003-08-03  0:28           ` David S. Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3F2AF141.2010308@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=davem@redhat.com \
    --cc=netdev@oss.sgi.com \
    --cc=willy@debian.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).