Adding new media types to if_media.h
Eric Joyner
ricera10 at gmail.com
Thu Feb 19 21:53:00 UTC 2015
It does look good! We already have at least a half-dozen new media types to
add.
---
- Eric Joyner
On Tue, Feb 17, 2015 at 9:26 AM, Adrian Chadd <adrian at freebsd.org> wrote:
> Looks good to me.
>
> Thanks for doing this!
>
>
> -a
>
>
> On 16 February 2015 at 17:50, Mike Karels <mike at karels.net> wrote:
> > On Feb 9, gnn wrote:
> >
> >> On 8 Feb 2015, at 22:41, Mike Karels wrote:
> >
> >> > Sorry to reply to a thread after such a long delay, but I think it is
> >> > unresolved, and needs more discussion. I'd like to elaborate a bit on
> >> > my goals and proposal. I believe Adrian has newer thoughts than have
> >> > been
> >> > circulated here as well.
> >> >
> >> > The last message(s) have gone to freebsd-arch and freebsd-net. If
> >> > someone
> >> > wants to pick one, we could consolidate, but this seems relevant to
> >> > both.
> >> >
> >> > I'm going to top-post to try to summarize and extend the discussion,
> >> > but the
> >> > preceding emails follow for reference.
> >> >
> >> > To recap: the existing if_media interface is running out of steam, at
> >> > least
> >> > in that the "Media variant" field, with 5 bits, is going to be
> >> > insufficient
> >> > to express existing 40 Gb/s variants. The if_media media type is a
> >> > 32-bit
> >> > int with a bunch of sub-fields for type (e.g. Ethernet),
> >> > subtype/variant
> >> > (e.g. 10baseT, 10base5, 1000baseT, etc), flags, and some MII-related
> >> > fields.
> >> >
> >> > I made a proposal to extend the interface in a small way, specifically
> >> > to
> >> > replace the "media word" with a 64-bit int that is mostly the same,
> >> > but
> >> > has a new, larger variant/subtype field. The main reason for this
> >> > proposal
> >> > is to maintain the driver KPI (glimpse showed me 240 inclusions of
> >> > if_media.h
> >> > in the kernel in 8.2). That interface includes an initialization
> >> > using a
> >> > scalar value of fields ORed with each other. It would also be easy to
> >> > preserve a 32-bit user-level API/ABI that can express most of the
> >> > current
> >> > state, with a subtype/variant field value reserved for "other" (there
> >> > is
> >> > already one for "unknown", but that is not quite the same). fwiw, I
> >> > found 45 references to this user-level API in our tree, including both
> >> > base and "ports"-type software, which includes libpcap, snmpd,
> >> > dhclient,
> >> > quagga, xorp, atm, devd, and rtsold, which argues for a
> >> > backward-compatible
> >> > API/ABI as well as a more-complete current interface for ifconfig at
> >> > least.
> >> >
> >> > More generally, I see two problems with the existing if_media
> >> > interface:
> >> >
> >> > 1. It doesn't have enough bits for all the fields, in particular,
> >> > variant/
> >> > subtype for Ethernet. That is the immediate issue.
> >> >
> >> > 2. The interface is not sufficiently generic; it was designed around
> >> > Ethernet
> >> > including MII, token ring, FDDI, and a few other interface types.
> >> > Some of
> >> > the fields like "instance" are primarily for MII as far as I know, and
> >> > are
> >> > basically unused. It is definitely not sufficient for 802.11, which
> >> > has
> >> > rolled its own interfaces.
> >> >
> >> > To solve the second problem, I think the right approach would be to
> >> > reduce
> >> > this interface to a truly generic one, such as media type (e.g.
> >> > Ethernet),
> >> > generic flags, and perhaps generic status. Then there should be a
> >> > separate
> >> > media-specific interface for each type, such as Ethernet and 802.11.
> >> > To a
> >> > small extent, we already have that. Solving the second, more general
> >> > problem,
> >> > requires a whole new driver KPI that will require surgery to every
> >> > driver,
> >> > which is not an exercise that I would consider.
> >> >
> >> > Using a separate int for each existing field, as proposed, would break
> >> > the
> >> > driver KPI, but would not really make the interface generic. Trying
> >> > to
> >> > make a single interface with the union of all network interface
> >> > requirements
> >> > seems like a bad idea to me (we failed last time; the "we" is BSDi,
> >> > where
> >> > I was the architect when this interface was first designed). (No, I
> >> > didn't
> >> > design this interface.)
> >> >
> >> > Solving the first problem only, I think it is preferable to preserve a
> >> > compatible driver KPI, which means using a scalar value encoding what
> >> > is
> >> > necessary. Although that interface is rather Ethernet-centric, that
> >> > is
> >> > really what it is used for.
> >> >
> >> > An additional, selfish goal is to make it easy to back-port drivers
> >> > using
> >> > the new interface to older versions (which I am quite likely to do).
> >> > Preserving the KPI and general user API will be highly useful there.
> >> > I'd be likely to do a 11-style version of ifconfig personally, but it
> >> > might not be difficult to do in a more general way.
> >> >
> >> > I am willing to do a prototype for -current for evaluation.
> >> >
> >> > Comments, alternatives, ?
> >
> >> I agree with your statements above and I'd like to see the prototype.
> >
> > Well, I developed the prototype as I had planned, using a 64-bit media
> > word, and found that I got about 100 files in GENERIC that didn't
> compile;
> > they attempted to store "media words" in an int. My kingdom for a
> typedef.
> > That didn't meet my goal of KPI compatibility, so I went to Plan B.
> >
> > Plan B is to steal an unused bit (RFU) to indicate an "extended" media
> > type. I then used the variant/subtype field to store the extended type.
> > Effectively, the previously unused bit doubles the effective size of the
> > subtype field. Given that the previous 5-bit field lasted us 18 years,
> > I figured that doubling it would last a while. I also changed the
> > SIOGGIFMEDIA ioctl, splitting it for binary compatibility; extended
> > types are all mapped to IFM_OTHER (31) using the old interface, but
> > are visible using the new one.
> >
> > With these changes, I modified one driver (vtnet) to use an extended
> type,
> > and the rest of GENERIC is happy. The changes to ifconfig are also
> fairly
> > small. The patch is appended, where email programs will screw it up,
> > or at ftp://ftp.karels.net/outgoing/if_media.patch.
> >
> > The VFAST subtype is a throw-away for testing.
> >
> > This seems like a reasonably pragmatic change to support the new 40 Gb/s
> > media types until someone wants to design an improved but non-backward-
> > compatible interface. I think it meets the goal of suitability for
> > back-porting; it could be MFCed.
> >
> > Mike
> >
> > Index: sys/net/if_media.h
> > ===================================================================
> > --- sys/net/if_media.h (revision 278804)
> > +++ sys/net/if_media.h (working copy)
> > @@ -120,15 +120,29 @@
> > * 5-7 Media type
> > * 8-15 Type specific options
> > * 16-18 Mode (for multi-mode devices)
> > - * 19 RFU
> > + * 19 "extended" bit for media variant
> > * 20-27 Shared (global) options
> > * 28-31 Instance
> > */
> >
> > /*
> > + * As we have used all of the original values for the media variant
> (subtype)
> > + * for Ethernet, extended subtypes have been added, marked with
> XSUBTYPE,
> > + * which is effectively the "high bit" of the media variant (subtype)
> field.
> > + * IFM_OTHER (the highest basic type) is reserved to indicate use of an
> > + * extended type when using an old SIOCGIFMEDIA operation. This is true
> > + * for all media types, not just Ethernet.
> > + */
> > +#define XSUBTYPE 0x80000 /* extended variant high
> bit */
> > +#define _X(var) ((var) | XSUBTYPE) /* extended
> variant */
> > +#define IFM_OTHER 31 /* Other: some
> extended type */
> > +#define OMEDIA(var) (((var) & XSUBTYPE) ? IFM_OTHER : (var))
> > +
> > +/*
> > * Ethernet
> > */
> > #define IFM_ETHER 0x00000020
> > +/* NB: 0,1,2 are auto, manual, none defined below */
> > #define IFM_10_T 3 /* 10BaseT - RJ45 */
> > #define IFM_10_2 4 /* 10Base2 - Thinnet */
> > #define IFM_10_5 5 /* 10Base5 - AUI */
> > @@ -156,11 +170,17 @@
> > #define IFM_40G_CR4 27 /* 40GBase-CR4 */
> > #define IFM_40G_SR4 28 /* 40GBase-SR4 */
> > #define IFM_40G_LR4 29 /* 40GBase-LR4 */
> > +#define IFM_AVAIL30 30 /* available */
> > +/* #define IFM_OTHER 31 Other: some extended type */
> > +/* note 31 is the max! */
> > +
> > +/* Extended variants/subtypes */
> > +#define IFM_VFAST _X(0) /* test "V.fast" */
> > +/* note _X(31) is the max! */
> > /*
> > * Please update ieee8023ad_lacp.c:lacp_compose_key()
> > * after adding new Ethernet media types.
> > */
> > -/* note 31 is the max! */
> >
> > #define IFM_ETH_MASTER 0x00000100 /* master mode
> (1000baseT) */
> > #define IFM_ETH_RXPAUSE 0x00000200 /* receive PAUSE frames
> */
> > @@ -170,6 +190,7 @@
> > * Token ring
> > */
> > #define IFM_TOKEN 0x00000040
> > +/* NB: 0,1,2 are auto, manual, none defined below */
> > #define IFM_TOK_STP4 3 /* Shielded twisted pair
> 4m - DB9 */
> > #define IFM_TOK_STP16 4 /* Shielded twisted pair
> 16m - DB9 */
> > #define IFM_TOK_UTP4 5 /* Unshielded twisted
> pair 4m - RJ45 */
> > @@ -187,6 +208,7 @@
> > * FDDI
> > */
> > #define IFM_FDDI 0x00000060
> > +/* NB: 0,1,2 are auto, manual, none defined below */
> > #define IFM_FDDI_SMF 3 /* Single-mode fiber */
> > #define IFM_FDDI_MMF 4 /* Multi-mode fiber */
> > #define IFM_FDDI_UTP 5 /* CDDI / UTP */
> > @@ -220,6 +242,7 @@
> > #define IFM_IEEE80211_OFDM27 23 /* OFDM 27Mbps */
> > /* NB: not enough bits to express MCS fully */
> > #define IFM_IEEE80211_MCS 24 /* HT MCS rate */
> > +/* #define IFM_OTHER 31 Other: some extended type */
> >
> > #define IFM_IEEE80211_ADHOC 0x00000100 /* Operate in
> Adhoc mode */
> > #define IFM_IEEE80211_HOSTAP 0x00000200 /* Operate in
> Host AP mode */
> > @@ -241,6 +264,7 @@
> > * ATM
> > */
> > #define IFM_ATM 0x000000a0
> > +/* NB: 0,1,2 are auto, manual, none defined below */
> > #define IFM_ATM_UNKNOWN 3
> > #define IFM_ATM_UTP_25 4
> > #define IFM_ATM_TAXI_100 5
> > @@ -277,7 +301,7 @@
> > * Masks
> > */
> > #define IFM_NMASK 0x000000e0 /* Network type */
> > -#define IFM_TMASK 0x0000001f /* Media sub-type */
> > +#define IFM_TMASK 0x0008001f /* Media sub-type */
> > #define IFM_IMASK 0xf0000000 /* Instance */
> > #define IFM_ISHIFT 28 /* Instance shift */
> > #define IFM_OMASK 0x0000ff00 /* Type specific options
> */
> > @@ -372,6 +396,7 @@
> > { IFM_40G_CR4, "40Gbase-CR4" }, \
> > { IFM_40G_SR4, "40Gbase-SR4" }, \
> > { IFM_40G_LR4, "40Gbase-LR4" }, \
> > + { IFM_VFAST, "V.fast" }, \
> > { 0, NULL }, \
> > }
> >
> > @@ -603,6 +628,7 @@
> > { IFM_AUTO, "autoselect" }, \
> > { IFM_MANUAL, "manual" }, \
> > { IFM_NONE, "none" }, \
> > + { IFM_OTHER, "other" }, \
> > { 0, NULL }, \
> > }
> >
> > @@ -673,6 +699,7 @@
> > { IFM_ETHER | IFM_40G_CR4, IF_Gbps(40ULL) }, \
> > { IFM_ETHER | IFM_40G_SR4, IF_Gbps(40ULL) }, \
> > { IFM_ETHER | IFM_40G_LR4, IF_Gbps(40ULL) }, \
> > + { IFM_ETHER | IFM_VFAST, IF_Gbps(40ULL) }, \
> > \
> > { IFM_TOKEN | IFM_TOK_STP4, IF_Mbps(4) }, \
> > { IFM_TOKEN | IFM_TOK_STP16, IF_Mbps(16) }, \
> > Index: sys/sys/sockio.h
> > ===================================================================
> > --- sys/sys/sockio.h (revision 278810)
> > +++ sys/sys/sockio.h (working copy)
> > @@ -128,5 +128,6 @@
> > #define SIOCGIFGROUP _IOWR('i', 136, struct ifgroupreq) /*
> get ifgroups */
> > #define SIOCDIFGROUP _IOW('i', 137, struct ifgroupreq) /*
> delete ifgroup */
> > #define SIOCGIFGMEMB _IOWR('i', 138, struct ifgroupreq) /*
> get members */
> > +#define SIOCGIFXMEDIA _IOWR('i', 139, struct ifmediareq) /*
> get net xmedia */
> >
> > #endif /* !_SYS_SOCKIO_H_ */
> > Index: sys/net/if.c
> > ===================================================================
> > --- sys/net/if.c (revision 278749)
> > +++ sys/net/if.c (working copy)
> > @@ -2561,6 +2561,7 @@
> > case SIOCGIFPSRCADDR:
> > case SIOCGIFPDSTADDR:
> > case SIOCGIFMEDIA:
> > + case SIOCGIFXMEDIA:
> > case SIOCGIFGENERIC:
> > if (ifp->if_ioctl == NULL)
> > return (EOPNOTSUPP);
> > Index: sys/net/if_media.c
> > ===================================================================
> > --- sys/net/if_media.c (revision 278804)
> > +++ sys/net/if_media.c (working copy)
> > @@ -67,7 +67,9 @@
> > static struct ifmedia_entry *ifmedia_match(struct ifmedia *ifm,
> > int flags, int mask);
> >
> > +#define IFMEDIA_DEBUG
> > #ifdef IFMEDIA_DEBUG
> > +#include <net/if_var.h>
> > int ifmedia_debug = 0;
> > SYSCTL_INT(_debug, OID_AUTO, ifmedia, CTLFLAG_RW, &ifmedia_debug,
> > 0, "if_media debugging msgs");
> > @@ -271,6 +273,7 @@
> > * Get list of available media and current media on interface.
> > */
> > case SIOCGIFMEDIA:
> > + case SIOCGIFXMEDIA:
> > {
> > struct ifmedia_entry *ep;
> > int *kptr, count;
> > @@ -278,8 +281,13 @@
> >
> > kptr = NULL; /* XXX gcc */
> >
> > - ifmr->ifm_active = ifmr->ifm_current = ifm->ifm_cur ?
> > - ifm->ifm_cur->ifm_media : IFM_NONE;
> > + if (cmd == SIOCGIFMEDIA) {
> > + ifmr->ifm_active = ifmr->ifm_current =
> ifm->ifm_cur ?
> > + OMEDIA(ifm->ifm_cur->ifm_media) : IFM_NONE;
> > + } else {
> > + ifmr->ifm_active = ifmr->ifm_current =
> ifm->ifm_cur ?
> > + ifm->ifm_cur->ifm_media : IFM_NONE;
> > + }
> > ifmr->ifm_mask = ifm->ifm_mask;
> > ifmr->ifm_status = 0;
> > (*ifm->ifm_status)(ifp, ifmr);
> > @@ -317,7 +325,10 @@
> > ep = LIST_FIRST(&ifm->ifm_list);
> > for (; ep != NULL && count < ifmr->ifm_count;
> > ep = LIST_NEXT(ep, ifm_list), count++)
> > - kptr[count] = ep->ifm_media;
> > + if (cmd == SIOCGIFMEDIA)
> > + kptr[count] =
> OMEDIA(ep->ifm_media);
> > + else
> > + kptr[count] = ep->ifm_media;
> >
> > if (ep != NULL)
> > error = E2BIG; /* oops! */
> > @@ -505,7 +516,7 @@
> > printf("<unknown type>\n");
> > return;
> > }
> > - printf(desc->ifmt_string);
> > + printf("%s", desc->ifmt_string);
> >
> > /* Any mode. */
> > for (desc = ttos->modes; desc && desc->ifmt_string != NULL;
> desc++)
> >
> > Index: sys/dev/virtio/network/if_vtnet.c
> > ===================================================================
> > --- sys/dev/virtio/network/if_vtnet.c (revision 278749)
> > +++ sys/dev/virtio/network/if_vtnet.c (working copy)
> > @@ -938,6 +938,7 @@
> > ifmedia_init(&sc->vtnet_media, IFM_IMASK, vtnet_ifmedia_upd,
> > vtnet_ifmedia_sts);
> > ifmedia_add(&sc->vtnet_media, VTNET_MEDIATYPE, 0, NULL);
> > + ifmedia_add(&sc->vtnet_media, IFM_ETHER | IFM_VFAST, 0, NULL);
> > ifmedia_set(&sc->vtnet_media, VTNET_MEDIATYPE);
> >
> > /* Read (or generate) the MAC address for the adapter. */
> > @@ -1103,6 +1104,7 @@
> >
> > case SIOCSIFMEDIA:
> > case SIOCGIFMEDIA:
> > + case SIOCGIFXMEDIA:
> > error = ifmedia_ioctl(ifp, ifr, &sc->vtnet_media, cmd);
> > break;
> > Index: sbin/ifconfig/ifmedia.c
> > ===================================================================
> > --- sbin/ifconfig/ifmedia.c (revision 278749)
> > +++ sbin/ifconfig/ifmedia.c (working copy)
> > @@ -109,11 +109,17 @@
> > {
> > struct ifmediareq ifmr;
> > int *media_list, i;
> > + int xmedia = 1;
> >
> > (void) memset(&ifmr, 0, sizeof(ifmr));
> > (void) strncpy(ifmr.ifm_name, name, sizeof(ifmr.ifm_name));
> >
> > - if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0) {
> > + /*
> > + * Check if interface supports extended media types.
> > + */
> > + if (ioctl(s, SIOCGIFXMEDIA, (caddr_t)&ifmr) < 0)
> > + xmedia = 0;
> > + if (xmedia == 0 && ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0) {
> > /*
> > * Interface doesn't support SIOC{G,S}IFMEDIA.
> > */
> > @@ -130,8 +136,13 @@
> > err(1, "malloc");
> > ifmr.ifm_ulist = media_list;
> >
> > - if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0)
> > - err(1, "SIOCGIFMEDIA");
> > + if (xmedia) {
> > + if (ioctl(s, SIOCGIFXMEDIA, (caddr_t)&ifmr) < 0)
> > + err(1, "SIOCGIFXMEDIA");
> > + } else {
> > + if (ioctl(s, SIOCGIFMEDIA, (caddr_t)&ifmr) < 0)
> > + err(1, "SIOCGIFMEDIA");
> > + }
> >
> > printf("\tmedia: ");
> > print_media_word(ifmr.ifm_current, 1);
> > @@ -194,6 +205,7 @@
> > {
> > static struct ifmediareq *ifmr = NULL;
> > int *mwords;
> > + int xmedia = 1;
> >
> > if (ifmr == NULL) {
> > ifmr = (struct ifmediareq *)malloc(sizeof(struct
> ifmediareq));
> > @@ -213,7 +225,10 @@
> > * the current media type and the top-level type.
> > */
> >
> > - if (ioctl(s, SIOCGIFMEDIA, (caddr_t)ifmr) < 0) {
> > + if (ioctl(s, SIOCGIFXMEDIA, (caddr_t)ifmr) < 0) {
> > + xmedia = 0;
> > + }
> > + if (xmedia == 0 && ioctl(s, SIOCGIFMEDIA, (caddr_t)ifmr)
> < 0) {
> > err(1, "SIOCGIFMEDIA");
> > }
> >
> > @@ -225,8 +240,13 @@
> > err(1, "malloc");
> >
> > ifmr->ifm_ulist = mwords;
> > - if (ioctl(s, SIOCGIFMEDIA, (caddr_t)ifmr) < 0)
> > - err(1, "SIOCGIFMEDIA");
> > + if (xmedia) {
> > + if (ioctl(s, SIOCGIFXMEDIA, (caddr_t)ifmr) < 0)
> > + err(1, "SIOCGIFXMEDIA");
> > + } else {
> > + if (ioctl(s, SIOCGIFMEDIA, (caddr_t)ifmr) < 0)
> > + err(1, "SIOCGIFMEDIA");
> > + }
> > }
> >
> > return ifmr;
> > _______________________________________________
> > freebsd-net at freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-net
> > To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
> _______________________________________________
> freebsd-arch at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe at freebsd.org"
>
More information about the freebsd-net
mailing list