Introduce the USB umb(4) network driver

[khorben_defora.org]

Summary

This includes the port of a driver originally from OpenBSD, later ported to NetBSD by myself:

• The umb(4) kernel driver
• The umbctl(8) companion tool

This driver supports USB network devices implementing the Mobile Broadband Interface Model (MBIM), often found in modern (internal) USB models for 4G/LTE mobile broadband access. It is currently limited to IPv4.

umbctl has to be used to display or set MBIM cellular modem interface parameters (4G/LTE).

This has historically been submitted and progress tracked at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263783.

Sponsored by: The FreeBSD Foundation

Test Plan

The driver is available as a module. After building and installing the updated kernel and companion tool:

# kldload umb
# umbctl umb0 apn <apn> [username <username>] [password <password>]
# ifconfig umb0 up
# ifconfig umb0
[obtain the destination IP]
# route add default <destination IP>

[lwhsu]

sbin/Makefile

66

Is umbctl needed in boot or single user mode? perhaps it's more suitable in /usr/sbin ?

sbin/umbctl/umbctl.8

2

If this is ported from NetBSD, we need the full VCS string.

5

"All rights reserved" is not necessary anymore.

sbin/umbctl/umbctl.c

2

We don't add VCS string in new files anymore.

3

Need full NetBSD VCS string.

42

To confirm: do you (they) mean to use the 3-clause BSD license?

sys/dev/usb/net/if_umb.c

2

We don't add VCS string anymore.

3

Full NetBSD VCS string.

35

To confirm: do you (they) mean to use the 3-clause BSD license?

sys/dev/usb/net/mbim.h

33

To confirm: do you (they) mean to use the 3-clause BSD license?

sys/modules/usb/umb/Makefile

3

Those blank lines are leftover from removing $FreeBSD$ and not needed anymore

[adrian]

out of curiousity - what do people think about extracting a bunch of code out of umbctl and stuffing it in a library that can be reused?

(Don't block landing this on it though - we can definitely do a library-ify pass once it's landed.)

[adrian]

(also nice work!)

[imp]

Where would it be reused?

[adrian]

i could forsee someone writing something like a network manager plugin that doesn't specifically just want to call and parse shell output.

(i've been asked again about this for net80211 and scan management into various other userland things, and i'm helping someone through it, that's why it's on top of mind right now)

[khorben_defora.org]

sbin/Makefile

66

I think it's possible to encounter situations where umb(4) will be needed early by the user, since it's a network interface and there is no dependency on anything from /usr. NFS over broadband network is an example, however unlikely it might be. In a broader picture, umbctl(8) is really an extension to ifconfig(8), which resides in /sbin itself.

sbin/umbctl/umbctl.c

42

Adista asked me to use this text explicitly for their part of the license terms.

[khorben_defora.org]

Address feedback from lwhsu@; thanks!

[lwhsu]

sbin/umbctl/umbctl.c

42

Got it. Then we need to add all of them in the SPDX header, possibly it will be like: SPDX-License-Identifier: (BSD-2-Clause AND BSD-3-Clause)

[adrian]

I think it's fine as-is to go in the tree, it's been two years and I'd like to see this in before -15.

Thanks! Are you able to land it or do you need one of us to?

[khorben_defora.org]

In D48167#1103156, @adrian wrote:

I think it's fine as-is to go in the tree, it's been two years and I'd like to see this in before -15.

This would be awesome.

Thanks! Are you able to land it or do you need one of us to?

I do not have a commit bit myself, so I need someone to push it for me.

I am currently looking into adding the SPDX license identifier, but it can also be done in a separate review.

The current diff is also available at https://github.com/khorben/freebsd-src/tree/khorben/umb-review, if it helps.

[adrian]

Ok, i'll wait for the SPDX addition and then if I don't get any other feedback I'll land it

[khorben_defora.org]

In D48167#1103157, @khorben_defora.org wrote:

I am currently looking into adding the SPDX license identifier, but it can also be done in a separate review.

That extra commit is now available at https://github.com/khorben/freebsd-src/tree/khorben/umb-review-spdx.
I simply used the most restrictive terms (e.g., just SPDX-License-Identifier: BSD-3-Clause where applicable) as per another place found in the source, instead of the construction with AND suggested by @lwhsu. (e.g., sbin/ifconfig/ifieee80211.c)

[ziaee]

