netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org, socketcan@hartkopp.net, hadi@cyberus.ca,
	xemul@sw.ru, tgraf@suug.ch
Subject: Re: [RFC RTNETLINK 00/09]: Netlink link creation API
Date: Wed, 06 Jun 2007 09:25:38 -0600	[thread overview]
Message-ID: <m1d509umtp.fsf@ebiederm.dsl.xmission.com> (raw)
In-Reply-To: <46669FA0.3030405@trash.net> (Patrick McHardy's message of "Wed, 06 Jun 2007 13:50:56 +0200")

Patrick McHardy <kaber@trash.net> writes:

> Eric W. Biederman wrote:
>> Reading through the patches they look usable to me.
>> 
>> Having to patch iproute to create the more interesting network
>> devices sucks, but that problem seems fundamental.  We might
>> be able to avoid it if we allowed fields to be reused between
>> different types of devices but that makes the error checking
>> trickier, and we aren't likely to have that many types of
>> devices so there likely isn't much value in generalizing.
>
>
> You don't really need to patch it, installing a new iplink_XXX.so
> file is enough. Generalizing driver specific options more than
> what we currently have doesn't look very promising. I think your
> driver was simple enough to get along with the generic device
> attributes though (IFLA_LINK or IFLA_MASTER).

I need to know the other device in the pair of devices I am creating.
If ifindex was selectable IFLA_LINK or IFLA_MASTER might be
interesting however they are currently are not, and I'm not quite
certain about letting a user select the ifindex.  Although there may
come a point when dealing with migration when it makes sense.

Hmm.  I guess if I had a reasonable default I could find out the
pair device by looking at the returned NEW_LINK message.

Looking more close.  IFLA_MASTER does not work because I don't
have a master/slave relationship.

IFLA_LINK looks like it will work.  I don't precisely match the
semantics though which makes me nervous.  In particular my other
device is not something I am sending through but what I am sending
to.  The way the IPv6 code uses iflink to get the link local address
starting with the hardware address of the iflink would be completely
the wrong thing to do in my case.  Now my device won't have the magic
IPv6 tunnel arp type so that code won't trigger.  Still it is
a challenge.

I still think adding a IFLA_PARTNER or a custom attribute is cleaner
in this case.  Slight semantic mismatches are the worst design bugs
to correct.

To some extent this is a practical problematic point for me, because
in the context of multiple network namespaces I could theoretically
have both network devices have the same name and the same ifindex in
different network namespaces.  Although it really doesn't matter
unless they are in the same network namespace in which case they
can't have the same ifindex or ifname.

>> I do think we should specify the IFLA_KIND (was: IFLA_NAME) values in
>> a header file.  So it is easy to get a list of all of the different
>> kinds and so we don't conflict.
>
>
> I don't think conflicts are going to be a problem (it would be
> nice if modpost would warn about duplicate aliases though).
> How is listing IFLA_KIND types in a header file going to help
> get a list? Presuming the user knows what kind of device he
> wants to set up and is not just looking for things to play
> around with I also don't see any real value in this.

This isn't about the user this is about maintaining the ABI.

We have to control set of strings for IFLA_KIND.  Having them all
in a single header file means that we can easily look when we add
support for a new kind to see if some other driver has already
used that kind.

The same reason we stick the rest of the enumerations into a header
file.

Strings don't conflict as easily as small integers do, but it is still
possible to have a conflict, so having something like an ifla_kind.h to
hold them would be useful.

Eric

  reply	other threads:[~2007-06-06 15:27 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-05 14:12 [RFC RTNETLINK 00/09]: Netlink link creation API Patrick McHardy
2007-06-05 14:12 ` [RFC NETLINK 01/09]: Mark netlink policies const Patrick McHardy
2007-06-05 19:39   ` David Miller
2007-06-05 14:12 ` [RFC RTNETLINK 02/09]: ifindex 0 does not exist Patrick McHardy
2007-06-05 19:40   ` David Miller
2007-06-05 14:12 ` [RFC RTNETLINK 03/09]: Split up rtnl_setlink Patrick McHardy
2007-06-05 19:41   ` David Miller
2007-06-05 14:12 ` [RFC RTNETLINK 04/09]: Link creation API Patrick McHardy
2007-06-05 19:43   ` David Miller
2007-06-05 21:09     ` Patrick McHardy
2007-06-05 22:03   ` Stephen Hemminger
2007-06-05 23:17     ` Patrick McHardy
2007-06-05 23:16       ` Stephen Hemminger
2007-06-06 11:42         ` Patrick McHardy
2007-06-06 16:37   ` Eric W. Biederman
2007-06-06 16:50     ` Patrick McHardy
2007-06-06 18:02       ` Eric W. Biederman
2007-06-05 14:12 ` [RFC DUMMY 05/09]: Use dev->stats Patrick McHardy
2007-06-05 19:44   ` David Miller
2007-06-05 14:13 ` [RFC DUMMY 06/09]: Keep dummy devices on list Patrick McHardy
2007-06-05 19:44   ` David Miller
2007-06-05 14:13 ` [RFC DUMMY 07/09]: Use rtnl_link API Patrick McHardy
2007-06-05 19:46   ` David Miller
2007-06-05 14:13 ` [RFC IFB 08/09]: Keep ifb devices on list Patrick McHardy
2007-06-05 14:13 ` [RFC IFB 09/09]: Use rtnl_link API Patrick McHardy
2007-06-05 14:18 ` [RFC IPROUTE]: iplink: use netlink for link configuration Patrick McHardy
2007-06-05 19:37 ` [RFC RTNETLINK 00/09]: Netlink link creation API David Miller
2007-06-05 21:10   ` Patrick McHardy
2007-06-05 22:00 ` jamal
2007-06-05 22:07   ` Patrick McHardy
2007-06-05 22:29     ` jamal
2007-06-06  0:40 ` Eric W. Biederman
2007-06-06 11:50   ` Patrick McHardy
2007-06-06 15:25     ` Eric W. Biederman [this message]
2007-06-06 15:35       ` Patrick McHardy
2007-06-06 15:56         ` Alexey Kuznetsov
2007-06-06 16:32           ` Patrick McHardy
2007-06-06 17:31             ` Eric W. Biederman
2007-06-06 19:14             ` Alexey Kuznetsov
2007-06-07  8:06               ` Eric W. Biederman
2007-06-06 16:13         ` Eric W. Biederman
2007-06-06 16:25           ` Patrick McHardy
2007-06-06 13:46 ` Patrick McHardy

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=m1d509umtp.fsf@ebiederm.dsl.xmission.com \
    --to=ebiederm@xmission.com \
    --cc=hadi@cyberus.ca \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    --cc=socketcan@hartkopp.net \
    --cc=tgraf@suug.ch \
    --cc=xemul@sw.ru \
    /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).