Project

General

Profile

Actions

Bug #8765

closed

dladm tries setting persistent prop on temporary link

Added by Ryan Zezeski almost 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
cmd - userland programs
Start date:
2017-11-05
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:
External Bug:

Description

dladm allows the user to attempt setting a persistent property on a temporary link.

(1) <GZ> root@sys76 [~]
# dladm set-linkprop -p maxbw=0 tmp687240
dladm: warning: invalid link property 'maxbw'

(1) <GZ> root@sys76 [~]
# dladm show-linkprop -p maxbw tmp687240
LINK         PROPERTY        PERM VALUE          DEFAULT        POSSIBLE
tmp687240    maxbw           rw       0          --             --

<GZ> root@sys76 [~]
# dladm set-linkprop -p maxbw=99 tmp687240
dladm: warning: invalid link property 'maxbw'

(1) <GZ> root@sys76 [~]
# dladm show-linkprop -p maxbw tmp687240
LINK         PROPERTY        PERM VALUE          DEFAULT        POSSIBLE
tmp687240    maxbw           rw      99          --             --

In the snippet above notice the property takes effect but dladm complains that it's an invalid property. If you set it as a temporary property then dladm doesn't complain.

<GZ> root@sys76 [~]
# dladm set-linkprop -t -p maxbw=101 tmp687240

<GZ> root@sys76 [~]
# dladm show-linkprop -p maxbw tmp687240
LINK         PROPERTY        PERM VALUE          DEFAULT        POSSIBLE
tmp687240    maxbw           rw     101          --             --

The issue is that dladm will take the code path for setting a persistent property even though it's for a temporary link which has no persistent configuration.

39759/1@1:            -> libdladm:dladm_set_linkprop()
39759/1@1:              -> libdladm:i_dladm_set_linkprop()
39759/1@1:                -> libdladm:dladm_datalink_id2info()
39759/1@1:                  -> libdladm:dladm_door_call()
39759/1@1:                    -> libdladm:dladm_door_fd(0x80c8fd8, 0x80469e4, 0xfedd2a40, 0xfed6d3b6)
39759/1@1:                    <- libdladm:dladm_door_fd() = 0
39759/1:        door_call(4, 0x080469E8)                        = 0
39759/1@1:                    -> libdladm:dladm_errno2status(0x0, 0x80469e8, 0xfedd2a40, 0xfed6d3b6)
39759/1@1:                    <- libdladm:dladm_errno2status() = 0
39759/1@1:                  <- libdladm:dladm_door_call() = 0
39759/1@1:                <- libdladm:dladm_datalink_id2info() = 0
39759/1@1:                -> libdladm:i_dladm_set_single_prop()
39759/1@1:                  -> libdladm:check_maxbw()
39759/1@1:                    -> libdladm:dladm_str2bw()
39759/1@1:                    <- libdladm:dladm_str2bw() = 0
39759/1@1:                  <- libdladm:check_maxbw() = 0
39759/1@1:                  -> libdladm:set_resource()
39759/1@1:                    -> libdladm:i_dladm_buf_alloc_by_name()
39759/1@1:                      -> libdladm:dladm_name2prop(0xfed7e48a, 0xfef20000)
39759/1@1:                      <- libdladm:dladm_name2prop() = 0xfed810c8
39759/1@1:                      -> libdladm:i_dladm_buf_alloc_impl(0x323c, 0x3, 0xfed7e48a, 0x36)
39759/1@1:                      <- libdladm:i_dladm_buf_alloc_impl() = 0x80d5008
39759/1@1:                    <- libdladm:i_dladm_buf_alloc_by_name() = 0x80d5008
39759/1@1:                    -> libdladm:extract_maxbw()
39759/1@1:                    <- libdladm:extract_maxbw() = 0
39759/1@1:                    -> libdladm:i_dladm_macprop()
39759/1@1:                      -> libdladm:dladm_dld_fd(0x80c8fd8, 0x0, 0x0)
39759/1@1:                      <- libdladm:dladm_dld_fd() = 3
39759/1:        ioctl(3, DLDIOC_SETMACPROP, 0x080D5008)         = 0
39759/1@1:                    <- libdladm:i_dladm_macprop() = 0
39759/1@1:                  <- libdladm:set_resource() = 0
39759/1@1:                <- libdladm:i_dladm_set_single_prop() = 0
39759/1@1:              <- libdladm:i_dladm_set_linkprop() = 0
39759/1@1:              -> libdladm:i_dladm_set_linkprop_db()
39759/1@1:                -> libdladm:dladm_open_conf()
39759/1@1:                  -> libdladm:dladm_door_call(0x80c8fd8, 0x8046698, 0x8, 0x8046690)
39759/1@1:                    -> libdladm:dladm_door_fd(0x80c8fd8, 0x8046624, 0x0, 0xfed6d3b6)
39759/1@1:                    <- libdladm:dladm_door_fd() = 0
39759/1:        door_call(4, 0x08046628)                        = 0
39759/1@1:                    -> libdladm:dladm_errno2status(0x2, 0x8046628, 0x0, 0xfed6d3b6)
39759/1@1:                    <- libdladm:dladm_errno2status() = 5
39759/1@1:                  <- libdladm:dladm_door_call() = 5
39759/1@1:                <- libdladm:dladm_open_conf() = 5
39759/1@1:              <- libdladm:i_dladm_set_linkprop_db() = 5
39759/1@1:            <- libdladm:dladm_set_linkprop() = 5

Here truss shows us dladm calling into the kernel to set the property (via DLDIOC_SETMACPROP), but then also calling i_dladm_set_linkprop_db which fails since there is no config for this temporary link.

The fix I made in SmartOS was to make dladm smart enough to notice that the link is temporary and to avoid updating the persistent database in that case. That way if the user elides the -t we still set the property (as we did before) but we don't attempt to update the database which would cause an error and confuse the user.

Actions #1

Updated by Electric Monk almost 5 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 638cab1445079b522f4c99f02eb7eba4d5819aae

commit  638cab1445079b522f4c99f02eb7eba4d5819aae
Author: Ryan Zezeski <rpz@joyent.com>
Date:   2017-11-09T14:29:56.000Z

    8765 dladm tries setting persistent prop on temporary link
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Reviewed by: Sebastian Wiedenroth <sebastian.wiedenroth@skylime.net>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Gordon Ross <gwr@nexenta.com>

Actions

Also available in: Atom PDF