Project

General

Profile

Actions

Feature #14206

open

IP interface MTU changes could emit routing socket messages

Added by Nahum Shalman 3 months ago. Updated about 2 months ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
networking
Start date:
Due date:
% Done:

50%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

Likely starting point: http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/common/inet/ip/ip_if.c?r=62366fbb&fi=ip_sioctl_mtu#ip_sioctl_mtu

Per Dan McDonald (see https://log.omnios.org/smartos/2021-10-29#1635519697-180196):

That's the function, and you probably need to call ip_rts_ifmsg() inside it.
I think all you really need to do is put the call to ip_rts_ifmsg() near the bottom of ip_sioctl_mtu, near the SCTP check.

Similar functionality as implemented in FreeBSD:
https://github.com/freebsd/freebsd-src/commit/48f71763a8428a94be0caabc374188f15b467f6c
Interface down and up also emit messages on FreeBSD.

The goal is to to enable wireguard-go on illumos to detect these events similar to how it is done on FreeBSD:
https://github.com/WireGuard/wireguard-go/blob/39e0b6dade0c64673330a26d4eb86d624337daf0/tun/tun_freebsd.go#L79-L133

Actions #1

Updated by Nahum Shalman about 2 months ago

For MTU, the following change appears to work.

diff --git a/usr/src/uts/common/inet/ip/ip_if.c b/usr/src/uts/common/inet/ip/ip_if.c
index 638a5e4b21..bb5288c0dc 100644
--- a/usr/src/uts/common/inet/ip/ip_if.c
+++ b/usr/src/uts/common/inet/ip/ip_if.c
@@ -10898,6 +10898,10 @@ ip_sioctl_mtu(ipif_t *ipif, sin_t *sin, queue_t *q, mblk_t *mp,

        /* Update the MTU in SCTP's list */
        sctp_update_ipif(ipif, SCTP_IPIF_UPDATE);
+
+       /* Make sure routing socket sees that mtu has changed */
+       ip_rts_ifmsg(ipif, RTSQ_DEFAULT);
+
        return (0);
 }

I have, however, noticed a mismatch sometimes between the interface index in the message and the interface index of the tun device. I'm not sure how that is happening and it's a bit concerning.

I'm also unsure which function(s) would need to be updated to emit messages on interface up and down.
Confounding that is that the tun device, once set down with ifconfig refuses to be set back up with ifconfig.

Actions #2

Updated by Dan McDonald about 2 months ago

I have, however, noticed a mismatch sometimes between the interface index in the message and the interface index of the tun device. I'm not sure how that is happening and it's a bit concerning.

Can you show the details? What does "route monitor" or your routing message reader show for the interface index vs. what "ifconfig -a" says?

Actions #3

Updated by Nahum Shalman about 2 months ago

$ ifconfig tun4
tun4: flags=10010008d0<POINTOPOINT,RUNNING,NOARP,MULTICAST,IPv4,FIXEDMTU> mtu 1420 index 28
        inet 0.0.0.0 --> 0.0.0.0 netmask 0
$ route monitor &
[2] 69994
$ sudo ifconfig tun4 mtu 1200
Nahum: 16 vs 28 vs 28
got message of size 336
RTM_IFINFO: iface status change: len 336, if# 28, flags:<PTP,RUNNING,NOARP,MULTICAST,IPv4>
sockaddrs: <IFP>
 tun4
Nahum2: &{336 3 14 16 16779472 28 6 1200 0 0 0 {0 0 0 0 0 0 0 0 0 0}}
DEBUG: (tun4) 2021/11/29 21:32:12 MTU updated: 1200
$ git diff | grep Nahum
+                       fmt.Printf("Nahum: %d vs %d vs %d\n",msg.Index, tunIfindex, msg.Addrs)
+                       fmt.Printf("Nahum2: %v\n",msg)

I'm looking at
https://src.illumos.org/source/xref/illumos-gate/usr/src/uts/common/net/route.h?r=550b6e40&mo=6625&fi=165#152-165
and
https://github.com/golang/sys/blob/master/unix/ztypes_solaris_amd64.go#L326-L339

The golang struct looks right to me, but then the message as parsed by golang looks like the correct value is in the wrong spot...

Actions #4

Updated by Dan McDonald about 2 months ago

Golang is not forcing 32-bit alignment, and is likely not addressing a hole. Let mdb tell you how it's supposed to be laid out:

> ::print -ath  rt_msghdr_t
0 rt_msghdr_t {
    0 ushort_t rtm_msglen 
    2 uchar_t rtm_version 
    3 uchar_t rtm_type 
    4 ushort_t rtm_index 
    6 uint16_t <<HOLE>>   <<<===== XXX KEBE SAYS AHA!
    8 int rtm_flags 
    c int rtm_addrs 
    10 pid_t rtm_pid 
    14 int rtm_seq 
    18 int rtm_errno 
    1c int rtm_use 
    20 uint_t rtm_inits 
    24 struct rt_metrics rtm_rmx {
        24 uint32_t rmx_locks 
        28 uint32_t rmx_mtu 
        2c uint32_t rmx_hopcount 
        30 uint32_t rmx_expire 
        34 uint32_t rmx_recvpipe 
        38 uint32_t rmx_sendpipe 
        3c uint32_t rmx_ssthresh 
        40 uint32_t rmx_rtt 
        44 uint32_t rmx_rttvar 
        48 uint32_t rmx_pksent        
    }
}
Actions #5

Updated by Dan McDonald about 2 months ago

I'd recommend inserting "Hole uint16" in between Index and Flags on your go structure.

Actions #6

Updated by Nahum Shalman about 2 months ago

Looks like the golang code was broken in https://github.com/golang/sys/commit/9029056a598ab609e3c81aae2b8607ac1bfe02eb
I'll file a bug on the golang side. Thanks!!

Edit: Filed as https://github.com/golang/go/issues/49863

Actions #7

Updated by Nahum Shalman about 2 months ago

The swapped fields have to do with the difference between how to parse
rt_msghdr_t vs if_msghdr_t
http://src.illumos.org/source/xref/illumos-gate/usr/src/cmd/cmd-inet/usr.sbin/route.c?r=550b6e40#2614-2620

Actions #8

Updated by Nahum Shalman about 2 months ago

Dan McDonald any thoughts on

I'm also unsure which function(s) would need to be updated to emit messages on interface up and down.

Actions #9

Updated by Electric Monk about 2 months ago

  • Gerrit CR set to 1838
Actions #10

Updated by Nahum Shalman about 2 months ago

  • Status changed from New to In Progress
  • Assignee set to Nahum Shalman
Actions #11

Updated by Nahum Shalman about 2 months ago

  • % Done changed from 0 to 20
Actions #12

Updated by Nahum Shalman about 2 months ago

  • Subject changed from IP interface link up/down and MTU changes could emit routing socket messages to IP interface MTU changes could emit routing socket messages
Actions #13

Updated by Nahum Shalman about 2 months ago

Nahum Shalman wrote in #note-8:

I'm also unsure which function(s) would need to be updated to emit messages on interface up and down.

We already do. The tun device is weird, so they don't quite help wireguard-go, but that's a separate issue. :)

Actions #14

Updated by Nahum Shalman about 2 months ago

  • % Done changed from 20 to 50
Actions

Also available in: Atom PDF