Adding new media types to if_media.h
Jack Vogel
jfvogel at gmail.com
Tue Feb 17 07:32:37 UTC 2015
Nice Mike, I like it also.
Jack
On Mon, Feb 16, 2015 at 5:50 PM, 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"
>
More information about the freebsd-net
mailing list