ifconfig(8): show parent device for USB Ethernet interfaces
AcceptedPublic
Actions

Authored by lwhsu on Thu, Aug 20, 9:29 PM.

Details

Reviewers

aokblast

Group Reviewers

USB

Summary

ifconfig(8): show parent device for USB Ethernet interfaces

Report net.ue.<unit>.%parent as "parent interface", consistent with
wlan(4):

  # ifconfig ue0
  ue0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
          ether 00:00:5e:00:53:2a
          inet 192.0.2.10/24 broadcast 192.0.2.255
          parent interface: ure0
          media: Ethernet autoselect (2500Base-T <full-duplex>)
          status: active

Sponsored by:	The FreeBSD Foundation

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 76194
Build 73077: arc lint + arc unit

Event Timeline

lwhsu created this revision.Thu, Aug 20, 9:29 PM

Herald added a subscriber: imp. · View Herald TranscriptThu, Aug 20, 9:29 PM

lwhsu requested review of this revision.Thu, Aug 20, 9:29 PM

Harbormaster completed remote builds in B75930: Diff 184616.Thu, Aug 20, 9:29 PM

lwhsu added a reviewer: USB.Thu, Aug 20, 10:20 PM

Just leaving this here... I think we also need an uether.4 man page listing all the ue drivers and that the drivers can .Xr? @ziaee any opinion on that?

Both ifvlan.c and ifieee80211.c have a dedicated ioctl to call for this.
I think it might be time to make this something more generic in ifconfig then if we start growing a "parent interface: " printf line duplication.

In D59060#1354644, @bz wrote:

Both ifvlan.c and ifieee80211.c have a dedicated ioctl to call for this.
I think it might be time to make this something more generic in ifconfig then if we start growing a "parent interface: " printf line duplication.

Yes, I think so too. The long term solution is probably netlink. We have IFLA_PARENT_DEV_NAME in the header, but there seems no implementation yet. That would also avoid adding one more ioctl.

Once we have it, ifconfig can print this line in one place, and we can migrate vlan/wlan/ue to it. There is already a "TODO: convert to netlink" comment in status_nl() in ifconfig_netlink.c.

This patch does not add any new kernel interface, but only reads an existing sysctl. It should be the easiest one to migrate later. I also looked into ifvlan.c and ifieee80211.c to see if it is possible to share some code, but the only common part is the printf. vlan prints it in the middle of a line and ieee80211 uses LINE_CHECK. I feel sharing a helper would not save much for now.

I am currently working on some USB Ethernet improvements, and this patch helps me develop and debug them. I am happy to come back later and look at making this generic in ifconfig.

aokblast added a subscriber: aokblast.Mon, Aug 24, 1:46 AM

aokblast added inline comments.

sbin/ifconfig/ifue.c
38	It is fine but compiler can optimize strlen to __builtin_strlen so maybe it is fine to use strlen directly.
42	Maybe strtol instead of manually implemeting this?
54	Why this is sizeof(parent)? parentlen should return the correct size?

aokblast requested changes to this revision.Mon, Aug 24, 1:47 AM

This revision now requires changes to proceed.Mon, Aug 24, 1:47 AM

lwhsu added inline comments.Wed, Aug 26, 9:33 PM

sbin/ifconfig/ifue.c
38	yeah this is just my personal preference, I am also fine with strlen().
42	after discussion, we can fold with other codes with sscanf().
54	Was trying to be defensive, but after checking the sysctl code more I believe this line can be just removed. parent will be NULL-terminated on success, or sysctlbyname() will return non-zero.

revise after discussing with aokblast.

Harbormaster completed remote builds in B76194: Diff 185170.Wed, Aug 26, 9:33 PM

In D59060#1354642, @bz wrote:

Just leaving this here... I think we also need an uether.4 man page listing all the ue drivers and that the drivers can .Xr? @ziaee any opinion on that?

Yes, absolutely!
I would think it should be ue.4 mlinked to if_ue?

Thanks for tagging me!

sbin/ifconfig/ifue.c
1	Nit: This hyphen was for a long abandoned project that never worked, it has been removed from all of our style guides.

Also this seems like it could be very helpful

LG for implementation part.

sbin/ifconfig/ifue.c
54	No, what you did is. parent[sizeof(parent) - 1]: abcd........\0 what we actually want is parent[returned_size]: abcd\0. But since you have removed the code...