RFC: interface description

[Xin LI-2]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi guys,

While playing with some OpenBSD installation I found that they have an
interesting feature - adding description to a NIC.  This is useful for
system administrators to "tag" the interface, also, the ladvd program
has a feature to use the SIOCSIFDESCR ioctl to document the remote CDP
peer like:

em0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        ether 00:11:22:33:44:55
        description: connected to myrouter.home (CDP)
        [...]

The attached patch ported the feature to FreeBSD.

Cheers,
- --
Xin LI <delphij@...> http://www.delphij.net/
FreeBSD - The Power to Serve!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.12 (FreeBSD)

iEYEARECAAYFAkqD7qgACgkQi+vbBBjt66CF+QCeO6INwh3S1T/LvhIUTjZ/Ix4H
zQkAniftv+SQ+irEcnItGHTbLH0HyUez
=cEsJ
-----END PGP SIGNATURE-----

Index: sbin/ifconfig/ifconfig.8
===================================================================
--- sbin/ifconfig/ifconfig.8 (revision 196163)
+++ sbin/ifconfig/ifconfig.8 (working copy)
@@ -28,7 +28,7 @@
 .\"     From: @(#)ifconfig.8 8.3 (Berkeley) 1/5/94
 .\" $FreeBSD$
 .\"
-.Dd July 8, 2009
+.Dd September 14, 2009
 .Dt IFCONFIG 8
 .Os
 .Sh NAME
@@ -258,6 +258,12 @@ Disable permanently promiscuous mode.
 Another name for the
 .Fl alias
 parameter.
+.It Cm description Ar value
+Specify a description of the interface.
+This can be used to label interfaces in situations where they may
+otherwise be difficult to distinguish.
+.It Cm -description
+Clear the interface description.
 .It Cm down
 Mark an interface
 .Dq down .
@@ -2443,6 +2449,10 @@ Configure the interface
 to use 100baseTX, full duplex Ethernet media options:
 .Dl # ifconfig xl0 media 100baseTX mediaopt full-duplex
 .Pp
+Label the em0 interface as an uplink:
+.Pp
+.Dl # ifconfig em0 description \&"Uplink to Gigabit Switch 2\&"
+.Pp
 Create the software network interface
 .Li gif1 :
 .Dl # ifconfig gif1 create
Index: sbin/ifconfig/ifconfig.c
===================================================================
--- sbin/ifconfig/ifconfig.c (revision 196163)
+++ sbin/ifconfig/ifconfig.c (working copy)
@@ -83,6 +83,7 @@ static const char rcsid[] =
 struct ifreq ifr;
 
 char name[IFNAMSIZ];
+char descr[IFDESCRSIZE];
 int setaddr;
 int setmask;
 int doalias;
@@ -822,6 +823,36 @@ setifname(const char *val, int dummy __unused, int
  free(newname);
 }
 
+/* ARGSUSED */
+static void
+setifdescr(const char *val, int dummy __unused, int s,
+    const struct afswtch *afp)
+{
+ char *newdescr;
+
+ newdescr = strdup(val);
+ if (newdescr == NULL) {
+ warn("no memory to set ifdescr");
+ return;
+ }
+ ifr.ifr_data = newdescr;
+ if (ioctl(s, SIOCSIFDESCR, (caddr_t)&ifr) < 0) {
+ warn("ioctl (set descr)");
+ free(newdescr);
+ return;
+ }
+ strlcpy(descr, newdescr, sizeof(descr));
+ free(newdescr);
+}
+
+/* ARGSUSED */
+static void
+unsetifdescr(const char *val, int value, int s, const struct afswtch *afp)
+{
+
+ setifdescr("", 0, s, 0);
+}
+
 #define IFFBITS \
 "\020\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6SMART\7RUNNING" \
 "\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX\15LINK0\16LINK1\17LINK2" \