Here's what I've got so far for the manual. I ordered some parts to test but they're not all here yet.

sbin/umbctl/umbctl.8

39–50

54

You could still indent it if you want but that doesn't work well with sometimes narrow consoles and giant fonts.

107

share/man/man4/umb.4

25

We usually put what options are needed to build it into the kernel, and any sysctls that can be used to configure it. Lately a little about devmatch. u3g(4) is how I normally see it, vt(4) is the absolute best one I know of, and rtw89(4) for the most recent one with the devmatch example.

45–54

This section gets pulled into the hardware notes, so it needs to have the driver name and device type for searching. I've been writing a section for style.mdoc but it's maybe not ready yet. Bullet lists make a really nice subsections.

https://www.freebsd.org/releases/14.2R/hardware/#misc-network

60

We don't have this one.

71–73

[khorben_defora.org]

This update to the diff brings:

• SPDX headers
• Some fixes and improvements to the manual page for umb(4) as suggested

[ziaee]

sbin/umbctl/umbctl.c

4

What does original mean in this context?

[khorben_defora.org]

sbin/umbctl/umbctl.c

4

I wrote this tool for NetBSD originally, under my own copyright, before applying some changes under contract on behalf of ADISTA SAS.
I can check how close the version submitted here is to the NetBSD one, it is possible that the changes applied are not sufficient to justify the additional copyright and license, but IANAL so I'd rather not touch it if there's any doubt.

[ziaee]

I'm so excited to test this driver this weekend! Thank you so much for submitting this! All of the suggestions I made are to bring it into alignment with the rest of the freebsd manual and are specified in the manual page style guide, style.mdoc(5). Sorry I'm so late.

sbin/umbctl/umbctl.8

129

In the FreeBSD manual we always put the explaination before the example, with a colon instead of a period.

sbin/umbctl/umbctl.c

4

I see. Just wondering, thanks.

share/man/man4/umb.4

45–54

If it goes in this way, it will render super ragged in the hardware notes with no description. Here's what we have so far for the HARDWARE section style guide: https://reviews.freebsd.org/D48343

[khorben_defora.org]

This change applies the rest of the changes suggested to the manual pages.

[khorben_defora.org]

In D48167#1103394, @ziaee wrote:

I'm so excited to test this driver this weekend! Thank you so much for submitting this! All of the suggestions I made are to bring it into alignment with the rest of the freebsd manual and are specified in the manual page style guide, style.mdoc(5). Sorry I'm so late.

No worries! And this is great feedback, thank you!

The previous update I uploaded was an intermediate version for two reasons: I had to head out for lunch yet wanted to share the progress made so far, and I had based my changes from the e-mail notification instead of from the detailed changes, missing some of your suggestions as a result.

I hope it will work well for you as expected this week-end; let me know how it goes!

sbin/umbctl/umbctl.8

39–50

On the command line, the syntax expects parameters to be passed as separate arguments, as per ifconfig(8).

54

I believe this is a remnant of the original import from OpenBSD; I am not fluent in the macros for manual pages myself.

[ziaee]

The manpage changes look great to me!

[zlei]

sys/dev/usb/net/if_umb.c

592

To keep consistent with other wrappers.

608

The line

	IFQ_SET_MAXLEN(&ifp->if_snd, ifqmaxlen);
	ifp->if_snd.ifq_drv_maxlen = ifqmaxlen;

can be replaced by the wrapper function

	if_setsendqlen(ifp, ifqmaxlen);

609

Use wrapper function.

862

The interface may be moved to another vnet, so this unconditional vnet0 will make troubles. We should set curvnet to interface's vnet, aka ifp->if_vnet.

1083

Ditto.

1761

And this.

1964

Ditto.

[khorben_defora.org]

Integrate suggestions from Zhenlei Huang (zlei@); thank you!

Tested on FreeBSD/amd64.

[adrian]

hi! I just tried compiling this on -HEAD (to land it today!) and it failed; it looks like you need to go adapt it to the iflib/ifnet changes (ie, that hid struct ifnet behind if_private.h.)

Lemme know when you've done that. Look in if_var.h for the access functions/macros you need to use rather than directly deref'ing the ifnet internals.

Thanks!

[khorben_defora.org]

Thanks for the heads up! Looking into it...

[khorben_defora.org]

Updated to compile and work with the new ifabi changes.

Tested on FreeBSD/amd64

[adrian]

yay!