Page MenuHomeFreeBSD

Add extended media types to if_media.h and ifconfig
AbandonedPublic

Authored by hselasky on Mar 12 2015, 7:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 22, 2:14 PM
Unknown Object (File)
Fri, Apr 5, 1:34 PM
Unknown Object (File)
Fri, Apr 5, 5:18 AM
Unknown Object (File)
Mar 2 2024, 2:21 PM
Unknown Object (File)
Jan 11 2024, 11:26 PM
Unknown Object (File)
Dec 22 2023, 11:35 AM
Unknown Object (File)
Dec 20 2023, 12:20 AM
Unknown Object (File)
Dec 14 2023, 9:07 AM
Subscribers
None

Details

Summary

Fork of D1965 to minimize changes needed.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

hselasky retitled this revision from to Add extended media types to if_media.h and ifconfig.
hselasky updated this object.
hselasky edited the test plan for this revision. (Show Details)
hselasky added reviewers: adrian, jfv, gnn, erj, karels.
hselasky set the repository for this revision to rS FreeBSD src repository - subversion.
hselasky set the repository for this revision to rS FreeBSD src repository - subversion.

Update ieee8023ad_lacp.c:lacp_compose_key() as required by if_media.h

I like the idea of using bits from the Ethernet-specific options field, so as to get more than one additional bit. I'm not sure it is wise to use all of them; perhaps one can be kept (still allows 511 variants).

if_media.h needs some added and changed comments to make it clear that the Ethernet option bits are in use, both in the listing of the bit assignments and where the existing Ethernet options are.

I appreciate the simplicity, but I think some additional complexity is needed. In particular, I think it is bad if old binaries get the media word with SIOCGIFMEDIA and interpret only the low 5 bits as the Ethernet variant and get a portion of the variant. I prefer the approach I used in D1965, reserving a variant value (preferably 31) for "other", and mapping variants above that to that value for SIOCGIFMEDIA. Of course, there needs to be a new ioctl that returns the full value for new ifconfig and any other programs that know about the extended variants. The ifconfig code in D1965 would probably apply without modification. This change affects only ifconfig and the drivers that use extended types.

Hi,

I think we should avoid a new IOCTL. I think the right way is to extend the ifmedia structure used by the IOCTL in question and have a compat entry for older versions of FreeBSD like other drivers are currently doing it. Remember, once you add the extra IOCTL you will be stuck having to do two IOCTLs requests instead of one, like in your patch.

I will update the patch to reserve one more bit, so we don't use em all up at the same time, like you suggest.

--HPS

I think it is not a big problem if old ifconfig binaries display the media word wrong until an update is done. It is very trivial to re-compile ifconfig from -stable and -current. It should not affect any other ifconfig usage.

--HPS

A permanent solution for ifconfig, is to have a new IOCTL which lets the kernel convert the media information into a string. Then ifconfig never needs to be updated in this regard! What do you think?

hselasky edited edge metadata.

Use one less bit for media type giving us a total of 511 ethernet media types.
Make sure the rate is part of all ethernet types.

Binary compatibility is not just for old ifconfig binaries. IIRC, there were on the order of 60 users of SIOCGIFMEDIA in our tree, which included things like SNMP. Doing 2 ioctls in ifconfig is not a big deal.

Hi,

I just did a grep through all the sources and from what I can see, no applications are using IFM_TMASK to get the subtype field. All that I could find are using the IFM_SUBTYPE() macro to do that. The other bits work like before, so I doubt using the same IOCTL like I suggest will cause any breakage.

To make sure this is caught in the future we can rename IFM_TMASK. If any ports are using that should be trivial to clean up!

Can you give an example of a problem with applications in user-space?

--HPS

I don't have a specific example, although SNMP comes to mind as a candidate. However, you seem to be assuming that anything from ports will be recompiled, and that is not a reasonable assumption for embedded systems and appliances (such as I work on).

From old email:

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,

"Our tree" refers to McAfee Firewall Enterprise (Sidewinder).

I see essentially no downside to maintaining better binary compatibility.

Hi,

The SNMP daemon does not use the interface subtype field of the media integer. It is not affected by this change. Maybe in the other patches suggested it would.

Only software which actually uses IFM_TMASK needs updating.

--HPS

Don't forget IFM_SUBTYPE(); not even ifconfig uses TMASK directly.

Note that the pool I searched includes a small fraction of ports.

What's the downside to providing unambiguous binary compatibilty?

net-snmp definitely uses IFM_SUBTYPE. libpcap uses it as well, although it only checks for AUTO. I would think these would be sufficient to justify reasonable binary compatibility.

The IFM_SUBTYPE is the same like before. I don't see the problem.

We need a solution that is backwards compatible, for MFC to 9-stable and 10-stable, that's why I don't want to add new IOCTLs. Possibly in the future we should let the kernel convert and provide those strings, to avoid problems in userspace.

--HPS

IFM_SUBTYPE uses IFM_TMASK, and it's a macro. Thus, anything that uses IFM_SUBTYPE uses IFM_TMASK. It's precisely because of MFCs that I want good binary compatibility. Only ifconfig and drivers using new subtypes need to use the new ioctl, and any ports that want to be able to detect the new subtypes.

Using a string for the subtype would be OK, and is a reasonable choice for 11. Other things should be available separately, e.g. the speed.

Did you check how many applications use the SUBTYPE? In my grep very very few need the SUBTYPE, including ifconfig. The other media flags are more common though.

--HPS

Yes, as I said earlier, libpcap and net-snmp use IFM_SUBTYPE. ifconfig certainly uses it as well. I don't know how many ports use it, or other network monitoring software (including commercial software).

Are you saying it's OK to return incorrect information to applications if it is not very many applications? That doesn't make sense to me. We can't return complete information, but at least we can avoid returning misleading information. It is much better to return "other" than appear to encode "auto" or some ancient type.

I was experimenting with this version, and noticed that VFAST is not in the extended range. If I move it there, I can no longer select it for vtnet. The problem is that the IFM_SUBTYPE_ETHERNET_DESCRIPTIONS array encodes the non-shifted value (in my case, 50), and that doesn't match the shifted value in the if_media list. So it appears that this patch does not work for selecting extended media types.

I think the best way to fix this would be to stop shifting and masking, and define the types as they are encoded. I'll give this a try.

Are you saying it's OK to return incorrect information to applications if it is not very many applications?

Yes, because this information does not affect the usage of the network adapters. The network adapters I work with only supports autonegotiate, so that setting the new media types from user-space is strictly not needed.

--HPS

hselasky set the repository for this revision to rS FreeBSD src repository - subversion.

Add more media types.