@@ -866,6 +897,11 @@ status(const struct afswtch *afp, const struct soc
  printf(" mtu %d", ifr.ifr_mtu);
  putchar('\n');
 
+ ifr.ifr_data = (caddr_t)&descr;
+ if (ioctl(s, SIOCGIFDESCR, &ifr) == 0 &&
+    strlen(ifr.ifr_data) > 0)
+ printf("\tdescription: %s\n", ifr.ifr_data);
+
  if (ioctl(s, SIOCGIFCAP, (caddr_t)&ifr) == 0) {
  if (ifr.ifr_curcap != 0) {
  printb("\toptions", ifr.ifr_curcap, IFCAPBITS);
@@ -1035,6 +1071,10 @@ static struct cmd basic_cmds[] = {
  DEF_CMD("-arp", IFF_NOARP, setifflags),
  DEF_CMD("debug", IFF_DEBUG, setifflags),
  DEF_CMD("-debug", -IFF_DEBUG, setifflags),
+ DEF_CMD_ARG("description", setifdescr),
+ DEF_CMD_ARG("descr", setifdescr),
+ DEF_CMD("-description", 0, unsetifdescr),
+ DEF_CMD("-descr", 0, unsetifdescr),
  DEF_CMD("promisc", IFF_PPROMISC, setifflags),
  DEF_CMD("-promisc", -IFF_PPROMISC, setifflags),
  DEF_CMD("add", IFF_UP, notealias),
Index: share/man/man4/netintro.4
===================================================================
--- share/man/man4/netintro.4 (revision 196163)
+++ share/man/man4/netintro.4 (working copy)
@@ -32,7 +32,7 @@
 .\"     @(#)netintro.4 8.2 (Berkeley) 11/30/93
 .\" $FreeBSD$
 .\"
-.Dd June 18, 2004
+.Dd September 15, 2009
 .Dt NETINTRO 4
 .Os
 .Sh NAME
@@ -277,6 +277,15 @@ and
 fields of the
 .Vt ifreq
 structure, respectively.
+.It Dv SIOCGIFDESCR Fa "struct ifreq *"
+Get the interface description, returned in the
+.Va ifru_data
+field.
+.It Dv SIOCSIFDESCR Fa "struct ifreq *"
+Set the interface description to the value of the
+.Va ifru_data
+field, limited to the size of
+.Dv IFDESCRSIZE .
 .It Dv SIOCSIFFLAGS
 Set interface flags field.
 If the interface is marked down,
Index: sys/kern/kern_jail.c
===================================================================
--- sys/kern/kern_jail.c (revision 196163)
+++ sys/kern/kern_jail.c (working copy)
@@ -3437,6 +3437,7 @@ prison_priv_check(struct ucred *cred, int priv)
  case PRIV_NET_SETIFMETRIC:
  case PRIV_NET_SETIFPHYS:
  case PRIV_NET_SETIFMAC:
+ case PRIV_NET_SETIFDESCR:
  case PRIV_NET_ADDMULTI:
  case PRIV_NET_DELMULTI:
  case PRIV_NET_HWIOCTL:
Index: sys/net/if.c
===================================================================
--- sys/net/if.c (revision 196163)
+++ sys/net/if.c (working copy)
@@ -1927,6 +1927,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t d
  int new_flags, temp_flags;
  size_t namelen, onamelen;
  char new_name[IFNAMSIZ];
+ char ifdescrbuf[IFDESCRSIZE];
  struct ifaddr *ifa;
  struct sockaddr_dl *sdl;
 
@@ -1965,6 +1966,25 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t d
  ifr->ifr_phys = ifp->if_physical;
  break;
 
+ case SIOCGIFDESCR:
+ bzero(ifdescrbuf, sizeof(ifdescrbuf));
+ strlcpy(ifdescrbuf, ifp->if_description, IFDESCRSIZE);
+ error = copyout(ifdescrbuf, ifr->ifr_data, IFDESCRSIZE);
+ break;
+
+ case SIOCSIFDESCR:
+ error = priv_check(td, PRIV_NET_SETIFDESCR);
+ if (error)
+ return (error);
+ error = copyinstr(ifr->ifr_data, ifdescrbuf,
+ IFDESCRSIZE, NULL);
+ if (error == 0) {
+ bzero(ifp->if_description, IFDESCRSIZE);
+ strlcpy(ifp->if_description, ifdescrbuf, IFDESCRSIZE);
+ getmicrotime(&ifp->if_lastchange);
+ }
+ break;
+
  case SIOCSIFFLAGS:
  error = priv_check(td, PRIV_NET_SETIFFLAGS);
  if (error)
Index: sys/net/if.h
===================================================================
--- sys/net/if.h (revision 196163)
+++ sys/net/if.h (working copy)
@@ -60,6 +60,10 @@ struct ifnet;
 #define IFNAMSIZ IF_NAMESIZE
 #define IF_MAXUNIT 0x7fff /* historical value */
 #endif
+
+/* Length of interface description, including terminating '\0'. */
+#define IFDESCRSIZE 64
+
 #if __BSD_VISIBLE
 
 /*
Index: sys/net/if_var.h
===================================================================
--- sys/net/if_var.h (revision 196163)
+++ sys/net/if_var.h (working copy)
@@ -197,6 +197,7 @@ struct ifnet {
  void *if_pf_kif;
  void *if_lagg; /* lagg glue */
  u_char if_alloctype; /* if_type at time of allocation */
+ char if_description[IFDESCRSIZE]; /* interface description */
 
  /*
  * Spare fields are added so that we can modify sensitive data
Index: sys/sys/priv.h
===================================================================
--- sys/sys/priv.h (revision 196163)
+++ sys/sys/priv.h (working copy)
@@ -335,6 +335,7 @@
 #define PRIV_NET_LAGG 415 /* Administer lagg interface. */
 #define PRIV_NET_GIF 416 /* Administer gif interface. */
 #define PRIV_NET_SETIFVNET 417 /* Move interface to vnet. */
+#define PRIV_NET_SETIFDESCR 418 /* Set interface description. */
 
 /*
  * 802.11-related privileges.

_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[sthaug]

> While playing with some OpenBSD installation I found that they have an
> interesting feature - adding description to a NIC.  This is useful for
> system administrators to "tag" the interface, also, the ladvd program
> has a feature to use the SIOCSIFDESCR ioctl to document the remote CDP
> peer like:

Yes please! This is an "obvious" feature to those of us who are router
geeks, and I've been wishing for this to show up in FreeBSD for years.

Now if I could have "sticky" static routes also I would be even happier
(as in: Next hop disappears -> route disappears. Next hop available
again -> route shows up in routing table again).

Steinar Haug, Nethelp consulting, sthaug@...
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Andrey V. Elsukov]

Xin LI wrote:
> While playing with some OpenBSD installation I found that they have an
> interesting feature - adding description to a NIC.  This is useful for
> system administrators to "tag" the interface, also, the ladvd program
> has a feature to use the SIOCSIFDESCR ioctl to document the remote CDP

Something similar was rejected at least two times :)
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/83622
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/110720

--
WBR, Andrey V. Elsukov
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Luiz Otavio O Souza-4]

> Xin LI wrote:
>> While playing with some OpenBSD installation I found that they have an
>> interesting feature - adding description to a NIC.  This is useful for
>> system administrators to "tag" the interface, also, the ladvd program
>> has a feature to use the SIOCSIFDESCR ioctl to document the remote CDP
>
> Something similar was rejected at least two times :)
> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/83622
> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/110720
>

That was a long time ago...

The Xin Li patch is a quite complete (manual pages updated as well) and only
suffer from the 64 byte limit on interface description (and the 64 more
bytes on ifnet), wich IMHO, should be easy to fix.

Thank you Xin Li for bringing this =)

Luiz

_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Julian Elischer]

Andrey V. Elsukov wrote:
> Xin LI wrote:
>> While playing with some OpenBSD installation I found that they have an
>> interesting feature - adding description to a NIC.  This is useful for
>> system administrators to "tag" the interface, also, the ladvd program
>> has a feature to use the SIOCSIFDESCR ioctl to document the remote CDP
>
> Something similar was rejected at least two times :)


not rejected..

suffered from lack of enthusiasm maybe.

> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/83622
> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/110720
>

_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Bjoern A. Zeeb]

On Thu, 13 Aug 2009, Julian Elischer wrote:

> Andrey V. Elsukov wrote:
>> Xin LI wrote:
>>> While playing with some OpenBSD installation I found that they have an
>>> interesting feature - adding description to a NIC.  This is useful for
>>> system administrators to "tag" the interface, also, the ladvd program
>>> has a feature to use the SIOCSIFDESCR ioctl to document the remote CDP
>>
>> Something similar was rejected at least two times :)
>>
>> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/83622
>> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/110720
>
> not rejected..
>
> suffered from lack of enthusiasm maybe.

My point has always been - if I have to add/do an ioctl I can always also
use a library call that will read it from a .txt, .xml, .db file
or whatever and I don't have to go to the kernel, handle all the
string length problems there, ... especially as the kernel cannot do
anything with that string.

/bz

--
Bjoern A. Zeeb           What was I talking about and who are you again?
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[sthaug]

> My point has always been - if I have to add/do an ioctl I can always also
> use a library call that will read it from a .txt, .xml, .db file
> or whatever and I don't have to go to the kernel, handle all the
> string length problems there, ... especially as the kernel cannot do
> anything with that string.

Personally, I couldn't care less if an interface description is stored
in the kernel or in a file. I *do* care about being able to set and
show the description via ifconfig.

Steinar Haug, Nethelp consulting, sthaug@...
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Xin LI-2]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Bjoern A. Zeeb wrote:

> On Thu, 13 Aug 2009, Julian Elischer wrote:
>
>> Andrey V. Elsukov wrote:
>>> Xin LI wrote:
>>>> While playing with some OpenBSD installation I found that they have an
>>>> interesting feature - adding description to a NIC.  This is useful for
>>>> system administrators to "tag" the interface, also, the ladvd program
>>>> has a feature to use the SIOCSIFDESCR ioctl to document the remote CDP
>>>
>>> Something similar was rejected at least two times :)
>>>
>>> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/83622
>>> http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/110720
>>
>> not rejected..
>>
>> suffered from lack of enthusiasm maybe.
>
> My point has always been - if I have to add/do an ioctl I can always also
> use a library call that will read it from a .txt, .xml, .db file
> or whatever and I don't have to go to the kernel, handle all the
> string length problems there, ... especially as the kernel cannot do
> anything with that string.

I have also received some private e-mail regarding this, and I think
that would be more sensible (better flexibility, and of course, a better
chance that we can MFC it since it won't change kernel ABI).

The only question I have would be, that is it possible to uniquely
identify a NIC without assistance from kernel?  For instance, one can
change an interface from being called "em0" to "eth0" and from "bge0" to
"em0".  It's easy to track this information through ifconfig(8) with a
callback, clean up the file upon restart, but we can not prevent other
programs from calling IOCSIFNAME on the interface.  Any idea for this?

Cheers,
- --
Xin LI <delphij@...> http://www.delphij.net/
FreeBSD - The Power to Serve!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.12 (FreeBSD)

iEYEARECAAYFAkqEatIACgkQi+vbBBjt66Ah7gCgsv2O9FpF+fAbIPJqkICWkC+I
HmYAoK4GmPynk4qh2FL2CnJY12MOBzEB
=MG3y
-----END PGP SIGNATURE-----
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Andrey V. Elsukov]

Xin LI wrote:
> The only question I have would be, that is it possible to uniquely
> identify a NIC without assistance from kernel?  For instance, one can
> change an interface from being called "em0" to "eth0" and from "bge0" to
> "em0".  It's easy to track this information through ifconfig(8) with a
> callback, clean up the file upon restart, but we can not prevent other
> programs from calling IOCSIFNAME on the interface.  Any idea for this?

What about using interface index as a key(see if_nameindex(3))?

--
WBR, Andrey V. Elsukov
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Bjoern A. Zeeb]

On Fri, 14 Aug 2009, Andrey V. Elsukov wrote:

Hi,

> Xin LI wrote:
>> The only question I have would be, that is it possible to uniquely
>> identify a NIC without assistance from kernel?  For instance, one can
>> change an interface from being called "em0" to "eth0" and from "bge0" to
>> "em0".  It's easy to track this information through ifconfig(8) with a
>> callback, clean up the file upon restart, but we can not prevent other
>> programs from calling IOCSIFNAME on the interface.  Any idea for this?
>
> What about using interface index as a key(see if_nameindex(3))?

So here comes the usual catch 22 on a classic PC system:
  you can change everything.

Using RFC 2553 Section 4 is probably the best indeed but has drawbacks
as well.


If you match by xname, renaming the interface will break things.
If you match by dname+unit, unit can still change between boots (see
     further on).
If you match by MAC address the MAC address can be changed by the
     user.
If you match by (PCI/..) slot, cards can be moved.
If you match by card serial number, *oops* most of those don't have
     that in the consumer or even server world so that's not an option.
Because of the above we don't have any metadata that would allow us to
     have persistent ifIndexes between boots.
Once you add portable interfaces (pcmcia, usb, pc card, ..) to the game
     even your ifIndex upon boot may vary depending on if a card is
     plugged in or not or if you added another one, possibly using the
     same driver changing the unit number if unlucky (see above).
Now add cloned interfaces from gif, tun, .. to wlan and you'll
     understand the big (problem|picture).


Now let us look at this from a different view - what can venders do to
avoid all this or how can they handle things:

They might have ways to uniquely identify each interface so an ifIndex
     could possibly always be the same after the interface was first
     put into a device (interface serial number for example).
They usually do not allow renaming of interface names.
They usually do not have physical interfaces coming and going very
     often.
They do have "cloned-a-like" interfaces as well so they have to handle
     them somehow. I have seldomly seen descriptions on those if they
     come and go.  They possibly treat static "tunnel" interfaces, etc.
     more statically having a well defined bringup order.
The cards lose their description if moved to a different slot or
     rather as the interface goes away usually and even if plugged into
     a different slot would lose the description but maybe not the
     ifIndex.


What does that mean for us?

For "dynamic" interfaces storing things with the kernel would for sure
     be easier as if you remove the interface the description is gone.
If not storing in the kernel we get interface insertion and removal
     events already to userspace/devd imho we could use.
The moment you add the description it doesn't matter what the
     interface is, as you can name it always some way.
If you take the classic BSD scheme and will seed the "storage" upon
     boot from scratch from rc.conf or other places you will have a
     consitent ifIndex for all hard wired physical interfaces
     (non-hotpluggable).
     [Unless you move hardware that should even be the same if we don't
     change bus scanning orders, etc.]
For all the cloned stuff things are harder, as we to my knowledge even
     recycle ifIndexes to not have huge gaps in the array (which is a
     real pain), so there are kernel limitations as well in the way
     we store/lookup things that also had come up during the vnet
     merges and that people are aware of.
Bsnmp should have to handle parts of this all already and is for sure
     one of the first consumers so talking to harti and syrinx or
     looking at that code might also be a good thing todo.


My biggest worry comes with the cloned interfaces like tuns or similar
ones if they come and go often; if an ifIndex is recycled and even if
the interface name is the same afterwards there should be no old
description left, so "purging" entries on interface deletion and
possibly asserting that on interface creation would be a good idea.


JustMy2ctAndEndOfBrainDumpInTheMorningBeforeTheFirstCupOfCoffe

/bz


--
Bjoern A. Zeeb           What was I talking about and who are you again?
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Louis Mamakos]

On Aug 14, 2009, at 3:58 AM, Bjoern A. Zeeb wrote:

> On Fri, 14 Aug 2009, Andrey V. Elsukov wrote:
>
> Hi,
>
>> Xin LI wrote:
>>> The only question I have would be, that is it possible to uniquely
>>> identify a NIC without assistance from kernel?  For instance, one  
>>> can
>>> change an interface from being called "em0" to "eth0" and from  
>>> "bge0" to
>>> "em0".  It's easy to track this information through ifconfig(8)  
>>> with a
>>> callback, clean up the file upon restart, but we can not prevent  
>>> other
>>> programs from calling IOCSIFNAME on the interface.  Any idea for  
>>> this?
>>
>> What about using interface index as a key(see if_nameindex(3))?
>
> So here comes the usual catch 22 on a classic PC system:
> you can change everything.
>
> Using RFC 2553 Section 4 is probably the best indeed but has drawbacks
> as well.
>
>
> If you match by xname, renaming the interface will break things.
> If you match by dname+unit, unit can still change between boots (see
>    further on).
> If you match by MAC address the MAC address can be changed by the
>    user.
> If you match by (PCI/..) slot, cards can be moved.
> If you match by card serial number, *oops* most of those don't have
>    that in the consumer or even server world so that's not an option.
> Because of the above we don't have any metadata that would allow us to
>    have persistent ifIndexes between boots.
> Once you add portable interfaces (pcmcia, usb, pc card, ..) to the  
> game
>    even your ifIndex upon boot may vary depending on if a card is
>    plugged in or not or if you added another one, possibly using the
>    same driver changing the unit number if unlucky (see above).
> Now add cloned interfaces from gif, tun, .. to wlan and you'll
>    understand the big (problem|picture).
>
>
> Now let us look at this from a different view - what can venders do to
> avoid all this or how can they handle things:
>
> They might have ways to uniquely identify each interface so an ifIndex
>    could possibly always be the same after the interface was first
>    put into a device (interface serial number for example).
> They usually do not allow renaming of interface names.
> They usually do not have physical interfaces coming and going very
>    often.
> They do have "cloned-a-like" interfaces as well so they have to handle
>    them somehow. I have seldomly seen descriptions on those if they
>    come and go.  They possibly treat static "tunnel" interfaces, etc.
>    more statically having a well defined bringup order.
> The cards lose their description if moved to a different slot or
>    rather as the interface goes away usually and even if plugged into
>    a different slot would lose the description but maybe not the
>    ifIndex.
>
>
> What does that mean for us?
>
> For "dynamic" interfaces storing things with the kernel would for sure
>    be easier as if you remove the interface the description is gone.
> If not storing in the kernel we get interface insertion and removal
>    events already to userspace/devd imho we could use.
> The moment you add the description it doesn't matter what the
>    interface is, as you can name it always some way.
> If you take the classic BSD scheme and will seed the "storage" upon
>    boot from scratch from rc.conf or other places you will have a
>    consitent ifIndex for all hard wired physical interfaces
>    (non-hotpluggable).
>    [Unless you move hardware that should even be the same if we don't
>    change bus scanning orders, etc.]
> For all the cloned stuff things are harder, as we to my knowledge even
>    recycle ifIndexes to not have huge gaps in the array (which is a
>    real pain), so there are kernel limitations as well in the way
>    we store/lookup things that also had come up during the vnet
>    merges and that people are aware of.
> Bsnmp should have to handle parts of this all already and is for sure
>    one of the first consumers so talking to harti and syrinx or
>    looking at that code might also be a good thing todo.
>
>
> My biggest worry comes with the cloned interfaces like tuns or similar
> ones if they come and go often; if an ifIndex is recycled and even if
> the interface name is the same afterwards there should be no old
> description left, so "purging" entries on interface deletion and
> possibly asserting that on interface creation would be a good idea.
>
>
> JustMy2ctAndEndOfBrainDumpInTheMorningBeforeTheFirstCupOfCoffe
>
> /bz
>

Interface names (bge0, fxp1, etc.) name ports on the local
host/router.  The use of "interface descriptions" is almost
always an attempt to name or describe what's connected on the other
side of the network interface.  This is what the various "discovery"
protocols, like CDP[1] and LLDP[2] try to solve; to dynamically discover
WTF this port is plugged into.

The problem with ifIndex stability is somewhat related, but you could
imagine choosing some physical identifier (like the manufactured MAC
address) as the name.  The ifIndex in the general case doesn't account
for the user deciding to externally swap the ethernet cables between
a pair of ports.

Louis Mamakos

[1] http://en.wikipedia.org/wiki/Cisco_Discovery_Protocol
[2] http://en.wikipedia.org/wiki/Link_Layer_Discovery_Protocol

_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Li, Qing]

>
> My point has always been - if I have to add/do an ioctl I can always
> also use a library call that will read it from a .txt, .xml, .db file
> or whatever and I don't have to go to the kernel, handle all the
> string length problems there, ... especially as the kernel cannot do
> anything with that string.
>

The interface description feature is a useful feature. Quite a few
products out there actually put a label on the physical box so it's
reasonable to have the ability to label the ports in the kernel.

There are quite a few embedded systems and not-so-standalone boxes
out there that are derivatives of FreeBSD. These systems might not
have the luxury of a file system. And getting coredumps from the
field with such information embedded in the ifnet{} just makes
debugging field issues a little bit easier.

>
> So here comes the usual catch 22 on a classic PC system:
> you can change everything.
>
> Using RFC 2553 Section 4 is probably the best indeed but has
> drawbacks as well.
>

Seems rather off topic ...

-- Qing


_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Brooks Davis-2]

On Fri, Aug 14, 2009 at 10:49:24AM -0700, Li, Qing wrote:

> >
> > My point has always been - if I have to add/do an ioctl I can always
> > also use a library call that will read it from a .txt, .xml, .db file
> > or whatever and I don't have to go to the kernel, handle all the
> > string length problems there, ... especially as the kernel cannot do
> > anything with that string.
>
> The interface description feature is a useful feature. Quite a few
> products out there actually put a label on the physical box so it's
> reasonable to have the ability to label the ports in the kernel.
>
> There are quite a few embedded systems and not-so-standalone boxes
> out there that are derivatives of FreeBSD. These systems might not
> have the luxury of a file system. And getting coredumps from the
> field with such information embedded in the ifnet{} just makes
> debugging field issues a little bit easier.

I think this is a decently compelling argument for in-kernel
descriptions.  They do solve some synchronization issues and one pointer
is probably an acceptable price to pay given all the edge cases related
to keeping a file in sync if you went the totally user land route.

I general, I don't think we should try too hard to solve every problem
here.  Adding a pointer to ifnet, a quick get/set ioctl, ifconfig
support, and support for ifdesc_<ifp> or similar variables in rc.conf
is probably as much as makes sense to do.  This is mostly a feature for
appliance builders.  If I were working on adding the ability to slim down
ifnet to the base system, I'd certainly make this an optional feature,
but there are much fatter targets at the moment.

-- Brooks

[Xin LI-2]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi, guys,

Here is a patch that implements the functionality in userland (as
setifdescription/getifdescription functions in libutil); with this I
think we can also provide an option that some kernel feature (like Qing
Li said it might be useful for embedded systems, but of course the
kernel feature would require more careful design) being used without
modifying programs.  This version uses if_dname plus if_dunit as
distinguished name, and a Berkeley DB (hash, /etc/ifdescr.db) to store
the information.

In order to obtain if_dname and if_duint I had to create a new ioctl, as
we don't seem to expose these two fields in an KBI-stable manner in the
past.  I have not took a look at bsnmp yet but I'll take a look at it to
see if we have some better ways to distinguish the interface name.

Cheers,
- --
Xin LI <delphij@...> http://www.delphij.net/
FreeBSD - The Power to Serve!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.12 (FreeBSD)

iEYEARECAAYFAkqGAc0ACgkQi+vbBBjt66D82QCeMV3G+2FepN5asaxvAwpc0qKS
1poAoIhQ719JdYPE7sq+lseTtszr++o0
=ph72
-----END PGP SIGNATURE-----

Index: sbin/ifconfig/ifconfig.c
===================================================================
--- sbin/ifconfig/ifconfig.c (revision 196234)
+++ sbin/ifconfig/ifconfig.c (working copy)
@@ -63,6 +63,7 @@ static const char rcsid[] =
 #include <netdb.h>
 
 #include <ifaddrs.h>
+#include <if_description.h>
 #include <ctype.h>
 #include <err.h>
 #include <errno.h>
@@ -822,6 +823,24 @@ setifname(const char *val, int dummy __unused, int
  free(newname);
 }
 
+/* ARGSUSED */
+static void
+setifdescr(const char *val, int dummy __unused, int s,
+    const struct afswtch *afp __unused)
+{
+
+ setifdescription(s, &ifr, val);
+}
+
+/* ARGSUSED */
+static void
+unsetifdescr(const char *val __unused, int dummy __unused, int s,
+    const struct afswtch *afp __unused)
+{
+
+ setifdescription(s, &ifr, NULL);
+}
+
 #define IFFBITS \
 "\020\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6SMART\7RUNNING" \
 "\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX\15LINK0\16LINK1\17LINK2" \
@@ -843,6 +862,7 @@ status(const struct afswtch *afp, const struct soc
  struct ifaddrs *ift;
  int allfamilies, s;
  struct ifstat ifs;
+ char *description = NULL;
 
  if (afp == NULL) {
  allfamilies = 1;
@@ -866,6 +886,11 @@ status(const struct afswtch *afp, const struct soc
  printf(" mtu %d", ifr.ifr_mtu);
  putchar('\n');
 
+ if (getifdescription(s, &ifr, &description) == 0 && description != NULL)
+ printf("\tdescription: %s\n", description);
+ free(description);
+ description = NULL;
+
  if (ioctl(s, SIOCGIFCAP, (caddr_t)&ifr) == 0) {
  if (ifr.ifr_curcap != 0) {
  printb("\toptions", ifr.ifr_curcap, IFCAPBITS);
@@ -1035,6 +1060,10 @@ static struct cmd basic_cmds[] = {
  DEF_CMD("-arp", IFF_NOARP, setifflags),
  DEF_CMD("debug", IFF_DEBUG, setifflags),
  DEF_CMD("-debug", -IFF_DEBUG, setifflags),
+ DEF_CMD_ARG("description", setifdescr),
+ DEF_CMD_ARG("descr", setifdescr),
+ DEF_CMD("-description", 0, unsetifdescr),
+ DEF_CMD("-descr", 0, unsetifdescr),
  DEF_CMD("promisc", IFF_PPROMISC, setifflags),
  DEF_CMD("-promisc", -IFF_PPROMISC, setifflags),
  DEF_CMD("add", IFF_UP, notealias),
Index: sbin/ifconfig/Makefile
===================================================================
--- sbin/ifconfig/Makefile (revision 196234)
+++ sbin/ifconfig/Makefile (working copy)
@@ -27,8 +27,8 @@ SRCS+= ifgre.c # GRE keys etc
 SRCS+= ifgif.c # GIF reversed header workaround
 
 SRCS+= ifieee80211.c regdomain.c # SIOC[GS]IEEE80211 support
-DPADD+= ${LIBBSDXML} ${LIBSBUF} ${LIBJAIL}
-LDADD+= -lbsdxml -ljail -lsbuf
+DPADD+= ${LIBBSDXML} ${LIBSBUF} ${LIBJAIL} ${LIBUTIL}
+LDADD+= -lbsdxml -ljail -lsbuf -lutil
 
 SRCS+= ifcarp.c # SIOC[GS]VH support
 SRCS+= ifgroup.c # ...
Index: lib/libutil/Makefile
===================================================================
--- lib/libutil/Makefile (revision 196234)
+++ lib/libutil/Makefile (working copy)
@@ -10,11 +10,12 @@ SHLIB_MAJOR= 8
 
 SRCS= _secure_path.c auth.c expand_number.c flopen.c fparseln.c gr_util.c \
  hexdump.c humanize_number.c kinfo_getfile.c kinfo_getvmmap.c kld.c \
+ if_description.c \
  login.c login_auth.c login_cap.c \
  login_class.c login_crypt.c login_ok.c login_times.c login_tty.c \
  logout.c logwtmp.c pidfile.c property.c pty.c pw_util.c realhostname.c \
  stub.c trimdomain.c uucplock.c
-INCS= libutil.h login_cap.h
+INCS= if_description.h libutil.h login_cap.h
 
 WARNS?= 6
 
@@ -31,7 +32,7 @@ MAN+= kld.3 login.3 login_auth.3 login_tty.3 logou
  _secure_path.3 uucplock.3 property.3 auth.3 realhostname.3 \
  realhostname_sa.3 trimdomain.3 fparseln.3 humanize_number.3 \
  pidfile.3 flopen.3 expand_number.3 hexdump.3 \
- kinfo_getfile.3 kinfo_getvmmap.3
+ kinfo_getfile.3 kinfo_getvmmap.3 if_description.3
 MAN+= login.conf.5 auth.conf.5
 MLINKS+= kld.3 kld_isloaded.3 kld.3 kld_load.3
 MLINKS+= property.3 properties_read.3  property.3 properties_free.3
@@ -59,5 +60,6 @@ MLINKS+=pidfile.3 pidfile_open.3 \
  pidfile.3 pidfile_write.3 \
  pidfile.3 pidfile_close.3 \
  pidfile.3 pidfile_remove.3
+MLINKS+= if_description.3 setifdescription.3 if_description.3 getifdescription.3
 
 .include <bsd.lib.mk>
Index: lib/libutil/if_description.3
===================================================================
--- lib/libutil/if_description.3 (revision 0)
+++ lib/libutil/if_description.3 (revision 0)
@@ -0,0 +1,90 @@
+.\"-
+.\" Copyright (c) 2009 Xin LI <delphij@...>
+.\" All rights reserved.
+.\"
+.\" Redistribution and use in source and binary forms, with or without
+.\" modification, are permitted provided that the following conditions
+.\" are met:
+.\" 1. Redistributions of source code must retain the above copyright
+.\"    notice, this list of conditions and the following disclaimer.
+.\" 2. Redistributions in binary form must reproduce the above copyright
+.\"    notice, this list of conditions and the following disclaimer in the
+.\"    documentation and/or other materials provided with the distribution.
+.\"
+.\" THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+.\" ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+.\" IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+.\" ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+.\" FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+.\" DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+.\" OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+.\" SUCH DAMAGE.
+.\"
+.\" $FreeBSD$
+.\"
+.Dd September 15, 2009
+.Os
+.Dt IFSETDESCRIPTION 3
+.Sh NAME
+.Nm ifsetdescription ,
+.Nm ifgetdescription
+.Nd ifnet description utility functions
+.Sh LIBRARY
+.Lb libutil
+.Sh SYNOPSIS
+.In if_description.h
+.Ft int
+.Fn setifdescription "int s" "struct ifreq *ifr" "const char *val"
+.Ft int
+.Fn getifdescription "int s" "struct ifreq *ifr" "char **val"
+.Sh DESCRIPTION
+These functions manipulates auxiliary interface description facility
+from userland.
+.Pp
+The
+.Fn setifdescription
+function store the description
+.Fa val
+on the socket
+.Fa s
+referencing the interface and a struct ifreq pointer
+.Fa ifr
+that is already filled in with appropriate request information.
+If
+.Fa val
+is NULL or is a pointer to empty string,
+the interface description is removed.
+.Pp
+The
+.Fn getifdescription
+function reads and fills the description information into
+.Fa *val
+from the socket
+.Fa s
+with a struct ifreq pointer
+.Fa ifr
+that is already filled in with appropriate request information.
+The previous
+.Fa *val
+will be freed and reallocated if it is not NULL.
+.Sh RETURN VALUES
+.Rv -std
+.Sh SEE ALSO
+.Xr netintro 4 ,
+.Sh HISTORY
+The
+.Fn ifsetdescription
+and
+.Fn ifgetdescrition
+functions first appeared in
+.Fx 9.0 .
+.Sh AUTHORS
+The
+.Fn ifsetdescription
+and
+.Fn ifgetdescription
+functions and this manual page were written by
+.An Xin LI Aq delphij@... .

Property changes on: lib/libutil/if_description.3
___________________________________________________________________
Added: svn:mime-type
   + text/plain
Added: svn:keywords
   + FreeBSD=%H
Added: svn:eol-style
   + native

Index: lib/libutil/if_description.c
===================================================================
--- lib/libutil/if_description.c (revision 0)
+++ lib/libutil/if_description.c (revision 0)
@@ -0,0 +1,107 @@
+/*-
+ * Copyright (c) 2009 Xin LI <delphij@...>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD$
+ */
+
+#include <sys/param.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/socket.h>
+#include <sys/cdefs.h>
+#include <net/if.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <db.h>
+#include <if_description.h>
+
+int
+setifdescription(int s, struct ifreq *ifr, const char *val)
+{
+ DB *res;
+ DBT key, entry;
+ char dname[IFNAMSIZ];
+
+ bzero(dname, sizeof(dname));
+ ifr->ifr_data = (caddr_t)dname;
+
+ errno = ioctl(s, SIOCGIFDNAME, ifr);
+ if (errno == 0) {
+ res = dbopen(_PATH_IFDESCR_DB,
+ O_RDWR|O_CREAT|O_EXLOCK,
+ S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH,
+ DB_HASH, NULL);
+ if (res != NULL) {
+ key.data = dname;
+ key.size = strlen(dname);
+
+ if (val == NULL || strlen(val) == 0) {
+ (res->del)(res, &key, 0);
+ } else {
+ entry.data = __DECONST(void *, val);
+ entry.size = strlen(val) + 1;
+ (res->put)(res, &key, &entry, 0);
+ }
+ (res->close)(res);
+ return 0;
+ }
+ }
+
+ return -1;
+}
+
+int
+getifdescription(int s, struct ifreq *ifr, char **val)
+{
+ DB *res;
+ DBT key, entry;
+ char dname[IFNAMSIZ];
+
+ bzero(dname, sizeof(dname));
+ ifr->ifr_data = (caddr_t)dname;
+
+ free(*val);
+ *val = NULL;
+
+ errno = ioctl(s, SIOCGIFDNAME, ifr);
+ if (errno == 0) {
+ res = dbopen(_PATH_IFDESCR_DB,
+ O_RDONLY|O_SHLOCK, 0, DB_HASH, NULL);
+ if (res != NULL) {
+ key.data = dname;
+ key.size = strlen(dname);
+
+ if ((res->get)(res, &key, &entry, 0) == 0) {
+ *val = malloc(entry.size);
+ strlcpy(*val, entry.data, entry.size);
+ }
+ (res->close)(res);
+ }
+ }
+ return 0;
+}
+

Property changes on: lib/libutil/if_description.c
___________________________________________________________________
Added: svn:mime-type
   + text/plain
Added: svn:keywords
   + FreeBSD=%H
Added: svn:eol-style
   + native

Index: lib/libutil/if_description.h
===================================================================
--- lib/libutil/if_description.h (revision 0)
+++ lib/libutil/if_description.h (revision 0)
@@ -0,0 +1,37 @@
+/*-
+ * Copyright (c) 2009 Xin LI <delphij@...>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD$
+ */
+
+#ifndef _IF_DESCRIPTION_H_
+#define _IF_DESCRIPTION_H_
+
+#define _PATH_IFDESCR_DB "/etc/ifdescr.db"
+
+int setifdescription(int s, struct ifreq *ifr, const char *val);
+int getifdescription(int s, struct ifreq *ifr, char **val);
+
+#endif /* _IF_DESCRIPTION_H_ */

Property changes on: lib/libutil/if_description.h
___________________________________________________________________
Added: svn:mime-type
   + text/plain
Added: svn:keywords
   + FreeBSD=%H
Added: svn:eol-style
   + native

Index: sys/net/if.c
===================================================================
--- sys/net/if.c (revision 196234)
+++ sys/net/if.c (working copy)
@@ -1927,6 +1927,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t d
  int new_flags, temp_flags;
  size_t namelen, onamelen;
  char new_name[IFNAMSIZ];
+ char dname[IFNAMSIZ];
  struct ifaddr *ifa;
  struct sockaddr_dl *sdl;
 
@@ -1965,6 +1966,18 @@ ifhwioctl(u_long cmd, struct ifnet *ifp, caddr_t d
  ifr->ifr_phys = ifp->if_physical;
  break;
 
+ case SIOCGIFDNAME:
+ bzero(dname, sizeof(dname));
+ if (ifp->if_dunit != IF_DUNIT_NONE)
+ snprintf(dname, IFNAMSIZ, "%s%d", ifp->if_dname,
+ ifp->if_dunit);
+ else
+ strlcpy(dname, ifp->if_dname, sizeof(dname));
+ error = copyout(dname, ifr->ifr_data, sizeof(dname));
+ if (error)
+ return (error);
+ break;
+
  case SIOCSIFFLAGS:
  error = priv_check(td, PRIV_NET_SETIFFLAGS);
  if (error)
Index: sys/sys/sockio.h
===================================================================
--- sys/sys/sockio.h (revision 196234)
+++ sys/sys/sockio.h (working copy)
@@ -82,6 +82,7 @@
 #define SIOCGIFMAC _IOWR('i', 38, struct ifreq) /* get IF MAC label */
 #define SIOCSIFMAC _IOW('i', 39, struct ifreq) /* set IF MAC label */
 #define SIOCSIFNAME _IOW('i', 40, struct ifreq) /* set IF name */
+#define SIOCGIFDNAME _IOWR('i', 41, struct ifreq) /* get IF dname */
 
 #define SIOCADDMULTI _IOW('i', 49, struct ifreq) /* add m'cast addr */
 #define SIOCDELMULTI _IOW('i', 50, struct ifreq) /* del m'cast addr */

_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Andre Oppermann-2]

Xin LI wrote:

> Hi, guys,
>
> Here is a patch that implements the functionality in userland (as
> setifdescription/getifdescription functions in libutil); with this I
> think we can also provide an option that some kernel feature (like Qing
> Li said it might be useful for embedded systems, but of course the
> kernel feature would require more careful design) being used without
> modifying programs.  This version uses if_dname plus if_dunit as
> distinguished name, and a Berkeley DB (hash, /etc/ifdescr.db) to store
> the information.

I don't like this approach.  Adding unconditional dependencies to libutil
and libbsdxml unnecessarily complicates and bloats up ifconfig, which is
a very central utility that resides on every rescue and minimal boot disk.

Additionally the DB stored description can easily go out of sync when
interface disappear and reappear.  On top of that it complicates porting
of routing daemons (Quagga suite, OpenBGPD/OSPFD).  Having it only for
ifconfig but nowhere else is not entirely pointless but seriously reduces
its usefulness.

IMHO interface description is a very useful feature and it should be stored
in the kernel attached to struct ifnet.  However it probably should only be
a pointer to malloc'ed memory.  It is only infrequently accessed and never
in a critical code path.  A slight disadvantage is either a separate IOCTL
to retrieve the description or a little hack to copy it into struct ifnet
just when it is retrieved by userspace from the kernel.  [Which reminds me
that somewhen we need something more flexible than the current routing socket.]

--
Andre

_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Julian Elischer]

Andre Oppermann wrote:

> Xin LI wrote:
>> Hi, guys,
>>
>> Here is a patch that implements the functionality in userland (as
>> setifdescription/getifdescription functions in libutil); with this I
>> think we can also provide an option that some kernel feature (like Qing
>> Li said it might be useful for embedded systems, but of course the
>> kernel feature would require more careful design) being used without
>> modifying programs.  This version uses if_dname plus if_dunit as
>> distinguished name, and a Berkeley DB (hash, /etc/ifdescr.db) to store
>> the information.
>
> I don't like this approach.  Adding unconditional dependencies to libutil
> and libbsdxml unnecessarily complicates and bloats up ifconfig, which is
> a very central utility that resides on every rescue and minimal boot disk.
>
> Additionally the DB stored description can easily go out of sync when
> interface disappear and reappear.  On top of that it complicates porting
> of routing daemons (Quagga suite, OpenBGPD/OSPFD).  Having it only for
> ifconfig but nowhere else is not entirely pointless but seriously reduces
> its usefulness.
>
> IMHO interface description is a very useful feature and it should be stored
> in the kernel attached to struct ifnet.  However it probably should only be
> a pointer to malloc'ed memory.  It is only infrequently accessed and never
> in a critical code path.  A slight disadvantage is either a separate IOCTL
> to retrieve the description or a little hack to copy it into struct ifnet
> just when it is retrieved by userspace from the kernel.  [Which reminds me
> that somewhen we need something more flexible than the current routing
> socket.]
>

 From my perspective, putting it in a separate db outside the kernel
kind of defeats the purpose. I thought the  first patches had the
right idea. though for me the current ability to rename an interface
is good enough.  I mean is you can cal your interface "Sydney0" or
"Melbourne2"  that is really enough..
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[sthaug]

>  From my perspective, putting it in a separate db outside the kernel
> kind of defeats the purpose. I thought the  first patches had the
> right idea. though for me the current ability to rename an interface
> is good enough.  I mean is you can cal your interface "Sydney0" or
> "Melbourne2"  that is really enough..

Having read the discussion, I agree that the description should be
in the kernel. However, being a router geek the ability to rename
an interface to "Sydney0" or "Melbourne2" is not at all enough. For
the routers & switches I work with we really want a description of
at least 50 characters - and it's important to be able to include
space.

Steinar Haug, Nethelp consulting, sthaug@...
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[randyb]

>>  From my perspective, putting it in a separate db outside the kernel
>> kind of defeats the purpose. I thought the  first patches had the
>> right idea. though for me the current ability to rename an interface
>> is good enough.  I mean is you can cal your interface "Sydney0" or
>> "Melbourne2"  that is really enough..
> Having read the discussion, I agree that the description should be
> in the kernel. However, being a router geek the ability to rename
> an interface to "Sydney0" or "Melbourne2" is not at all enough. For
> the routers & switches I work with we really want a description of
> at least 50 characters - and it's important to be able to include
> space.

also a router geek.  but for the sake of simplicity, i am quite willing
to s/\ /_/ or whatever.

randy
_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."

[Xin LI-2]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Here is a revised patchset based on previous comments.  This version
would store the description in kernel space, but struct ifnet would only
have a reference to the sbuf storing the description, supporting
arbitrary description length.

Note that, because we now have a different KPI/API with OpenBSD's
counterpart, this patchset would break some stuff like libpcap which
expect OpenBSD API.

Comments?

Cheers,
- --
Xin LI <delphij@...> http://www.delphij.net/
FreeBSD - The Power to Serve!       Live free or die
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.13 (FreeBSD)

iEYEARECAAYFAkr0f08ACgkQi+vbBBjt66CLRwCfcn0b+95uQvAI5k+gs4VdQ6F+
DOcAniPvnqclIoxRbhzxI1uZOivqU/N9
=bB5n
-----END PGP SIGNATURE-----

Index: sbin/ifconfig/ifconfig.8
===================================================================
--- sbin/ifconfig/ifconfig.8 (revision 198964)
+++ sbin/ifconfig/ifconfig.8 (working copy)
@@ -28,7 +28,7 @@
 .\"     From: @(#)ifconfig.8 8.3 (Berkeley) 1/5/94
 .\" $FreeBSD$
 .\"
-.Dd September 23, 2009
+.Dd November 11, 2009
 .Dt IFCONFIG 8
 .Os
 .Sh NAME
@@ -258,6 +258,12 @@
 Another name for the
 .Fl alias
 parameter.
+.It Cm description Ar value
+Specify a description of the interface.
+This can be used to label interfaces in situations where they may
+otherwise be difficult to distinguish.
+.It Cm -description
+Clear the interface description.
 .It Cm down
 Mark an interface
 .Dq down .
@@ -2512,6 +2518,10 @@
 to use 100baseTX, full duplex Ethernet media options:
 .Dl # ifconfig xl0 media 100baseTX mediaopt full-duplex
 .Pp
+Label the em0 interface as an uplink:
+.Pp
+.Dl # ifconfig em0 description \&"Uplink to Gigabit Switch 2\&"
+.Pp
 Create the software network interface
 .Li gif1 :
 .Dl # ifconfig gif1 create
Index: sbin/ifconfig/ifconfig.c
===================================================================
--- sbin/ifconfig/ifconfig.c (revision 198964)
+++ sbin/ifconfig/ifconfig.c (working copy)
@@ -83,6 +83,8 @@
 struct ifreq ifr;
 
 char name[IFNAMSIZ];
+char *descr = NULL;
+size_t descrlen = 64;
 int setaddr;
 int setmask;
 int doalias;
@@ -822,6 +824,36 @@
  free(newname);
 }
 
+/* ARGSUSED */
+static void
+setifdescr(const char *val, int dummy __unused, int s,
+    const struct afswtch *afp)
+{
+ char *newdescr;
+
+ newdescr = strdup(val);
+ if (newdescr == NULL) {
+ warn("no memory to set ifdescr");
+ return;
+ }
+ ifr.ifr_buffer.buffer = newdescr;
+ ifr.ifr_buffer.length = strlen(newdescr);
+ if (ioctl(s, SIOCSIFDESCR, (caddr_t)&ifr) < 0) {
+ warn("ioctl (set descr)");
+ free(newdescr);
+ return;
+ }
+ free(newdescr);
+}
+
+/* ARGSUSED */
+static void
+unsetifdescr(const char *val, int value, int s, const struct afswtch *afp)
+{
+
+ setifdescr("", 0, s, 0);
+}
+
 #define IFFBITS \
 "\020\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6SMART\7RUNNING" \
 "\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX\15LINK0\16LINK1\17LINK2" \
@@ -866,6 +898,23 @@
  printf(" mtu %d", ifr.ifr_mtu);
  putchar('\n');
 
+ descr = reallocf(descr, descrlen);
+ if (descr != NULL) {
+ do {
+ ifr.ifr_buffer.buffer = descr;
+ ifr.ifr_buffer.length = descrlen;
+ if (ioctl(s, SIOCGIFDESCR, &ifr) == 0) {
+    if (strlen(descr) > 0)
+ printf("\tdescription: %s\n", descr);
+    break;
+ }
+ if (errno == ENAMETOOLONG) {
+ descrlen *= 2;
+ descr = reallocf(descr, descrlen);
+ }
+ } while (errno == ENAMETOOLONG);
+ }
+
  if (ioctl(s, SIOCGIFCAP, (caddr_t)&ifr) == 0) {
  if (ifr.ifr_curcap != 0) {
  printb("\toptions", ifr.ifr_curcap, IFCAPBITS);
@@ -1035,6 +1084,10 @@
  DEF_CMD("-arp", IFF_NOARP, setifflags),
  DEF_CMD("debug", IFF_DEBUG, setifflags),
  DEF_CMD("-debug", -IFF_DEBUG, setifflags),
+ DEF_CMD_ARG("description", setifdescr),
+ DEF_CMD_ARG("descr", setifdescr),
+ DEF_CMD("-description", 0, unsetifdescr),
+ DEF_CMD("-descr", 0, unsetifdescr),
  DEF_CMD("promisc", IFF_PPROMISC, setifflags),
  DEF_CMD("-promisc", -IFF_PPROMISC, setifflags),
  DEF_CMD("add", IFF_UP, notealias),
Index: share/man/man4/netintro.4
===================================================================
--- share/man/man4/netintro.4 (revision 198964)
+++ share/man/man4/netintro.4 (working copy)
@@ -32,7 +32,7 @@
 .\"     @(#)netintro.4 8.2 (Berkeley) 11/30/93
 .\" $FreeBSD$
 .\"
-.Dd June 18, 2004
+.Dd November 11, 2009
 .Dt NETINTRO 4
 .Os
 .Sh NAME
@@ -277,6 +277,15 @@
 fields of the
 .Vt ifreq
 structure, respectively.
+.It Dv SIOCGIFDESCR Fa "struct ifreq *"
+Get the interface description, returned in the
+.Va ifru_data
+field.
+.It Dv SIOCSIFDESCR Fa "struct ifreq *"
+Set the interface description to the value of the
+.Va ifru_data
+field, limited to the size of
+.Dv IFDESCRSIZE .
 .It Dv SIOCSIFFLAGS
 Set interface flags field.
 If the interface is marked down,
Index: sys/kern/kern_jail.c
===================================================================
--- sys/kern/kern_jail.c (revision 198964)
+++ sys/kern/kern_jail.c (working copy)
@@ -3467,6 +3467,7 @@
  case PRIV_NET_SETIFMTU:
  case PRIV_NET_SETIFFLAGS:
  case PRIV_NET_SETIFCAP:
+ case PRIV_NET_SETIFDESCR:
  case PRIV_NET_SETIFNAME :
  case PRIV_NET_SETIFMETRIC:
  case PRIV_NET_SETIFPHYS:
Index: sys/net/if.c
===================================================================
--- sys/net/if.c (revision 198964)
+++ sys/net/if.c (working copy)
@@ -463,6 +463,8 @@
 #ifdef MAC
  mac_ifnet_destroy(ifp);
 #endif /* MAC */
+ if (ifp->if_description != NULL)
+ sbuf_delete(ifp->if_description);
  IF_AFDATA_DESTROY(ifp);
  IF_ADDR_LOCK_DESTROY(ifp);
  ifq_delete(&ifp->if_snd);
@@ -2090,6 +2092,45 @@
  ifr->ifr_phys = ifp->if_physical;
  break;
 
+ case SIOCGIFDESCR:
+ IF_AFDATA_RLOCK(ifp);
+ if (ifp->if_description == NULL)
+ error = ENOMSG;
+ else
+ error = copystr(sbuf_data(ifp->if_description),
+ ifr->ifr_buffer.buffer,
+ ifr->ifr_buffer.length, NULL);
+ IF_AFDATA_RUNLOCK(ifp);
+ break;
+
+ case SIOCSIFDESCR:
+ error = priv_check(td, PRIV_NET_SETIFDESCR);
+ if (error)
+ return (error);
+
+ IF_AFDATA_WLOCK(ifp);
+ if (ifp->if_description == NULL) {
+ ifp->if_description = sbuf_new_auto();
+ if (ifp->if_description == NULL) {
+ error = ENOMEM;
+ IF_AFDATA_WUNLOCK(ifp);
+ break;
+ }
+ } else
+ sbuf_clear(ifp->if_description);
+
+ if (sbuf_copyin(ifp->if_description, ifr->ifr_buffer.buffer,
+ ifr->ifr_buffer.length) == -1)
+ error = EFAULT;
+
+ if (error == 0) {
+ sbuf_finish(ifp->if_description);
+ getmicrotime(&ifp->if_lastchange);
+ }
+ IF_AFDATA_WUNLOCK(ifp);
+
+ break;
+
  case SIOCSIFFLAGS:
  error = priv_check(td, PRIV_NET_SETIFFLAGS);
  if (error)
Index: sys/net/if.h
===================================================================
--- sys/net/if.h (revision 198964)
+++ sys/net/if.h (working copy)
@@ -56,6 +56,7 @@
  * Note: this is the same size as a generic device's external name.
  */
 #define IF_NAMESIZE 16
+
 #if __BSD_VISIBLE
 #define IFNAMSIZ IF_NAMESIZE
 #define IF_MAXUNIT 0x7fff /* historical value */
@@ -294,6 +295,7 @@
  struct sockaddr ifru_addr;
  struct sockaddr ifru_dstaddr;
  struct sockaddr ifru_broadaddr;
+ struct { size_t length; caddr_t buffer; } ifru_buffer;
  short ifru_flags[2];
  short ifru_index;
  int ifru_jid;
@@ -307,6 +309,7 @@
 #define ifr_addr ifr_ifru.ifru_addr /* address */
 #define ifr_dstaddr ifr_ifru.ifru_dstaddr /* other end of p-to-p link */
 #define ifr_broadaddr ifr_ifru.ifru_broadaddr /* broadcast address */
+#define ifr_buffer ifr_ifru.ifru_buffer /* user supplied buffer with its length */
 #define ifr_flags ifr_ifru.ifru_flags[0] /* flags (low 16 bits) */
 #define ifr_flagshigh ifr_ifru.ifru_flags[1] /* flags (high 16 bits) */
 #define ifr_jid ifr_ifru.ifru_jid /* jail/vnet */
Index: sys/net/if_var.h
===================================================================
--- sys/net/if_var.h (revision 198964)
+++ sys/net/if_var.h (working copy)
@@ -198,6 +198,7 @@
  void *if_pf_kif;
  void *if_lagg; /* lagg glue */
  u_char if_alloctype; /* if_type at time of allocation */
+ struct sbuf *if_description; /* interface description */
 
  /*
  * Spare fields are added so that we can modify sensitive data
@@ -205,7 +206,7 @@
  * be used with care where binary compatibility is required.
  */
  char if_cspare[3];
- void *if_pspare[8];
+ void *if_pspare[7];
  int if_ispare[4];
 };
 
Index: sys/sys/priv.h
===================================================================
--- sys/sys/priv.h (revision 198964)
+++ sys/sys/priv.h (working copy)
@@ -335,6 +335,7 @@
 #define PRIV_NET_LAGG 415 /* Administer lagg interface. */
 #define PRIV_NET_GIF 416 /* Administer gif interface. */
 #define PRIV_NET_SETIFVNET 417 /* Move interface to vnet. */
+#define PRIV_NET_SETIFDESCR 418 /* Set interface description. */
 
 /*
  * 802.11-related privileges.
Index: sys/sys/sockio.h
===================================================================
--- sys/sys/sockio.h (revision 198964)
+++ sys/sys/sockio.h (working copy)
@@ -82,6 +82,8 @@
 #define SIOCGIFMAC _IOWR('i', 38, struct ifreq) /* get IF MAC label */
 #define SIOCSIFMAC _IOW('i', 39, struct ifreq) /* set IF MAC label */
 #define SIOCSIFNAME _IOW('i', 40, struct ifreq) /* set IF name */
+#define SIOCSIFDESCR _IOW('i', 41, struct ifreq) /* set ifnet descr */
+#define SIOCGIFDESCR _IOWR('i', 42, struct ifreq) /* get ifnet descr */
 
 #define SIOCADDMULTI _IOW('i', 49, struct ifreq) /* add m'cast addr */
 #define SIOCDELMULTI _IOW('i', 50, struct ifreq) /* del m'cast addr */

_______________________________________________
freebsd-net@... mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@..."